Skip to content

Builder plans re-enrichment; succeeds schema errors are clean findings - #145

Merged
MaxGhenis merged 1 commit into
codex/thesis-ledger-factsfrom
fix/reenrichment-planning
Aug 7, 2026
Merged

Builder plans re-enrichment; succeeds schema errors are clean findings#145
MaxGhenis merged 1 commit into
codex/thesis-ledger-factsfrom
fix/reenrichment-planning

Conversation

@MaxGhenis

Copy link
Copy Markdown
Contributor

Closes the two accepted follow-ups from the #128 merge disposition (fourteenth adversarial review, findings 1–2):

  • Re-enrichment planning [MEDIUM]: the registry grammar has accepted succeeds on a retired successor since the twelfth review, but the builder aborted on UUID ownership before reaching enrichment planning. The enrichment pair-planning is extracted into one helper used by both the first-enrichment and the returning-successor paths; the reviewer's full lifecycle (placeholder → enrich → withdraw → reclaim → re-enrich) now builds, stages, and reloads end to end (test_builder_plans_reenrichment_of_retired_successor).
  • Clean schema findings [LOW]: succeeds dimension types validate before any key derivation, so "geography": "not-an-object" is a registry-invalid finding instead of an AttributeError (test_malformed_succeeds_geography_is_a_schema_finding).

Artifact bytes unchanged; 153 tests, ruff, doctest, --check green. Machine gates (catalog↔registry agreement, append-only vs base, anchor) apply as usual.

🤖 Generated with Claude Code

…rrors clean

Both accepted findings from the PR #128 merge disposition:

1. [MEDIUM] A retired successor returning on the current placeholder
   lineage is planned as a re-enrichment (retire + succeeds pair via
   the extracted _plan_enrichment helper) instead of dead-ending on
   UUID ownership before enrichment planning — the
   placeholder -> enrich -> withdraw -> reclaim -> re-enrich lifecycle
   now completes in the builder as it already did in the grammar.
2. [LOW] succeeds dimension types are validated before any key
   derivation, so a non-object succeeds.geography is a registry-invalid
   finding rather than a raw AttributeError.

153 tests; ruff, doctest, --check green; artifact bytes unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Adversarial review round 1 — verdict: MERGE (risk LOW)

Reviewer: gpt-5.6-sol (ultra reasoning), read-only worktree, local diff vs origin/codex/thesis-ledger-facts (base 7777818, head 35afef7 — this PR's head). REPORT verbatim:

REPORT
verdict: MERGE
risk: LOW
findings:
1. LOW — `_plan_enrichment` uses historical UUID ownership plus key liveness rather than verifying the live binding owns `row_uuid`. Invalid plans are rejected before writes, but a direct binding check would fail earlier.
validation-record:
- pytest: PASS — 153 tests
- ruff: PASS
- doctest: PASS
- catalog `--check`: PASS — 209 series
- adversarial lifecycle and guard repros: PASS
- base regressions: reproduced as expected
- generated catalog and registry bytes: unchanged
- GitHub CI: unavailable due blocked network access

Reviewer's independent verification also confirmed: the two new regression tests fail on base for the claimed reasons and pass on head; re-enrichment lifecycle stages correctly (catalog UUID/status, retire+succeeds continuity, staged registry reload, successor binding, liveness); guard attacks blocked (live canon, impostor placeholders, competing successors); artifact bytes identical across committed / base-generated / head-generated catalog+registry.

Disposition

Finding 1 (LOW) — accepted as a real defense-in-depth improvement, deferred rather than folded into this PR: _plan_enrichment is reachable only from the resolve_uuid branch where prior_uuid != binding and not registry.is_live(canon_key), and every plan it emits is re-validated by the staging replay before any write, so the earlier-failure benefit is diagnostic, not correctness. Changing the reviewed head for a LOW would invalidate the MERGE verdict for no behavioral difference. Logged in the catalog curation backlog alongside the reviewer's noted residual test gap (stale catalog UUID after the same placeholder key was superseded).

Merging on MERGE verdict + green CI per the standing gate.

@MaxGhenis
MaxGhenis merged commit 990fc13 into codex/thesis-ledger-facts Aug 7, 2026
8 checks passed
@MaxGhenis
MaxGhenis deleted the fix/reenrichment-planning branch August 7, 2026 18:42
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