Repository navigation
feat(evidence): add versioned per-record carriers - #112
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e59a32ceed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| report.EvidenceCarrierProjectionDebt, err = store.ListEvidenceCarrierProjectionDebt(ctx) | ||
| if err != nil { | ||
| return report, fmt.Errorf("scan evidence carrier projection debt: %w", err) |
There was a problem hiding this comment.
On an existing project whose local SQLite ledger is still at schema 58, runCheck opens the DB directly and does not go through the project-ledger readiness check, so this new unconditional query of evidence_carrier_projection_debt (created only by migration 59) fails with no such table before haft check can print its normal report or a migration/repair hint. Treat a missing debt table as an empty/diagnostic-unavailable inventory, or require the current schema before building the report.
Useful? React with 👍 / 👎.
| if evidenceFormalityBridge(formalityScale) != nil && item.FormalityBridge == nil { | ||
| return EvidenceCarrier{}, fmt.Errorf("evidence %s formality_bridge is required for scale %s", item.ID, formalityScale.ScaleID) |
There was a problem hiding this comment.
When a synced carrier declares a legacy or unversioned formality_scale, this only checks that formality_bridge is non-nil; ImportEvidenceCarrier then stores that bridge and WLNK reads item.FormalityBridge.Loss, so a hand-edited git carrier can set an arbitrary bridge/loss and silently change downstream bridge-loss diagnostics while still passing validation. Compare the supplied bridge to evidenceFormalityBridge(formalityScale) (and reject extra bridges when the canonical bridge is nil) before importing.
Useful? React with 👍 / 👎.
| if !unchanged { | ||
| item := carrier.Evidence | ||
| if err := s.addEvidenceItemWithExec(ctx, tx, &item, carrier.ArtifactRef); err != nil { | ||
| return "", fmt.Errorf("import evidence %s: %w", carrier.Evidence.ID, err) |
There was a problem hiding this comment.
For a pulled carrier, this goes straight to the low-level insert path, bypassing the validation in AttachEvidence that rejects claim_refs on non-decision parents and resolves/validates them against a decision's structured claims. A hand-edited .haft/evidence/*.md can therefore sync claim bindings onto a Note or reference non-existent decision claims, leaving SQLite in a state the normal evidence API would never create; import should load the parent and run the same binding validation before inserting.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
What
Why
Evidence attachments were SQLite-only, so git collaboration could not transfer them and concurrent attachments had no independent carrier. This implements the bound choice in dec-20260811-2e06aae9 without changing existing CLI or MCP request fields.
Root cause and failure semantics
The semantic EvidenceRecord and the git-facing representation had no explicit projection boundary. The new domain path keeps SQLite as the runtime projection, writes carriers atomically, records durable debt after post-commit publication failure, rejects missing or changed parents, and preserves divergent git and SQLite states for explicit reconciliation.
Compatibility and impact
Checks
Formal exact-SHA CI/full-race, installed-runtime P14, tag validation, and GitHub Release publication remain separate gates.
Fixes #100