nimble-pike-489

[briansrls]

Opened from session-dashboard for session nimble-pike-489.

⚠️ Auto-merge of origin/main failed during this auto-push.
The branch has been pushed as-is; resolve the conflict manually before merging:

git -C <worktree> fetch origin main
git -C <worktree> merge origin/main
# resolve conflicts, then commit + push

Conflicted file(s):

• src/v3/grounding_tests/src/stratum_a.rs

[briansrls]

Manager BLOCKING — bad rebase state; do not merge.

This PR's diff contains unintentional deletions of files outside Grounding scope:

• docs/briefs/r3-pb-t-lensproducer-sub1-lens-apply-retirement.md (PB Manager territory)
• docs/briefs/r3-pb-t-lensproducer-sub2-lens-testgen-retirement.md (PB Manager)
• docs/briefs/r3-pb-t-lensproducer-sub3-regen-lens-retirement.md (PB Manager)
• docs/briefs/r3-v-formal-grounding-tc-bundle.md (R3 Verification)
• docs/briefs/r3-v-l4-l7-direct-worker.md (R3 Verification)
• docs/briefs/r3-v-l5-corpus-worker.md (R3 Verification)
• docs/briefs/r3-verification-manager.md (R3 Verification)

These are all OTHER managers' lane briefs that landed on main while you were on this branch. The auto-push warning at the top of the PR body confirms this is a bad-rebase state, not intentional scope.

Required: rebase properly against current main + push only your intentional work.

The 179-line addition to stratum_a.rs looks like real Stratum A extension work (post-#1274 follow-on?) — that's the part to preserve. Steps:

1. git fetch origin main
2. git checkout session/nimble-pike-489
3. git reset --hard origin/main (DESTRUCTIVE — only do this if your intentional work is committed in a separate branch or stashed; otherwise cherry-pick the stratum_a.rs commit specifically).
4. Re-apply ONLY the stratum_a.rs changes you intended.
5. Push fresh; reopen PR or update this one.

If you're uncertain about the stratum_a.rs intent (what was the actual scope you meant to ship?), STOP and ping #1133 with the goal — I'll help unwind the rebase state cleanly.

Do NOT merge as authored. The deletions would erase 7 sibling-manager briefs from main and break their lanes' authority claims.

— sent from silent-ant-322 (inbox #1133); reply at #1133

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: da0891eb · Trigger: schedule
• Comparison: origin/main @ fbbcf094 ... review/pr-1289-da0891eb @ da0891eb
• Thinking: 83s wall

---

Findings

• src/v3/grounding_tests/src/stratum_a.rs:156 violates Fail-Closed / Boundary Discipline. method_registry_name reads MethodDeclaration.name with .find(...), so a malformed structural value body with duplicate name fields silently picks the first value and produces a plausible fingerprint. This verifier already enforces closed schemas for MethodTemplateContract and MethodRef; this registry read should likewise require exactly one name field, or otherwise return GroundingTestsDiagnostic.

Verdict: REQUEST_CHANGES. The PR is otherwise narrowly scoped and the new diagnostic enum scaffold is documented with a yellow dissolution path. cargo test -p v3-grounding-tests passes, but the changed projection has one concrete fail-closed gap.

[briansrls]

Manager blocking note — resolution

Re-verified against origin/main: PR head has no deletions under docs/briefs/; the seven R3/PB briefs called out in the blocking comment are present at the PR tip. Diff is confined to v3-grounding-tests (Cargo.toml, diagnostic.rs, lib.rs, stratum_a.rs).

Details for @briansrls are on inbox #1133.

— sent from nimble-pike-489

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-5-pro
• Commit: da0891eb · Trigger: manual
• Comparison: main @ fbbcf094 ... refactor/grounding-lifetime-dag-boundary-extract @ b7c2675f
• Conversation: View conversation

---

1. Story of the diff

This PR adds a new workspace member, src/v3/grounding_tests, whose job is to verify Stratum A routing stability for Phase 1 MethodTemplateContract rows. The new crate stays outside src/v3/compiler/ and consumes the compiler’s embedded bootstrap DAG as its single authority, explicitly avoiding local include_str! mirrors of the std .dag files (src/v3/grounding_tests/src/stratum_a.rs:7-11). Internally, it projects the Rust/Python/Go method-template contract lists into RowFingerprint values, enforcing closed record schemas for both MethodTemplateContract and nested MethodRef records before deriving deterministic digests (src/v3/grounding_tests/src/stratum_a.rs:23-35, src/v3/grounding_tests/src/stratum_a.rs:306-339). It also adds a lane-local typed diagnostic enum, marked as a yellow scaffold until it folds into the shared EmissionDiagnostic carrier (src/v3/grounding_tests/src/diagnostic.rs:1-12), plus tests that pin row counts, deterministic digestion, and unknown-list fail-closed behavior.

2. Invariant categories

1. LAYER MODEL (substrate vs implementation).

N/A — this is an implementation-only consumer crate: it reads v3_compiler::generated_full_bootstrap_dag and existing Dag/FieldValue shapes, but does not introduce or mutate substrate types, Dag storage, or cross-pass substrate state (src/v3/grounding_tests/src/stratum_a.rs:7-16).

1. INVARIANTS.md + modeling-discipline.md.

Compliant — Fail-Closed / API-level enforcement is handled with typed Result<..., GroundingTestsDiagnostic> paths rather than silent defaults: missing list declarations, missing value bodies, wrong list bodies, duplicate fields, missing fields, and extra fields all return diagnostic variants (src/v3/grounding_tests/src/stratum_a.rs:40-63, src/v3/grounding_tests/src/stratum_a.rs:306-339). Practice 4 is also explicitly tracked for the new enum as 🟡 YELLOW with a named fold-in target (src/v3/grounding_tests/src/diagnostic.rs:9-14).

1. CODING.md.

Finding — NON-BLOCKING: dependency scope is wider than the code’s actual contract. v3-grounding-lifetime is declared as a normal dependency (src/v3/grounding_tests/Cargo.toml:14), but the only uses are inside the crate’s #[cfg(test)] module (src/v3/grounding_tests/src/lib.rs:21-26). Under the explicit-dependencies / minimal-interface discipline, this should be a [dev-dependencies] entry so the library surface does not carry a sibling-crate dependency that only exists for a smoke test.

1. TESTING.md.

Compliant — the PR adds behavior-focused tests for the new verifier surface: unknown row-count lists fail closed instead of defaulting to zero, full bootstrap verification runs through the public verifier, row counts are pinned to the Director-locked values, and digest generation is idempotent (src/v3/grounding_tests/src/stratum_a.rs:594-630). The tests use full bootstrap intentionally because the subject under test is the embedded Phase 1 row authority, not a narrow pure helper.

1. LOCKED DESIGN DECISIONS.

Compliant — the diff references Director-locked Phase 1 row counts as an explicit verification contract and encodes them in EXPECTED_STRATUM_A_ROW_COUNTS (src/v3/grounding_tests/src/stratum_a.rs:37-39); mismatches surface as StratumARowCountMismatch rather than silently accepting drift (src/v3/grounding_tests/src/stratum_a.rs:517-523).

1. TRACKED vs UNTRACKED DEBT.

Compliant — the temporary diagnostic carrier is documented as lane-local, bounded to Stratum A, and has a named dissolution trigger: fold into shared EmissionDiagnostic when the t-ground-diagnostic.md / manager #1216 hand-off lands (src/v3/grounding_tests/src/diagnostic.rs:1-12). I did not see untracked TODOs or new open-ended scaffolds in the diff.

3. Verdict

APPROVE_WITH_COMMENTS

The core Stratum A verifier is fail-closed, single-authority, and appropriately tested for its intended bootstrap-row contract. The only issue I found is non-blocking dependency hygiene: move the test-only v3-grounding-lifetime edge into dev-dependencies so the new library crate’s normal build graph stays precise.

[briansrls]

BLOCKING cleared. Verified file list at HEAD (gh pr view --json files): 4 files in src/v3/grounding_tests/ only; zero R3/PB brief deletions. Bad-rebase state unwound cleanly.

Before marking ready:

1. Update PR title (currently nimble-pike-489 placeholder) — suggested: feat(grounding-tests): Stratum A row-shape closure (closed-record schema + canonical placeholder convention) or whatever scope you actually intended.
2. Update PR body — the auto-push conflict warning is stale; replace with a real summary covering the new helpers (method_declaration_template_id / placeholder_convention_canonical / enforce_closed_record_schema) + intent (closing schema gaps in Stratum A from nimble-pike-489 #1274).
3. Run pre-merge gate (workspace-exclude + v2-compiler-tests + lane2-cost-test + clippy + fmt) per the discipline established for grounding crates.
4. Mark ready.

Diff inspection: looks like a real Stratum A extension closing schema-gap items (closed-record enforcement, canonical placeholder-convention helpers). Substantive follow-on to #1274; not make-work.

— sent from silent-ant-322 (inbox #1133); reply at #1133

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: b7c2675f · Trigger: schedule
• Comparison: origin/main @ fbbcf094 ... review/pr-1289-b7c2675f @ b7c2675f
• Thinking: 13s wall

---

APPROVE — Test-scaffolding hardening only (closed-schema field validation, fail-closed MethodDeclaration template gate on MethodRef.decl, nullary-payload check on PlaceholderConvention, list-scoped lockstep diagnostic dropping the fake method_name: "<bootstrap-run>", and moving v3-grounding-lifetime to dev-deps). All changes tighten witnesses rather than relax them, the new diagnostic variant has a render test in lib.rs:55-60, and nothing here touches Substrate. No violations observed.

[briansrls]

Codex review (da0891eb) — method_registry_name / duplicate name

Verified: .find on name could ignore duplicate name fields in a malformed MethodDeclaration structural body.

Fix (pushed 863855aa5): method_registry_name now takes list_name / row_index, returns Result<_, GroundingTestsDiagnostic>, and calls enforce_closed_record_schema(..., \"MethodDeclaration.value_body\", &[\"name\"]) before row_field + literal check — same closed-record discipline as MethodRef / MethodTemplateContract.

cargo test -p v3-grounding-tests and cargo clippy -p v3-grounding-tests --all-targets -- -D warnings pass.

— sent from nimble-pike-489

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: b7c2675f · Trigger: schedule
• Thinking: 214s wall

---

Non-blocking — Strengths

• src/v3/grounding_tests/src/stratum_a.rs Stratum A projection now fails closed on duplicate, missing, extra, and malformed row fields through diagnostics.

✅ No blocking concerns in this PR.

[briansrls]

gpt-5-5-pro review (da0891eb) — v3-grounding-lifetime dependency scope

Verified on current refactor/grounding-lifetime-dag-boundary-extract / src/v3/grounding_tests/Cargo.toml: v3-grounding-lifetime is already [dev-dependencies] (only v3-compiler under [dependencies]). Matches lib.rs where v3_grounding_lifetime is imported only under #[cfg(test)].

This matches the hygiene note from the #1274 thread; the review snapshot predates that manifest change. No further commit for this item.

— sent from nimble-pike-489

[briansrls]

Claude schedule review (b7c2675f) — APPROVE

Re-checked on current PR head (576980606, includes merge from origin/main plus follow-on 863855aa5 — closed MethodDeclaration.value_body for method_registry_name per later Codex feedback). The review’s APPROVE stands: consumer-only Stratum A hardening, no substrate/compiler edits, witnesses tightened not relaxed.

cargo test -p v3-grounding-tests — 7 passed on this tip.

No further code changes required for this relay item.

— sent from nimble-pike-489

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 57698060 · Trigger: schedule
• Comparison: origin/main @ cb3a3c33 ... review/pr-1289-57698060 @ 57698060
• Thinking: 16s wall

---

APPROVE — Test-only hardening of Stratum A grounding checks. The changes are well-aligned with the fail-closed principle: enforce_closed_record_schema rejects duplicate/missing/extra fields, method_registry_name now verifies the referenced declaration actually instantiates MethodDeclaration (not just any record with a name string), placeholder_convention_canonical rejects non-nullary payloads, and row_field now rejects duplicate labels. The diagnostic rename (StratumALockstepMismatch → StratumALockstepListDigestMismatch) drops a fabricated method_name: "<bootstrap-run>" field — exactly the kind of fake-data smell modeling discipline calls out. Moving v3-grounding-lifetime to dev-dependencies is correct since it's only used in tests. No diff lines violate INVARIANTS, CODING, or TESTING rubrics.

[briansrls]

Codex schedule review (b7c2675f) — no blocking findings

Re-verified on current PR tip (576980606): Stratum A still fails closed on row-shape issues via GroundingTestsDiagnostic; the strengths note in the review remains accurate. No blocking items to implement.

cargo test -p v3-grounding-tests — 7 passed.

— sent from nimble-pike-489

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-5-pro
• Commit: 57698060 · Trigger: manual
• Conversation: View conversation

---

1. Story of the diff

This PR tightens the Stratum-A grounding test path so template-contract rows are no longer accepted merely because nearby fields look plausible. The main mechanism is a set of closed-shape projections in src/v3/grounding_tests/src/stratum_a.rs: MethodTemplateContract, MethodRef, MethodDeclaration.value_body, and PlaceholderConvention now reject missing fields, duplicates, extras, wrong instantiation targets, wrong literal kinds, and non-nullary placeholder payloads instead of silently projecting a digest. It also makes the lockstep mismatch diagnostic match the actual evidence: the digest comparison is list-scoped, so the diagnostic no longer fabricates a method_name for a whole-list mismatch (src/v3/grounding_tests/src/diagnostic.rs:27-29, src/v3/grounding_tests/src/stratum_a.rs:583). Finally, v3-grounding-lifetime is moved to dev-dependencies, matching the test-only direction of this grounding crate (src/v3/grounding_tests/Cargo.toml:15).

I reviewed this against the supplied invariant, modeling, coding, and testing references. chatgpt-review-c98c8b42-ad16-48…

chatgpt-review-a369cc2c-abd7-49…

chatgpt-review-6bbad6aa-a592-42…

chatgpt-review-e50bdee9-26c1-4f…

2. Invariant categories

1. LAYER MODEL — N/A. This diff reads existing Dag, DeclarationId, FieldValue, TypeConnective, and ValueBody shapes but does not add or mutate substrate types, dag.rs fields, cross-pass carriers, or new .dag variants; it is implementation/test-projection hardening in grounding_tests.
2. INVARIANTS.md + modeling-discipline.md — Compliant. Fail-closed and single-authority are improved: method_registry_name now requires the referenced declaration to be a MethodDeclaration instantiation before reading its name (src/v3/grounding_tests/src/stratum_a.rs:143-164), enforce_closed_record_schema rejects duplicate, missing, and extra fields instead of ignoring them (src/v3/grounding_tests/src/stratum_a.rs:341-374), and the lockstep diagnostic removes the fake method axis for list-level digest evidence (src/v3/grounding_tests/src/diagnostic.rs:27-29).
3. CODING.md — Compliant. The new logic is expressed as small free helpers with explicit inputs and typed result flow rather than methods on Dag: enforce_closed_record_schema(fields, list_name, row_index, record_kind, allowed) -> Result<(), GroundingTestsDiagnostic> is a precise data→diagnostic contract (src/v3/grounding_tests/src/stratum_a.rs:343-349), and the call sites propagate via ? rather than panicking or defaulting (src/v3/grounding_tests/src/stratum_a.rs:413-419, src/v3/grounding_tests/src/stratum_a.rs:438-444).
4. TESTING.md — Finding, non-blocking. The diff adds real fail-closed behavior at src/v3/grounding_tests/src/stratum_a.rs:343-374, src/v3/grounding_tests/src/stratum_a.rs:143-164, and src/v3/grounding_tests/src/stratum_a.rs:220-237, but the only changed unit assertion just checks that the new diagnostic variant’s display string is non-empty (src/v3/grounding_tests/src/lib.rs:56-60). For a fix/hardening PR, I would expect at least one behavior-level regression test for a malformed row shape, such as duplicate/extra MethodRef fields, non-MethodDeclaration decl, or a placeholder variant with payload. I would not block on this because the changed code is itself test infrastructure and the positive path is still exercised by the grounding checks, but the negative behavior is currently unpinned.
5. LOCKED DESIGN DECISIONS — N/A. The diff does not alter locked design docs, substrate design decisions, or declared locked surfaces. It stays inside the existing Stratum-A grounding-test machinery.
6. TRACKED vs UNTRACKED DEBT — N/A. I do not see new TODOs, compatibility bridges, deprecation scaffolds, or temporary adapters. The phase-bounded placeholder gate is a closed current contract that fails on unsupported variants (src/v3/grounding_tests/src/stratum_a.rs:204-237), not an unchecked bridge.

3. Verdict

APPROVE_WITH_COMMENTS

The core invariant direction is good: this PR removes permissive projections, preserves typed diagnostic flow, and avoids fabricated lockstep detail. My only comment is the missing negative regression coverage for the newly introduced fail-closed cases; adding one focused malformed-row test would make the hardening much harder to accidentally undo.

[briansrls]

Claude schedule review (57698060) — APPROVE

Confirmed: HEAD is 576980606 (matches the review snapshot). The APPROVE rationale matches the tree — closed-record enforcement, MethodDeclaration instantiation gate + closed name record in method_registry_name, nullary PlaceholderConvention, duplicate-safe row_field, list-scoped StratumALockstepListDigestMismatch, and v3-grounding-lifetime as a dev-dependency.

cargo test -p v3-grounding-tests — 7 passed. No code changes requested.

— sent from nimble-pike-489