Repository navigation
feat(grounding): T-Ground-Lifetime-Analyzer implementation (R2 scope a/b/c) - #1206
Conversation
|
Manager checkpoint review — substantial first-cut; good algorithm shape; three findings to address before ready-for-review. Strengths:
Findings to address:
Pre-merge gate (per #1195 lesson): before ready-for-review:
Process for finding 2: ping #1133 with your read on whether lowering surfaces the R2 graph today. If it does, fold extraction in. If not, post the cross-program gap finding so I can route to Substrate / Coercion-Fold sibling lanes. Don't go ready-for-review until findings 1 + 2 are resolved. Update PR title to |
|
Review metadata
Findings
Verdict APPROVE_WITH_COMMENTS — new lane-local crate, no SG-0/Dag substrate touched, P1 receipts present in |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
746273e1· Trigger:schedule - Thinking:
239s wall
BLOCKING (1)
Root Cause
src/v3/grounding_lifetime/src/program.rsUseKind::IndeterminateGrowability is modeled as a growability-only marker but also bypasses the parameter ownership/transience proof → check indeterminate before the borrowed fast path or split opaque use from definite transient use.
| } else { | ||
| Ownership::Borrowed | ||
| }; | ||
| let growable = if ownership == Ownership::Borrowed { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Drop unused Ownership/LifetimeScope variants; document target-vs-program sum split in P1 rustdoc. - Add ProgramTypeFamily on BindingDef; encoding axis fails closed (UnderRefined) when Unclassified. - Add extract_bootstrap_dag_yields_empty_lifetime_program + encoding regression tests. Made-with: Cursor
|
Response to api-review (scheduled) — all three findings addressed in code
Pushed: — sent from nimble-pike-489 |
|
Manager APPROVE (#1133 relay) — no further code changes from this message: title/body/workspace-exclude + integration deferral receipt were already landed before this approval. Api-review follow-up is already on the branch ( CI: — sent from nimble-pike-489 |
…(PR #1206) Hoist IndeterminateGrowability + load-bearing axis check before the Borrowed -> Growability::NotApplicable short-circuit on FunctionParameter. Regression: function_param_indeterminate_growability_fails_closed_even_when_borrowed. Made-with: Cursor
|
Review metadata
Findings
VerdictAPPROVE_WITH_COMMENTS — The diff is scoped to a new Exploratory (optional): |
|
Inline review Finding was valid: Fix (pushed Regression: — sent from nimble-pike-489 |
…s (PR #1206) IndeterminateGrowability must not bypass Case-A transience: if a parameter would meet as Borrowed but has indeterminate growability without any UseKind::Transient witness, fail closed UnderRefined(ownership) when the growability axis is not load-bearing (so growability UnderRefined cannot mask the gap). - LanguageSpecAxes::string_family_growability_not_load_bearing for tests - Doc UseKind::IndeterminateGrowability vs Transient - Regression: indeterminate-only + optional growability; transient+indeterminate ok Made-with: Cursor
|
Codex api-review (BLOCKING) — ownership / transience gap — fixed Verified on current code: Fix (pushed Also: — sent from nimble-pike-489 |
Per docs/modeling-discipline.md §4: 🟢/🟡 classification + ledger or named trigger on multi-variant pub enums (facts, program, diagnostic). Encoding noted as single-variant until LanguageSpec expands the axis. Non-blocking api-review (composer-2) addressed in code comments only. Made-with: Cursor
|
Composer-2 api-review (Practice 4 / modeling-discipline §4) — addressed in code Valid rubric gap: new multi-variant Rust enums lacked 🟢/🟡 checkpoint lines. Pushed Exploratory typed — sent from nimble-pike-489 |
|
Review metadata
FindingsNone. The diff adds a self-contained Nothing in this diff clearly breaches INVARIANTS.md (fail-closed behavior is explicit; scaffolds carry named dissolution triggers where YELLOW), docs/modeling-discipline.md (checkpoint annotations present), CODING.md (data + free functions; structured errors), or TESTING.md (behavior-named tests, minimal constructed inputs for the fold). VerdictAPPROVE — Scope is narrow (new sibling crate, no Exploratory observations (optional)
|
|
Composer-2 api-review ( Re-checked current Exploratory ( — sent from nimble-pike-489 |
|
Review metadata
Findings: None. The new Verdict: APPROVE — The diff is focused, documents tracked bridge work (empty extract, lane-local diagnostics), and does not show a concrete violation of the pinned INVARIANTS / modeling-discipline / CODING / TESTING bar for the code it touches. |
|
Composer-2 api-review ( Confirmed against HEAD:
Verdict alignment: APPROVE stands; nothing in this review requests a follow-up commit. — sent from nimble-pike-489 |
- Add docs/briefs/t-ground-diagnostic.md (S lane): EmissionDiagnostic carrier, diagnostic-only ordering, Q6.5 Layer-1 consumer-only, C-8, P1, tests, #1206 lifetime mirror convergence, gates/deps/out-of-scope. - Point r2-grounding-manager lane table + pending list at the new brief. Made-with: Cursor
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
60ff47fa· Trigger:schedule - Thinking:
328s wall
BLOCKING (1)
Root Cause
src/v3/grounding_lifetime/src/extract.rsDag->LifetimeProgram projection is stubbed without a bootstrap/user gate -> implement the R2 projection or reject non-empty/unsupported Dag input with a typed diagnostic until lowering supplies the bind/use graph.
| /// Fail-closed on constructs the R2 analyzer does not model (once lowering surfaces them). | ||
| pub fn extract_lifetime_program(dag: &Dag) -> Result<LifetimeProgram, EmissionDiagnostic> { | ||
| let _ = dag; | ||
| Ok(LifetimeProgram::empty()) |
There was a problem hiding this comment.
BLOCKING: The public Dag boundary returns Ok(empty) for every input, so a Dag containing R2 bindings is silently accepted with all lifetime facts dropped instead of failing closed (modeling-discipline principle 1 / INVARIANTS C-8).
* WIP: nimble-pike-489 * WIP: nimble-pike-489 * chore: apply cargo fmt * WIP: nimble-pike-489 * WIP: nimble-pike-489 * WIP: nimble-pike-489 * chore: apply cargo fmt * fix(grounding-lifetime): address api-review tidy-ups (PR #1206) - Drop unused Ownership/LifetimeScope variants; document target-vs-program sum split in P1 rustdoc. - Add ProgramTypeFamily on BindingDef; encoding axis fails closed (UnderRefined) when Unclassified. - Add extract_bootstrap_dag_yields_empty_lifetime_program + encoding regression tests. Made-with: Cursor * fix(grounding-lifetime): fail-closed growability for borrowed params (PR #1206) Hoist IndeterminateGrowability + load-bearing axis check before the Borrowed -> Growability::NotApplicable short-circuit on FunctionParameter. Regression: function_param_indeterminate_growability_fails_closed_even_when_borrowed. Made-with: Cursor * fix(grounding-lifetime): require transient proof for opaque param uses (PR #1206) IndeterminateGrowability must not bypass Case-A transience: if a parameter would meet as Borrowed but has indeterminate growability without any UseKind::Transient witness, fail closed UnderRefined(ownership) when the growability axis is not load-bearing (so growability UnderRefined cannot mask the gap). - LanguageSpecAxes::string_family_growability_not_load_bearing for tests - Doc UseKind::IndeterminateGrowability vs Transient - Regression: indeterminate-only + optional growability; transient+indeterminate ok Made-with: Cursor * docs(grounding-lifetime): Practice 4 enum checkpoints (PR #1206) Per docs/modeling-discipline.md §4: 🟢/🟡 classification + ledger or named trigger on multi-variant pub enums (facts, program, diagnostic). Encoding noted as single-variant until LanguageSpec expands the axis. Non-blocking api-review (composer-2) addressed in code comments only. Made-with: Cursor * WIP: nimble-pike-489 * docs(briefs): author T-Ground-Diagnostic lane brief - Add docs/briefs/t-ground-diagnostic.md (S lane): EmissionDiagnostic carrier, diagnostic-only ordering, Q6.5 Layer-1 consumer-only, C-8, P1, tests, #1206 lifetime mirror convergence, gates/deps/out-of-scope. - Point r2-grounding-manager lane table + pending list at the new brief. Made-with: Cursor
|
Re: BLOCKING @ Verified on current Current control flow (
So an opaque Landed fix: Regression: No additional commit from this worktree: the finding matches pre-fix ordering; post-merge HEAD already satisfies C-8 for this case. — sent from nimble-pike-489 |
|
Re: BLOCKING @ Verified: On the merged #1206 line, Fix (post-merge follow-up on
Open a small follow-up PR from — sent from nimble-pike-489 |
|
Follow-up PR: #1218 (draft) — sent from nimble-pike-489 |
|
Re: [api-review] codex @ Verified against Not stale: the gap is real on Fix path (implemented, pending merge): #1218 — fail-closed when the reflected Once #1218 merges to — sent from nimble-pike-489 |
…urface (C-8) * WIP: nimble-pike-489 * WIP: nimble-pike-489 * chore: apply cargo fmt * WIP: nimble-pike-489 * WIP: nimble-pike-489 * WIP: nimble-pike-489 * chore: apply cargo fmt * fix(grounding-lifetime): address api-review tidy-ups (PR #1206) - Drop unused Ownership/LifetimeScope variants; document target-vs-program sum split in P1 rustdoc. - Add ProgramTypeFamily on BindingDef; encoding axis fails closed (UnderRefined) when Unclassified. - Add extract_bootstrap_dag_yields_empty_lifetime_program + encoding regression tests. Made-with: Cursor * fix(grounding-lifetime): fail-closed growability for borrowed params (PR #1206) Hoist IndeterminateGrowability + load-bearing axis check before the Borrowed -> Growability::NotApplicable short-circuit on FunctionParameter. Regression: function_param_indeterminate_growability_fails_closed_even_when_borrowed. Made-with: Cursor * fix(grounding-lifetime): require transient proof for opaque param uses (PR #1206) IndeterminateGrowability must not bypass Case-A transience: if a parameter would meet as Borrowed but has indeterminate growability without any UseKind::Transient witness, fail closed UnderRefined(ownership) when the growability axis is not load-bearing (so growability UnderRefined cannot mask the gap). - LanguageSpecAxes::string_family_growability_not_load_bearing for tests - Doc UseKind::IndeterminateGrowability vs Transient - Regression: indeterminate-only + optional growability; transient+indeterminate ok Made-with: Cursor * docs(grounding-lifetime): Practice 4 enum checkpoints (PR #1206) Per docs/modeling-discipline.md §4: 🟢/🟡 classification + ledger or named trigger on multi-variant pub enums (facts, program, diagnostic). Encoding noted as single-variant until LanguageSpec expands the axis. Non-blocking api-review (composer-2) addressed in code comments only. Made-with: Cursor * WIP: nimble-pike-489 * docs(briefs): author T-Ground-Diagnostic lane brief - Add docs/briefs/t-ground-diagnostic.md (S lane): EmissionDiagnostic carrier, diagnostic-only ordering, Q6.5 Layer-1 consumer-only, C-8, P1, tests, #1206 lifetime mirror convergence, gates/deps/out-of-scope. - Point r2-grounding-manager lane table + pending list at the new brief. Made-with: Cursor * WIP: nimble-pike-489 * chore: apply cargo fmt * WIP: nimble-pike-489 * docs(grounding-lifetime): sync program IR rustdoc with extraction C-8 guard Made-with: Cursor * WIP: nimble-pike-489 * docs(briefs): split UnderRefined acceptance into Example 1 + Example 5 - Lineage, Scope, test plan, and dissolution explicitly require separate TestClaim receipts for bound UnderRefined (Example 1 / Modeling 5 sketch) and algebra ambiguity (Example 5, unspecified_axis "algebra"). - Closes api-review gap on PR #1216 (codex @ 613c5fd). Made-with: Cursor
Audit pass per manager dispatch (#1133 inbox 4348240942) over the 5 merged R2 Grounding briefs after the morning's regression+refactor cycle (#1187 / #1195 / #1196 / #1206 / #1218 / #1220 / #1229). Findings: - Status rows in r2-grounding-manager.md L65/L66/L69 still said "NOT YET AUTHORED"; updated to BRIEF LANDED (+ Phase 1 / Phase 2 partial / IMPL LANDED / PR citations). - Pending list at L140-150 listed lanes as pending without naming the merged briefs / impl PRs; updated each row with explicit PR list and outstanding-work pointers. - INVARIANTS.md:86-123 P1 procedure cite drifted to L94-129 (4 occurrences across 3 briefs). - emit_model.dag:302 LanguageSpec cite drifted to L303 (4 occurrences across 2 briefs). - pending list line numbers shifted by my own status-row update; diagnostic / cross-target-meta / tests / lifetime-analyzer briefs updated to point at correct shifted lines. No structural drift requiring escalation. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…itTemplate (#1236 follow-up) (#1238) * docs(briefs): post-merge line-citation + status-row audit Audit pass per manager dispatch (#1133 inbox 4348240942) over the 5 merged R2 Grounding briefs after the morning's regression+refactor cycle (#1187 / #1195 / #1196 / #1206 / #1218 / #1220 / #1229). Findings: - Status rows in r2-grounding-manager.md L65/L66/L69 still said "NOT YET AUTHORED"; updated to BRIEF LANDED (+ Phase 1 / Phase 2 partial / IMPL LANDED / PR citations). - Pending list at L140-150 listed lanes as pending without naming the merged briefs / impl PRs; updated each row with explicit PR list and outstanding-work pointers. - INVARIANTS.md:86-123 P1 procedure cite drifted to L94-129 (4 occurrences across 3 briefs). - emit_model.dag:302 LanguageSpec cite drifted to L303 (4 occurrences across 2 briefs). - pending list line numbers shifted by my own status-row update; diagnostic / cross-target-meta / tests / lifetime-analyzer briefs updated to point at correct shifted lines. No structural drift requiring escalation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(briefs): cite existing HigherOrderMethodSpec authority instead of proposed MethodEmitTemplate Per codex BLOCKING on PR #1236: the audit-pass status row cited `MethodEmitTemplate` (a proposed name from earlier dispatch text) as if it were a declared substrate authority, but no declaration exists on main. The actual dual-template carrier in question is `HigherOrderMethodSpec` at dsl/extdeps/languages/rust/emit.dag:265 (the legacy v2-emit shape Phase 1 Rust higher-order rows can't yet consolidate). Renamed both occurrences to cite the existing carrier + flag the cross-manager request to jolly-ram-908 (#1130) for the substrate-shape decision; no future-tense type name claimed as declared. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
T-Ground-Lifetime-Analyzer (R2 scope a/b/c)
Opened from session-dashboard for session
nimble-pike-489.Scope
Owned.CompilerDiagnosticKindextension,src/v3/compiler/edits.Integration deferral (PB-Zero–style receipt)
extract_lifetime_programruns against bootstrapDag::new(), which seeds the substrate and does not yet carry user-program bind/use graphs. Integration with compile-output Dags that carry real bindings and use sites is sibling-lane work (T-Ground-Coercion-Fold consumer wiring + lowering surface). This lane delivers the analyzer; lane integration delivers the consumer. Worked Examples 3–4 parity is exercised today via explicitLifetimeProgramfixtures +analyze_lifetime_program; the public entryanalyze_lifetime_facts(&Dag, &LanguageSpecAxes)preserves test plan item 7 (no annotation sidecar).Carriers & discipline
LifetimeFacts+ axis sums (Ownership,LifetimeScope,Growability,Encoding): P1 Steps 1–3 documented insrc/v3/grounding_lifetime/src/facts.rs.EmissionDiagnosticmirror (ContradictoryUse/UnderRefined/OutOfR2Scope); noCompilerDiagnosticKindextension (Q6.5 Layer-1 consumer).Pre-merge gates run
cargo test -p v3-grounding-lifetimecargo clippy -p v3-grounding-lifetime --all-targets -- -D warningscargo test -p v2-compiler-testscargo test -p v3-compiler --test integration lane2_stage_2d_symbolic_costcargo test --workspace --exclude v2-compiler-testscargo fmt --all --check