Skip to content

fix(auto): surface assumption_sources through envelope clients#1185

Merged
shaun0927 merged 6 commits into
Q00:mainfrom
shaun0927:fix/auto-assumption-sources-envelope
May 23, 2026
Merged

fix(auto): surface assumption_sources through envelope clients#1185
shaun0927 merged 6 commits into
Q00:mainfrom
shaun0927:fix/auto-assumption-sources-envelope

Conversation

@shaun0927
Copy link
Copy Markdown
Collaborator

Summary

Follow-up to merged #1169 after re-reviewing it against the #961 / #1157 SSOT direction.

#1169 correctly added the ledger-level AssumptionRecord and the typed AutoPipelineResult.assumption_sources field, but the L4 Auto Envelope v2 SSOT now says the lane is frozen and consumed by CLI, MCP, and future UI surfaces. Leaving the field only on the internal result object makes the lane paper-complete for MCP clients.

This PR closes that narrow envelope gap:

  • exports AssumptionRecord from ouroboros.auto alongside the other public auto primitives;
  • emits meta.assumption_sources from the MCP auto handler as [{text, source, confidence}] records, always present as [] when empty;
  • renders the records in CLI ooo auto --show-ledger output;
  • adds focused coverage for MCP meta, empty meta, public export, and CLI ledger rendering.

Why this is not over-engineering

This does not add a new policy, classifier, schema family, or control plane. It only routes the already-computed #1169 field through the same envelope channels used by the other L4 fields (defaulted_sections, evidence_backed_sections, assumption_only_sections).

Test plan

  • uv run ruff check src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py tests/unit/auto/test_surface.py tests/unit/cli/test_auto_command.py
  • uv run pytest tests/unit/auto/test_surface.py -k "ledger_provenance_breakdown or always_emits_provenance_keys or public_api_exports_assumption_record" -q
  • uv run pytest tests/unit/cli/test_auto_command.py -k "assumption_sources" -q
  • uv run pytest tests/unit/auto/test_ledger_grading_answerer.py tests/unit/auto/test_interview_pipeline.py -k "assumption_sources or assumption_class or surfaces_assumption" -q
  • uv run mypy src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py src/ouroboros/auto/ledger.py src/ouroboros/auto/pipeline.py

Refs #1169, #1157, #961.

PR Q00#1169 added the typed AutoPipelineResult field but left MCP metadata and CLI ledger rendering unable to consume the new assumption_sources contract. Surface the already-computed records through the same envelope channels as the other L4 fields and export the record type from the package API.

Constraint: Q00#1157 marks Auto Envelope v2 as frozen and consumed by CLI, MCP, and future UI surfaces.

Rejected: leaving rendering as a later follow-up | it keeps the lane paper-complete while MCP clients cannot read the new field.

Confidence: high

Scope-risk: narrow

Directive: keep assumption_sources additive and serialize it as text/source/confidence records.

Tested: uv run ruff check src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py tests/unit/auto/test_surface.py tests/unit/cli/test_auto_command.py

Tested: uv run pytest tests/unit/auto/test_surface.py -k 'ledger_provenance_breakdown or always_emits_provenance_keys or public_api_exports_assumption_record' -q

Tested: uv run pytest tests/unit/cli/test_auto_command.py -k 'assumption_sources' -q

Tested: uv run pytest tests/unit/auto/test_ledger_grading_answerer.py tests/unit/auto/test_interview_pipeline.py -k 'assumption_sources or assumption_class or surfaces_assumption' -q

Tested: uv run mypy src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py src/ouroboros/auto/ledger.py src/ouroboros/auto/pipeline.py

Not-tested: full repository test suite
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

Reviewing commit d6f19b4 for PR #1185

Review record: bab90f6c-4b89-4528-92fa-d325454a25eb

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

None.

Design Notes

Unable to assess architecture or implementation because the required source snapshot and diff could not be read.

Policy Notes

  • Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


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

The prior bot review approved the PR but reported that it could not read the source snapshot/diff, so this empty commit retriggers review without changing the already-verified code.

Constraint: user requested iterative bot-feedback incorporation until the design notes are clean.

Confidence: medium

Scope-risk: narrow

Tested: no code changes in this commit

Not-tested: no additional tests run for empty retrigger commit

Co-authored-by: OmX <omx@oh-my-codex.dev>
The full Python CI checks compare the complete MCP auto progress meta payload. Include the additive assumption_sources empty-list contract there as well, so the focused envelope change remains covered by the broad surface regression test.

Constraint: Q00#1157 L4 envelope fields should be present for MCP clients even when computed empty.

Rejected: conditional omission when empty | existing provenance fields intentionally distinguish computed-empty from absent.

Confidence: high

Scope-risk: narrow

Tested: uv run ruff check tests/unit/auto/test_surface.py src/ouroboros/mcp/tools/auto_handler.py

Tested: uv run pytest tests/unit/auto/test_surface.py -q

Tested: uv run pytest tests/unit/cli/test_auto_command.py -k 'assumption_sources' -q

Not-tested: full repository test suite locally

Co-authored-by: OmX <omx@oh-my-codex.dev>
@shaun0927
Copy link
Copy Markdown
Collaborator Author

Merge-readiness rationale after re-reviewing #1169 against #961 / #1157

I re-reviewed merged #1169 against the current #961 AgentOS sequencing SSOT and the #1157 ooo auto L4 Auto Envelope v2 SSOT.

What #1169 did

#1169 added an additive provenance surface for auto-mode assumptions:

  • AssumptionRecord(text, source, confidence) in the auto ledger;
  • SeedDraftLedger.assumption_sources() covering the three non-evidence-backed assumption-class sources: assumption, inference, and conservative_default;
  • AutoPipelineResult.assumption_sources populated from the ledger;
  • focused ledger/pipeline tests proving backwards compatibility for the legacy assumptions string tuple.

That direction is consistent with the SSOT: L4 is supposed to make the auto result envelope explicit enough that downstream CLI/MCP/UI clients can distinguish user/repo-backed facts from system-made assumptions. The implementation did not introduce a new AgentOS control plane, classifier, state machine, or external dependency, so I do not consider the original approach over-engineered.

Gap found and fixed here

The one gap was envelope reachability. #1169 added the field to AutoPipelineResult, but the current L4 SSOT now describes Auto Envelope v2 as the frozen contract consumed by CLI, MCP, and future UI surfaces. If assumption_sources stays only on the internal result object, MCP clients cannot consume the field and the lane is paper-complete rather than contract-complete.

This follow-up PR closes that narrow gap by:

  • exporting AssumptionRecord from ouroboros.auto;
  • emitting meta.assumption_sources from the MCP auto handler as stable {text, source, confidence} records, including [] when empty;
  • rendering the same records in CLI ooo auto --show-ledger;
  • adding regression coverage for MCP populated/empty meta, package export, and CLI rendering.

Bot / CI status

  • ouroboros-agent[bot] review verdict: APPROVE with no blocking findings and no non-blocking suggestions.
  • The bot design note reports a review-environment/source-snapshot read failure, not an implementation or architecture objection. I retriggered once with an empty commit and then pushed the CI expectation fix; the actionable signal remains the bot's APPROVE/no-findings result plus green CI.
  • GitHub checks are all passing: Ruff, MyPy, Bridge TypeScript, enforce-envelope, enforce-boundary, and Python 3.12/3.13/3.14 tests.

Why this is safe to merge

This PR is additive and narrow: it serializes already-computed AssumptionRecord values through existing result surfaces. It does not alter ledger conflict resolution, grading, interview closure, seed generation, run handoff, or any AgentOS tier gate. The added tests cover the new contract at the actual consumer boundaries, and the full CI matrix is green.

@shaun0927
Copy link
Copy Markdown
Collaborator Author

PR Review Summary

Verdict

Approve

Scope Reviewed

Blocking Issues

None.

Warnings

None.

Mutation-Test Thinking

  • Likely mutants that should be killed:
    • Removing meta["assumption_sources"] from _result_meta() should fail test_auto_handler_meta_exposes_ledger_provenance_breakdown and test_auto_handler_meta_always_emits_provenance_keys_when_empty.
    • Changing the serialized record keys (text, source, confidence) should fail the MCP meta assertion.
    • Dropping the public AssumptionRecord export should fail test_auto_public_api_exports_assumption_record.
    • Removing CLI rendering of result.assumption_sources should fail test_print_result_show_ledger_renders_assumption_sources.
  • Mutants current tests may not catch: cosmetic changes to the exact CLI label outside the asserted line may survive, but that is low risk because the structured MCP contract is the primary machine-consumed surface.
  • Additional tests recommended: none required for merge; a future UI/client PR can add client-specific rendering assertions once that surface exists.

Complexity / CRAP-style Risk

  • High-risk functions/modules: none. The changed logic is simple field routing/serialization.
  • Complexity increase: minimal; no new branching beyond iterating existing AssumptionRecord values.
  • Test coverage concern: acceptable. The PR covers the two important consumer boundaries: structured MCP metadata and CLI ledger display.
  • Refactoring recommendation: none. Introducing a helper serializer would be premature for one three-field record shape.

Test Quality Assessment (6/7)

  • Strong tests: MCP populated/empty meta tests verify the stable wire shape and the computed-empty contract; CLI output test verifies user-visible ledger rendering; public export test guards package API access.
  • Weak tests: CLI output is text-based and intentionally light; it does not assert Rich markup escaping behavior beyond the plain captured output.
  • Missing edge cases: none blocking. Records with unusual markup characters are escaped in the implementation via _rich_escape, but there is no dedicated malicious-markup test.
  • Mocking concerns: tests use existing fake AutoPipelineResult patterns and do not mock the serialization code under test.

Security / Operational Risk

None. The PR exposes already-computed assumption provenance already present in AutoPipelineResult; it does not introduce external IO, auth/authz paths, secrets, command execution, persistence mutation, or new runtime policy.

Looks Good

  • The implementation keeps feat(auto): additive assumption_sources provenance surface (PR-C2) #1169 additive: legacy assumptions behavior is unchanged.
  • MCP metadata now distinguishes computed-empty assumption_sources: [] from absent field, matching the existing provenance contract style.
  • CLI rendering is gated behind the existing --show-ledger path and escapes rich markup for source/text values.
  • The change aligns L4 Auto Envelope v2 with the SSOT expectation that CLI/MCP/future UI clients can consume the frozen envelope fields.

Final Recommendation

Approve. This PR is a narrow, contract-completing follow-up to #1169. It fixes the only material SSOT alignment gap found in the re-review, avoids over-engineering, and is merge-ready with green CI and focused regression coverage.

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

Reviewing commit be85a47 for PR #1185

Review record: f5d3b575-18dc-45d1-adf0-48f049c251df

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

None.

Design Notes

Unable to assess architecture or changed contracts because the review inputs could not be read in this environment.

Policy Notes

  • Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


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

shaun0927 and others added 3 commits May 22, 2026 23:25
The code audit found the PR direction remains a narrow Q00#1157 L4 envelope completion aligned with Q00#961 rather than a new AgentOS substrate. Add a no-op commit solely to request a fresh bot/source-snapshot review after the previous design note reported a read failure.

Constraint: maintainer requested iteration until bot approval and design notes are clean.

Rejected: semantic code change | local audit and focused tests found no implementation gap to patch.

Confidence: high

Scope-risk: narrow

Tested: uv run pytest tests/unit/auto/test_surface.py -k 'ledger_provenance_breakdown or always_emits_provenance_keys or public_api_exports_assumption_record or progress' -q

Tested: uv run pytest tests/unit/cli/test_auto_command.py -k 'assumption_sources' -q

Tested: uv run ruff check src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py tests/unit/auto/test_surface.py tests/unit/cli/test_auto_command.py

Tested: uv run mypy src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py src/ouroboros/auto/ledger.py src/ouroboros/auto/pipeline.py

Not-tested: fresh bot review before push

Co-authored-by: OmX <omx@oh-my-codex.dev>
The PR already uses Rich markup escaping for assumption-source text and source values. Add focused coverage for bracketed values so the user-visible ledger surface keeps the same safe rendering guarantee while remaining a narrow L4 envelope follow-up.

Constraint: Q00#1157 freezes L4 as a CLI/MCP/future-UI envelope contract, so consumer-boundary regressions should be caught at the surface.

Rejected: introduce a shared serializer | the current three-field CLI rendering remains simple and a new helper would be premature.

Confidence: high

Scope-risk: narrow

Tested: uv run pytest tests/unit/cli/test_auto_command.py -k 'assumption_sources' -q

Tested: uv run ruff check tests/unit/cli/test_auto_command.py

Not-tested: full repository test suite locally after this test-only reinforcement

Co-authored-by: OmX <omx@oh-my-codex.dev>
The prior test-only reinforcement was semantically correct but failed the CI formatting gate. Apply the formatter output so the bot/CI review can evaluate the actual envelope change instead of a style failure.

Constraint: GitHub lint runs ruff format --check across tests.

Rejected: force-push amend | preserving the already-pushed review history is safer for the active PR loop.

Confidence: high

Scope-risk: narrow

Tested: uv run ruff check tests/unit/cli/test_auto_command.py

Tested: uv run ruff format --check tests/unit/cli/test_auto_command.py

Tested: uv run pytest tests/unit/cli/test_auto_command.py -k 'assumption_sources' -q

Not-tested: full repository test suite locally

Co-authored-by: OmX <omx@oh-my-codex.dev>
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

Reviewing commit c7243ac for PR #1185

Review record: 817ef6fb-c145-44ec-bc7a-bf3002b7df27

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

None.

Design Notes

Unable to assess architecture or implementation because the provided diff and files could not be read in this environment.

Policy Notes

  • Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


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

@shaun0927
Copy link
Copy Markdown
Collaborator Author

Final merge-readiness rationale after SSOT and bot re-review

I rechecked this PR against the current #961 AgentOS roadmap SSOT and the sibling #1157 ooo auto L4 Auto Envelope v2 SSOT.

What this PR does

This is a narrow follow-up to #1169. #1169 introduced the ledger-level AssumptionRecord(text, source, confidence) and populated AutoPipelineResult.assumption_sources, but the L4 envelope is supposed to be consumable through the public auto package, MCP, CLI, and future UI clients. Keeping the value only on the internal result object left the envelope incomplete for clients.

This PR closes that specific reachability gap by:

  • exporting AssumptionRecord from ouroboros.auto;
  • emitting meta.assumption_sources from the MCP auto handler as stable {text, source, confidence} records, including [] when empty;
  • rendering the same records in ooo auto --show-ledger output;
  • covering populated MCP metadata, empty MCP metadata, public export, CLI rendering, and escaped bracketed CLI values.

SSOT / over-engineering assessment

I do not see this as over-engineering or a wrong AgentOS direction. The PR does not add a new policy layer, classifier, state machine, schema family, storage path, or control plane. It only routes an already-computed #1169 field through the existing L4 envelope surfaces, matching #1157's frozen Auto Envelope v2 lane and staying outside Track C tier-gate substrate work in #961.

The follow-up test-only commit for bracketed CLI values is also intentionally small: it protects the existing _rich_escape behavior at the user-visible ledger boundary without adding implementation abstraction.

Bot / CI status

  • Latest ouroboros-agent[bot] review on head c7243ac: APPROVE.
  • Blocking findings: none.
  • Non-blocking suggestions: none.
  • Design Notes: the bot reports an environment input-read limitation, but it does not raise an architecture, contract, or implementation objection. I cross-checked the source/diff locally and against the SSOTs above.
  • GitHub checks are green: Ruff Lint, MyPy Type Check, Bridge TypeScript, enforce-envelope, enforce-boundary, and Python 3.12 / 3.13 / 3.14 tests.

Local verification

  • uv run pytest tests/unit/auto/test_surface.py -k "ledger_provenance_breakdown or always_emits_provenance_keys or public_api_exports_assumption_record or progress" -q
  • uv run pytest tests/unit/cli/test_auto_command.py -k "assumption_sources" -q
  • uv run ruff check src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py tests/unit/auto/test_surface.py tests/unit/cli/test_auto_command.py
  • uv run ruff format --check tests/unit/cli/test_auto_command.py
  • uv run mypy src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py src/ouroboros/auto/ledger.py src/ouroboros/auto/pipeline.py

Why this is mergeable

The change is additive, consumer-boundary-focused, and covered at the relevant MCP and CLI surfaces. It preserves the legacy assumptions tuple behavior, does not alter ledger grading/interview/run handoff semantics, and completes the intended L4 envelope contract instead of expanding AgentOS substrate scope. With green CI and an APPROVE bot verdict, I consider this PR ready to merge.

@shaun0927
Copy link
Copy Markdown
Collaborator Author

PR Review Summary

Verdict

Approve

Scope Reviewed

  • PR intent: complete the Meta SSOT: ooo auto Vision — Autonomous Completion Engine #1157 L4 Auto Envelope v2 assumption_sources contract by making the feat(auto): additive assumption_sources provenance surface (PR-C2) #1169 ledger-derived assumption provenance visible through public auto exports, MCP metadata, and CLI --show-ledger output.
  • Main changed areas: src/ouroboros/auto/__init__.py, src/ouroboros/mcp/tools/auto_handler.py, src/ouroboros/cli/commands/auto.py, tests/unit/auto/test_surface.py, and tests/unit/cli/test_auto_command.py.
  • Tests reviewed: MCP populated/empty metadata tests, public API export test, CLI ledger rendering test, and the added bracketed-value rendering assertion for escaped assumption-source text/source values.
  • Checks considered: latest GitHub CI is green across Ruff, MyPy, Bridge TypeScript, enforce-envelope, enforce-boundary, and Python 3.12/3.13/3.14 tests; local targeted pytest/ruff/format/mypy checks also passed.

Blocking Issues

None.

Warnings

None.

Mutation-Test Thinking

  • Likely mutants that should be killed:
    • Removing meta["assumption_sources"] from _result_meta() should fail the MCP provenance tests, including the empty-list contract.
    • Renaming or omitting serialized record keys (text, source, confidence) should fail the populated MCP metadata assertion.
    • Dropping AssumptionRecord from ouroboros.auto.__all__ / lazy exports should fail test_auto_public_api_exports_assumption_record.
    • Removing CLI iteration over result.assumption_sources should fail test_print_result_show_ledger_renders_assumption_sources.
    • Removing Rich escaping from the CLI output path is more likely to be caught now that the test includes bracketed source/text values.
  • Mutants current tests may not catch: cosmetic changes to the exact section label or ordering outside the asserted lines may survive, but those are low-risk presentation changes; the structured MCP contract is the primary machine-consumed surface.
  • Additional tests recommended: none required for merge. Future UI/client work should add client-specific rendering assertions when that surface exists.

Complexity / CRAP-style Risk

  • High-risk functions/modules: none. The implementation is field routing/serialization over an existing AutoPipelineResult.assumption_sources tuple.
  • Complexity increase: minimal; no new decision branches, policies, persistence, classifiers, or AgentOS substrate objects.
  • Test coverage concern: acceptable. The PR covers both important consumer boundaries: MCP wire metadata and CLI ledger display.
  • Refactoring recommendation: none. A shared serializer would be premature for one simple three-field record shape.

Test Quality Assessment (6/7)

  • Strong tests: MCP tests verify populated and computed-empty metadata; public export coverage guards package API access; CLI output coverage verifies visible ledger rendering and bracketed values.
  • Weak tests: CLI tests remain plain-text capture tests and do not assert every Rich internal behavior; this is acceptable because escaping is exercised at the rendered-output boundary.
  • Missing edge cases: no blocking gaps. Unusual Unicode or very long assumption text could be covered later if UI formatting becomes more complex.
  • Mocking concerns: tests use existing fake AutoPipelineResult construction and do not mock the serialization/rendering code under review.

Security / Operational Risk

None. The PR exposes already-computed assumption provenance and does not introduce external IO, secrets handling, auth/authz changes, command execution, persistence mutation, timeout/retry behavior, or data migration.

Looks Good

Final Recommendation

Approve. This PR is a narrow, well-tested envelope-completion follow-up. It fixes the only material reachability gap left by #1169, avoids over-engineering, preserves existing behavior, and is merge-ready.

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

Reviewing commit c7243ac for PR #1185

Review record: bd6e6d4d-157d-4e85-8c6e-cf72d46e3d08

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

None.

Design Notes

Unable to assess architecture or implementation because the review inputs could not be read in this environment.

Policy Notes

  • Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


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

@shaun0927
Copy link
Copy Markdown
Collaborator Author

Fresh independent merge-readiness rationale after #961 / #1157 SSOT recheck

I independently re-reviewed this PR against the current #961 AgentOS roadmap SSOT and the sibling #1157 ooo auto SSOT, ignoring bot conclusions as requested and using the current PR head c7243ac.

What this PR does

PR #1185 is a narrow follow-up to #1169 and the frozen #1157 L4 Auto Envelope v2 lane. #1169 already introduced ledger-level AssumptionRecord(text, source, confidence) and populated AutoPipelineResult.assumption_sources; this PR makes that already-computed field reachable at the consumer boundaries:

  • public ouroboros.auto.AssumptionRecord export;
  • MCP meta.assumption_sources as stable {text, source, confidence} records, including [] when computed empty;
  • CLI ooo auto --show-ledger rendering for the same records;
  • focused tests for populated/empty MCP metadata, public export, CLI rendering, and bracketed Rich-markup-like values.

SSOT / over-engineering assessment

This remains aligned with #961 and #1157. It is not a new AgentOS substrate, not a new policy/control plane, not a new schema family, and not a classifier/runtime lane. It only completes the frozen L4 envelope reachability for a field that already exists in the auto pipeline result. That is exactly the kind of Track B / L4 contract-completion follow-up that #961 records outside Track C tier gating.

I specifically looked for over-engineering or wrong-direction implementation and did not find it. A shared serializer, new abstraction, or larger UI/client framework would have been premature here; the current direct serialization/rendering is appropriately small.

Current evidence

GitHub state:

  • Mergeability: MERGEABLE
  • Review decision: APPROVED
  • Checks: Ruff, MyPy, Bridge TypeScript, enforce-envelope, enforce-boundary, Python 3.12, Python 3.13, and Python 3.14 are all green.
  • Latest ouroboros-agent[bot] review is APPROVED with no blocking findings and no non-blocking suggestions. The design note is an environment/snapshot-read limitation, not an implementation objection.

Fresh local validation from an isolated worktree:

  • uv run pytest tests/unit/auto/test_surface.py -k "ledger_provenance_breakdown or always_emits_provenance_keys or public_api_exports_assumption_record or progress" -q → 5 passed
  • uv run pytest tests/unit/cli/test_auto_command.py -k "assumption_sources" -q → 1 passed
  • uv run ruff check src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py tests/unit/auto/test_surface.py tests/unit/cli/test_auto_command.py → passed
  • uv run mypy src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py src/ouroboros/auto/ledger.py src/ouroboros/auto/pipeline.py → passed

Recommendation

Merge. The PR is additive, contract-completing, and covered at the relevant MCP/CLI/package boundaries. It improves the L4 envelope from paper-complete to consumer-reachable without expanding AgentOS scope or weakening any runtime/ledger behavior.

@shaun0927
Copy link
Copy Markdown
Collaborator Author

PR Review Summary

Verdict

Approve

Scope Reviewed

  • PR intent: complete the Meta SSOT: ooo auto Vision — Autonomous Completion Engine #1157 L4 Auto Envelope v2 assumption_sources surface by making the feat(auto): additive assumption_sources provenance surface (PR-C2) #1169 ledger-derived provenance reachable through public auto exports, MCP metadata, and CLI --show-ledger output.
  • Main changed areas: src/ouroboros/auto/__init__.py, src/ouroboros/mcp/tools/auto_handler.py, src/ouroboros/cli/commands/auto.py, tests/unit/auto/test_surface.py, and tests/unit/cli/test_auto_command.py.
  • Tests reviewed: MCP populated/empty metadata tests, public API export test, CLI ledger rendering test, and bracketed-value CLI rendering coverage.
  • Checks considered: fresh local targeted pytest/ruff/mypy from an isolated worktree plus current GitHub Ruff, MyPy, Bridge TypeScript, enforce-envelope, enforce-boundary, and Python 3.12/3.13/3.14 checks, all passing.

Blocking Issues

None.

Warnings

None.

Mutation-Test Thinking

  • Likely mutants that should be killed:
    • Removing meta["assumption_sources"] from _result_meta() should fail the MCP provenance tests, including the empty-list contract.
    • Renaming or omitting serialized record keys (text, source, confidence) should fail the populated MCP metadata assertion.
    • Dropping AssumptionRecord from ouroboros.auto.__all__ / _EXPORTS should fail test_auto_public_api_exports_assumption_record.
    • Removing CLI iteration over result.assumption_sources should fail test_print_result_show_ledger_renders_assumption_sources.
    • Removing Rich escaping from source/text rendering is more likely to be caught by the bracketed-value assertion.
  • Mutants current tests may not catch: cosmetic changes to unasserted surrounding CLI text could survive, but the structured MCP contract is the primary machine-consumed surface and is covered.
  • Additional tests recommended: none required for merge. Future UI/client work should add client-specific rendering tests when that surface exists.

Complexity / CRAP-style Risk

  • High-risk functions/modules: none. This is simple field routing/serialization of an existing AutoPipelineResult.assumption_sources tuple.
  • Complexity increase: minimal; no new policy branch, persistence path, classifier, workflow IR, or AgentOS substrate surface.
  • Test coverage concern: acceptable. The PR covers the important public/package, MCP, and CLI consumer boundaries.
  • Refactoring recommendation: none. A shared serializer would be premature for one simple three-field record shape.

Test Quality Assessment (6/7)

  • Strong tests: MCP tests cover populated and computed-empty metadata; public export coverage guards package API access; CLI output coverage verifies visible ledger rendering and bracketed values.
  • Weak tests: CLI tests are plain rendered-output assertions, not exhaustive Rich internals; this is acceptable for the current user-visible boundary.
  • Missing edge cases: no blocking gaps. Very long or unusual Unicode assumption text can be deferred until a UI/client surface needs stricter formatting behavior.
  • Mocking concerns: tests use existing fake AutoPipelineResult construction and do not mock the serialization/rendering logic under review.

Security / Operational Risk

None. The PR exposes already-computed assumption provenance and does not introduce external IO, secrets handling, auth/authz changes, command execution, persistence mutation, timeout/retry behavior, or data migration.

Looks Good

Final Recommendation

Approve. This PR is a narrow, well-tested envelope-completion follow-up. It fixes the remaining consumer reachability gap for assumption_sources, avoids over-engineering, preserves existing behavior, and is merge-ready.

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

Reviewing commit c7243ac for PR #1185

Review record: 2e55f6f4-f669-4221-bcf8-75ad69611d5a

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

None.

Design Notes

Not assessed. The local execution environment prevented reading the PR snapshot and review artifacts.

Policy Notes

  • Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


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

@shaun0927 shaun0927 merged commit 3ed79e5 into Q00:main May 23, 2026
8 checks passed
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

PR #1185
Branch: fix/auto-assumption-sources-envelope | 5 files, +86/-1 | CI: Bridge TypeScript pass 15s https://github.com/Q00/ouroboros/actions/runs/26293951648/job/77401333900
Scope: architecture-level
HEAD checked: c7243ac9263e150ab79bac1dff659714dae2834f

What Improved

  • AssumptionRecord is now exported from ouroboros.auto.
  • MCP auto metadata now always includes assumption_sources, using [] when empty.
  • CLI ooo auto --show-ledger renders assumption source, confidence, and text.
  • Focused tests cover MCP populated/empty metadata, public export, CLI rendering, Rich escaping, and ledger/pipeline provenance behavior.

Issue #N/A Requirements

Requirement Status
Export AssumptionRecord from ouroboros.auto Satisfied
Emit MCP meta.assumption_sources as {text, source, confidence} records Satisfied
Always emit empty MCP assumption_sources as [] Satisfied
Render assumption-source records in CLI --show-ledger output Satisfied
Add meaningful focused coverage for new public, MCP, and CLI surfaces Satisfied

Prior Findings Status

Prior Finding Status
Prior review context MODIFIED — Prior CLI Rich-markup escaping concern is withdrawn: current HEAD escapes record.source and record.text before printing, and tests/unit/cli/test_auto_command.py:553 covers bracketed assumption-source output. No prior concern is maintained as a current-HEAD blocker.

Blockers

# File:Line Severity Confidence Finding

Follow-ups

# File:Line Priority Confidence Suggestion
None.

Test Coverage

  • Verified locally: uv run pytest tests/unit/auto/test_surface.py -k "ledger_provenance_breakdown or always_emits_provenance_keys or public_api_exports_assumption_record" -q
  • Verified locally: uv run pytest tests/unit/cli/test_auto_command.py -k "assumption_sources" -q
  • Verified locally: uv run pytest tests/unit/auto/test_ledger_grading_answerer.py tests/unit/auto/test_interview_pipeline.py -k "assumption_sources or assumption_class or surfaces_assumption" -q
  • Verified locally: uv run ruff check src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py tests/unit/auto/test_surface.py tests/unit/cli/test_auto_command.py
  • Verified locally: uv run mypy src/ouroboros/auto/__init__.py src/ouroboros/mcp/tools/auto_handler.py src/ouroboros/cli/commands/auto.py src/ouroboros/auto/ledger.py src/ouroboros/auto/pipeline.py

Design / Roadmap Gate

Affected-boundary audit covered the ledger provenance producer, typed AutoPipelineResult, MCP metadata projection, CLI ledger rendering, and public package export. The change is additive and does not alter state-machine transitions, persistence schema, replay semantics, or existing assumptions consumers. The MCP contract now exposes the already-computed assumption-source lane consistently with the existing ledger provenance fields.

Merge Recommendation

Post-merge audit found no current-HEAD blocker and no rollback recommendation.

Review-Metadata:
verdict: APPROVE
github_event: COMMENT
review_kind: post_merge_audit
merge_eligible: false
head_sha: c7243ac
source_read_ok: true
diff_read_ok: true
blocking_count: 0

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