[PR #6009] [MERGED] fix(reconstruction): keep complete contradictions beside incomplete entries #7118

Closed
opened 2026-11-01 21:45:30 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6009
Author: @kaoru0822-kitauji
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/5925-contradiction-missing-evidence


📝 Commits (1)

  • cbb0a70 fix(reconstruction): keep complete contradictions beside incomplete entries

📊 Changes

5 files changed (+151 additions, -80 deletions)

View changed files

📝 src/application/ReconstructionObligationLedger.closure.test.ts (+1 -57)
📝 src/application/ReconstructionObligationLedger.fixture.ts (+57 -0)
📝 src/application/ReconstructionObligationLedgerDiagnostics.ts (+30 -22)
📝 src/application/ReconstructionObligationLedgerEvaluation.ts (+1 -1)
📝 tests/boundary/cli/reconstructionObligationLedgerCli.test.ts (+62 -0)

📄 Description

What broke: build-reconstruction-obligation-ledger dropped a complete, Evidence-backed contradiction (obligation fell to implemented, ledger open, empty contradiction report) whenever a sibling contradiction entry for the same obligation named absent Evidence.
Fix: Contradiction entries are grouped as whole entries and evaluated one by one: any complete entry yields contradiction; absent references from any entry still yield contradiction-evidence-missing.
Risk: Low. Single-entry outcomes are unchanged; only multi-entry obligations change, and only toward reporting both facts. Missing IDs repeated across entries are now listed once.

Summary

Fixes #5925. The manifest schema permits several contradiction entries per obligation, each its own claim. The ledger previously flattened them into one Evidence-ID list, so a single absent reference in a stale entry hid an established contradiction. Now an established contradiction stays reported and the missing-Evidence diagnostic stays visible.

Problem and expected behavior

Trigger: a ready packaged-process obligation with contradictions [complete, incomplete] (or the reverse order), where incomplete names ev_fff…, which is absent from the bundle. Violated invariant: missing Evidence in one entry must not erase facts established by another entry. Expected and new result: obligation contradicted, ledger failed, reports.contradicted_obligation_ids: [id], with diagnostics contradiction-evidence-missing and contradiction.

Change and scope

  • groupManifestContradictions now groups whole manifest entries (mirroring groupManifestBindings) rather than concatenating their Evidence IDs. That flattening was the representation that lost each entry's identity.
  • contradictionDiagnostics evaluates each entry: it emits contradiction when any entry's Evidence is all present, and one contradiction-evidence-missing diagnostic listing the de-duplicated absent IDs across entries.
  • I checked the producer's other multi-entry folds. Fixture entries are already evaluated per entry (some(...)). Original cases and authority references use some. Multiple bindings are intentionally ambiguous-owner. None had the same masking flaw.
  • Test-only: moved readyProcessLedgerRequest from the closure test into the shared ledger fixture so the CLI boundary test can reuse it.

Scope: not applicable (ordinary fix within an existing boundary).

Contract and boundary impact

  • Semantic owner and earliest changed stage: src/application/ReconstructionObligationLedgerDiagnostics.ts (manifest contradiction grouping and diagnostics).

  • Contribution scope fit and linked maintainer decision: ordinary bug fix; not applicable.

  • CLI and MCP/tool-catalog contract: no schema change. Ledger output for multi-entry contradictions now matches the documented conservative closure.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: the obligation ledger now keeps established contradictions; no new fields.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/cli/reconstructionObligationLedgerCli.test.ts, "keeps a complete contradiction beside an entry naming absent Evidence". It runs the public CLI with both entry orders.

  • Base reproduction: on 6d356b843 the new test failed with obligation implemented, ledger open, contradicted_obligation_ids: [], and only contradiction-evidence-missing.

  • User-visible CLI output (issue repro, executed after the fix, 5 CLI runs):

    • none: ready / verified
    • complete: failed / contradicted / [contradiction]
    • incomplete: open / implemented / [contradiction-evidence-missing]
    • complete,incomplete: failed / contradicted / [contradiction-evidence-missing, contradiction]
    • incomplete,complete: failed / contradicted / [contradiction-evidence-missing, contradiction]
  • Remaining proof gaps: the MCP adapter shares the same service. I ran the existing MCP ledger tests, but this PR adds no MCP-specific contradiction test.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- tests/boundary/cli/reconstructionObligationLedgerCli.test.ts: 4 passed (the new test failed on the base revision before the fix)
  • npm run test:local -- src/application/ReconstructionObligationLedger.closure.test.ts src/application/ReconstructionObligationLedger.authority.test.ts src/application/ReconstructionObligationCandidates.test.ts: 14 passed
  • npm run test:focused -- tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts: 4 passed
  • Issue repro (temporary test, 5 CLI variants): results as listed above
  • npm run check:fast: passed
  • npm run format:check: passed

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable (pure ledger evaluation; macOS local run).
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no target interaction.

Author readiness

Human owner: @kaoru0822-kitauji

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6009 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5925-contradiction-missing-evidence` --- ### 📝 Commits (1) - [`cbb0a70`](https://github.com/morluto/rea/commit/cbb0a705966f0ba937d3aebcc95d74e09b109f4d) fix(reconstruction): keep complete contradictions beside incomplete entries ### 📊 Changes **5 files changed** (+151 additions, -80 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ReconstructionObligationLedger.closure.test.ts` (+1 -57) 📝 `src/application/ReconstructionObligationLedger.fixture.ts` (+57 -0) 📝 `src/application/ReconstructionObligationLedgerDiagnostics.ts` (+30 -22) 📝 `src/application/ReconstructionObligationLedgerEvaluation.ts` (+1 -1) 📝 `tests/boundary/cli/reconstructionObligationLedgerCli.test.ts` (+62 -0) </details> ### 📄 Description **What broke:** `build-reconstruction-obligation-ledger` dropped a complete, Evidence-backed contradiction (obligation fell to `implemented`, ledger `open`, empty contradiction report) whenever a sibling contradiction entry for the same obligation named absent Evidence. **Fix:** Contradiction entries are grouped as whole entries and evaluated one by one: any complete entry yields `contradiction`; absent references from any entry still yield `contradiction-evidence-missing`. **Risk:** Low. Single-entry outcomes are unchanged; only multi-entry obligations change, and only toward reporting both facts. Missing IDs repeated across entries are now listed once. ## Summary Fixes #5925. The manifest schema permits several contradiction entries per obligation, each its own claim. The ledger previously flattened them into one Evidence-ID list, so a single absent reference in a stale entry hid an established contradiction. Now an established contradiction stays reported and the missing-Evidence diagnostic stays visible. ## Problem and expected behavior Trigger: a ready packaged-process obligation with contradictions `[complete, incomplete]` (or the reverse order), where `incomplete` names `ev_fff…`, which is absent from the bundle. Violated invariant: missing Evidence in one entry must not erase facts established by another entry. Expected and new result: obligation `contradicted`, ledger `failed`, `reports.contradicted_obligation_ids: [id]`, with diagnostics `contradiction-evidence-missing` and `contradiction`. ## Change and scope - `groupManifestContradictions` now groups whole manifest entries (mirroring `groupManifestBindings`) rather than concatenating their Evidence IDs. That flattening was the representation that lost each entry's identity. - `contradictionDiagnostics` evaluates each entry: it emits `contradiction` when any entry's Evidence is all present, and one `contradiction-evidence-missing` diagnostic listing the de-duplicated absent IDs across entries. - I checked the producer's other multi-entry folds. Fixture entries are already evaluated per entry (`some(...)`). Original cases and authority references use `some`. Multiple bindings are intentionally `ambiguous-owner`. None had the same masking flaw. - Test-only: moved `readyProcessLedgerRequest` from the closure test into the shared ledger fixture so the CLI boundary test can reuse it. Scope: not applicable (ordinary fix within an existing boundary). ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/application/ReconstructionObligationLedgerDiagnostics.ts` (manifest contradiction grouping and diagnostics). - Contribution scope fit and linked maintainer decision: ordinary bug fix; not applicable. - CLI and MCP/tool-catalog contract: no schema change. Ledger output for multi-entry contradictions now matches the documented conservative closure. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: the obligation ledger now keeps established contradictions; no new fields. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/reconstructionObligationLedgerCli.test.ts`, "keeps a complete contradiction beside an entry naming absent Evidence". It runs the public CLI with both entry orders. - Base reproduction: on `6d356b843` the new test failed with obligation `implemented`, ledger `open`, `contradicted_obligation_ids: []`, and only `contradiction-evidence-missing`. - User-visible CLI output (issue repro, executed after the fix, 5 CLI runs): - none: `ready` / `verified` - complete: `failed` / `contradicted` / `[contradiction]` - incomplete: `open` / `implemented` / `[contradiction-evidence-missing]` - complete,incomplete: `failed` / `contradicted` / `[contradiction-evidence-missing, contradiction]` - incomplete,complete: `failed` / `contradicted` / `[contradiction-evidence-missing, contradiction]` - Remaining proof gaps: the MCP adapter shares the same service. I ran the existing MCP ledger tests, but this PR adds no MCP-specific contradiction test. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/boundary/cli/reconstructionObligationLedgerCli.test.ts`: 4 passed (the new test failed on the base revision before the fix) - `npm run test:local -- src/application/ReconstructionObligationLedger.closure.test.ts src/application/ReconstructionObligationLedger.authority.test.ts src/application/ReconstructionObligationCandidates.test.ts`: 14 passed - `npm run test:focused -- tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts`: 4 passed - Issue repro (temporary test, 5 CLI variants): results as listed above - `npm run check:fast`: passed - `npm run format:check`: passed ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (pure ledger evaluation; macOS local run). - Package or release metadata impact: none. - Security, privacy, process, or containment review: no target interaction. ## Author readiness Human owner: @kaoru0822-kitauji - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:30 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7118
No description provided.