Skip to content

test(canonical): align live harness docs and result stub#1225

Open
Q00 wants to merge 1 commit into
fix/canonical-live-harness-contractfrom
followup/pr-1214-canonical-doc
Open

test(canonical): align live harness docs and result stub#1225
Q00 wants to merge 1 commit into
fix/canonical-live-harness-contractfrom
followup/pr-1214-canonical-doc

Conversation

@Q00
Copy link
Copy Markdown
Owner

@Q00 Q00 commented May 25, 2026

Summary\n\n- Refreshes canonical fixture docs now that the live opt-in path exists.\n- Replaces the local Result-shaped _Ok stub with Result.ok so the harness test uses the real Result contract.\n\n## Changes\n\n- Updates tests/canonical/conftest.py docstring.\n- Updates tests/canonical/test_canonical.py to use Result.ok and property-based Result access.\n\n## Notes\n\nFollow-up for #1214 stale docs and the coordination note raised from #1218.

Refresh canonical harness fixture documentation now that the live opt-in path exists and replace the local Result-shaped stub with Result.ok. This keeps the #1214 opt-in test aligned with the property-based Result API called out from #1218.\n\nServices: shared\nAffected files:\n- tests/canonical/conftest.py\n- tests/canonical/test_canonical.py
Copy link
Copy Markdown
Contributor

@ouroboros-agent ouroboros-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — ouroboros-agent[bot]

Verdict: APPROVE

Metadata

| Field | Value |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.

---|---|
| PR | #1225 |
| HEAD checked | ae119544fa39bbefd14c2fc836d8c1ba50fe4483 |
| Request ID | req_1779704772_19 |
| Review record | 6dbb9d27-cca7-497e-b243-776756c8a8b4 |

What Improved

  • Canonical live-run test now uses the real Result property API (is_ok, is_err, error) instead of the old callable/fake shape.
  • Canonical fixture docs now describe the existing OUROBOROS_RUN_CANONICAL=1 opt-in path accurately.

Issue Requirements

Requirement Status
No linked issue requirement captured N/A

Prior Findings Status

No prior bot review findings were present in /tmp/pr_prior_bot_reviews_1225.md; no human or inline review comments were present to reconcile.

Blockers

No in-scope blocking findings remained after policy filtering.

Follow-up Findings

# File:Line Priority Confidence Suggestion
None.

Non-blocking Suggestions

None.

Test Coverage Notes

  • Reviewed tests/canonical/test_canonical.py, tests/canonical/conftest.py, tests/canonical/test_conftest.py, src/ouroboros/core/types.py, and src/ouroboros/mcp/tools/auto_handler.py.
  • Verified syntax for changed canonical files with python3 -m py_compile tests/canonical/test_canonical.py tests/canonical/conftest.py tests/canonical/test_conftest.py.
  • Could not run pytest: uv is not installed, pytest is not installed for system Python, and the snapshot declares Python 3.14 while the container only has Python 3.11.2.

Design Notes

The change is scoped to test harness correctness. It aligns the canonical live-run test with the project-wide Result contract without altering runtime code.

Design / Roadmap Gate

Affected boundary is the canonical acceptance harness calling AutoHandler.handle. Current HEAD evidence shows AutoHandler.handle returns Result[MCPToolResult, MCPServerError], and Result exposes is_ok, is_err, and error as properties. The PR removes the previous local fake that masked this contract and makes the hermetic opt-in test exercise the same shape as the live path.

Directional Notes

Maintainer memory emphasized docs/runtime honesty, so I checked that the updated fixture documentation matches the implemented opt-in live-run path and the real AutoHandler.handle return type.

Test Coverage

  • Reviewed tests/canonical/test_canonical.py, tests/canonical/conftest.py, tests/canonical/test_conftest.py, src/ouroboros/core/types.py, and src/ouroboros/mcp/tools/auto_handler.py.
  • Verified syntax for changed canonical files with python3 -m py_compile tests/canonical/test_canonical.py tests/canonical/conftest.py tests/canonical/test_conftest.py.
  • Could not run pytest: uv is not installed, pytest is not installed for system Python, and the snapshot declares Python 3.14 while the container only has Python 3.11.2.

Merge Recommendation

Approve. I found no blocking runtime, contract, persistence, or test-harness regressions in the changed boundary. Test execution is limited by the local environment, but direct source inspection and syntax checks support the change.

Review-Metadata:
verdict: APPROVE
head_sha: ae11954
request_id: req_1779704772_19
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true


Reviewed by ouroboros-agent[bot] via Codex deep analysis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant