[PR #3524] [MERGED] fix(reconstruction): merge repeated observations into one generated obligation #5801

Closed
opened 2026-10-31 01:09:33 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: fix/ledger-repeated-capture-obligation


📝 Commits (1)

  • 3356cf3 fix(reconstruction): merge repeated observations into one generated obligation

📊 Changes

3 files changed (+70 additions, -4 deletions)

View changed files

📝 src/application/ReconstructionObligationCandidates.ts (+16 -1)
📝 src/application/ReconstructionObligationLedger.closure.test.ts (+43 -0)
📝 src/application/ReconstructionObligationLedger.fixture.ts (+11 -3)

📄 Description

Summary

Two ordinary capture-process records of the same executable and scenario made build-reconstruction-obligation-ledger report duplicate-definition on the generated process obligation. That diagnostic is a closure blocker meant for competing definitions. Fixes #3375.

Problem and expected behavior

addCandidate merges candidates that share an obligation ID by adding their definitionCount. A generated ID is a digest of its derivation key (process:<executable_sha256>:<full_scenario_sha256>, or application: / semantic: node keys). Two generated candidates with the same ID are therefore the same claim seen by several Evidence records, not two definitions. The merge also kept only the first record's unavailable_authority and required_next_evidence, so a later capture's own residual unknowns were dropped.

Expected (from the issue and docs/reconstruction-obligation-ledgers.md): repeated observations add Evidence to one generated obligation, and real duplicate reviewed definitions still block closure.

Change and scope

In src/application/ReconstructionObligationCandidates.ts addCandidate:

  • Merging two generated candidates keeps one definition. Any merge that involves a reviewed definition still adds the counts, so duplicate-definition still fires.
  • unavailable_authority and required_next_evidence are unioned across every merged observation, sorted and deduplicated like the other merged lists.
  • Not changed: dependency_obligation_ids and residual_unknown_ids (always empty for generated candidates), the evaluation rules, and the schema.

This also covers application and semantic graph candidates, where the same node seen in two graph records hit the same path.

Contribution scope: an ordinary fix within existing boundaries, so no maintainer scope decision is needed.

Contract and boundary impact

  • Semantic owner and earliest changed stage: candidate derivation (addCandidate), before ledger evaluation.

  • Contribution scope fit and linked maintainer decision: not applicable (ordinary fix).

  • CLI and MCP/tool-catalog contract: no schema change. Fewer spurious duplicate-definition diagnostics, and more complete unavailable_authority / required_next_evidence for merged generated obligations.

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

  • Evidence, artifact, provenance, or reconstruction contract: every observation's authority references, cases and residual unknowns are kept.

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

  • Generated metadata: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated (src/application/ReconstructionObligationLedger.closure.test.ts):
    • merges repeated captures of one process scenario into one definition: two distinct process records give one obligation with the same ID as the single-record ledger, no duplicate-definition, both authority references, and the second capture's network: Second run only. unavailable authority. It fails on base (1 failed, 12 passed).
    • still reports a reviewed definition that reuses a generated ID: a guard for the remaining duplicate case. No test covered duplicate-definition before.
    • processEvidence() in the fixture now accepts capture overrides.
  • Real-app reproduction (the #3375 script): two real capture-process runs of node -e 'console.log("sample")' on macOS arm64, then the built CLI ledger:
request main diagnostics on obl_e087d392… this PR
single capture missing-original-case, missing-owner, 5× unavailable-authority unchanged
repeated captures duplicate-definition + the same same as single; 2 authority refs, 6 observed cases

Ledger build time is unchanged: about 330 ms per CLI call, before and after.

  • Remaining proof gaps: Linux process capture was not run. The change is in platform-independent candidate merging.

  • 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:local -- src/application/ReconstructionObligationLedger.closure.test.ts src/application/ReconstructionObligationLedger.authority.test.ts src/application/ReconstructionObligationCandidates.test.ts: 13 passed
  • npm run test:focused -- tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts tests/boundary/cli/reconstructionObligationLedgerCli.test.ts src/domain/reconstructionObligationLedgerSchemas.test.ts src/domain/reconstructionReadiness.test.ts: 26 passed
  • npm run check:fast: passed
  • Real capture-process ×2 plus the built CLI ledger: as listed above

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: real process capture on macOS 26.7.1 arm64.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no change. Analysis is pure.

Author readiness

Human owner:

  • 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/3524 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/ledger-repeated-capture-obligation` --- ### 📝 Commits (1) - [`3356cf3`](https://github.com/morluto/rea/commit/3356cf333f44fcfe3c963afc144553b97184bd42) fix(reconstruction): merge repeated observations into one generated obligation ### 📊 Changes **3 files changed** (+70 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ReconstructionObligationCandidates.ts` (+16 -1) 📝 `src/application/ReconstructionObligationLedger.closure.test.ts` (+43 -0) 📝 `src/application/ReconstructionObligationLedger.fixture.ts` (+11 -3) </details> ### 📄 Description ## Summary Two ordinary `capture-process` records of the same executable and scenario made `build-reconstruction-obligation-ledger` report `duplicate-definition` on the generated process obligation. That diagnostic is a closure blocker meant for competing definitions. Fixes #3375. ## Problem and expected behavior `addCandidate` merges candidates that share an obligation ID by adding their `definitionCount`. A generated ID is a digest of its derivation key (`process:<executable_sha256>:<full_scenario_sha256>`, or `application:` / `semantic:` node keys). Two generated candidates with the same ID are therefore the same claim seen by several Evidence records, not two definitions. The merge also kept only the first record's `unavailable_authority` and `required_next_evidence`, so a later capture's own residual unknowns were dropped. Expected (from the issue and `docs/reconstruction-obligation-ledgers.md`): repeated observations add Evidence to one generated obligation, and real duplicate reviewed definitions still block closure. ## Change and scope In `src/application/ReconstructionObligationCandidates.ts` `addCandidate`: - Merging two `generated` candidates keeps one definition. Any merge that involves a reviewed definition still adds the counts, so `duplicate-definition` still fires. - `unavailable_authority` and `required_next_evidence` are unioned across every merged observation, sorted and deduplicated like the other merged lists. - Not changed: `dependency_obligation_ids` and `residual_unknown_ids` (always empty for generated candidates), the evaluation rules, and the schema. This also covers application and semantic graph candidates, where the same node seen in two graph records hit the same path. Contribution scope: an ordinary fix within existing boundaries, so no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: candidate derivation (`addCandidate`), before ledger evaluation. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix). - CLI and MCP/tool-catalog contract: no schema change. Fewer spurious `duplicate-definition` diagnostics, and more complete `unavailable_authority` / `required_next_evidence` for merged generated obligations. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: every observation's authority references, cases and residual unknowns are kept. - Target selection, authority, upstream behavior, and cleanup details: unchanged. - Generated metadata: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated (`src/application/ReconstructionObligationLedger.closure.test.ts`): - `merges repeated captures of one process scenario into one definition`: two distinct process records give one obligation with the same ID as the single-record ledger, no `duplicate-definition`, both authority references, and the second capture's `network: Second run only.` unavailable authority. It fails on base (1 failed, 12 passed). - `still reports a reviewed definition that reuses a generated ID`: a guard for the remaining duplicate case. No test covered `duplicate-definition` before. - `processEvidence()` in the fixture now accepts capture overrides. - Real-app reproduction (the #3375 script): two real `capture-process` runs of `node -e 'console.log("sample")'` on macOS arm64, then the built CLI ledger: | request | main diagnostics on `obl_e087d392…` | this PR | |---|---|---| | single capture | missing-original-case, missing-owner, 5× unavailable-authority | unchanged | | repeated captures | **duplicate-definition** + the same | same as single; 2 authority refs, 6 observed cases | Ledger build time is unchanged: about 330 ms per CLI call, before and after. - Remaining proof gaps: Linux process capture was not run. The change is in platform-independent candidate merging. - [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:local -- src/application/ReconstructionObligationLedger.closure.test.ts src/application/ReconstructionObligationLedger.authority.test.ts src/application/ReconstructionObligationCandidates.test.ts`: 13 passed - `npm run test:focused -- tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts tests/boundary/cli/reconstructionObligationLedgerCli.test.ts src/domain/reconstructionObligationLedgerSchemas.test.ts src/domain/reconstructionReadiness.test.ts`: 26 passed - `npm run check:fast`: passed - Real `capture-process` ×2 plus the built CLI ledger: as listed above ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: real process capture on macOS 26.7.1 arm64. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no change. Analysis is pure. ## Author readiness Human owner: <!-- @GitHub-handle --> - [ ] 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](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-10-31 01:09:33 +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#5801
No description provided.