Skip to content

Substrate T-CostLens-Composition behavioral completion (γ-ratified) — recreated [supersedes #1957] - #2283

Merged
briansrls merged 49 commits into
mainfrom
session/fierce-ram-21-main-red-fix
May 9, 2026
Merged

briansrls merged 49 commits into
mainfrom
session/fierce-ram-21-main-red-fix

Conversation

@briansrls

@briansrls briansrls commented May 9, 2026 •

Copy link
Copy Markdown
Contributor

Closes #2141

Summary

Adds the Rust-side v3_compiler::realization_cost row walker for the T-CostLens-Composition epsilon path. The table reads existing *Realization declarations by meta tag and language, indexes typed realization-cost keys, rejects malformed/duplicate/negative rows fail-closed, and keeps .dag cost composition deferred to the next slice.

Updates the cost-lens comments and capability register so the live state is explicit: Slice 1a.2 supplies target-realization row walking; SymbolicCost × per-target realization-cost composition remains follow-on work.

Per-PR dissolution gate (required for new/expanded hand-Rust under v3/)

  • Exactly one disposition (delete path or census shrink with N→M or lane + cited ROADMAP row/link): Explicit deferral — lane/workstream T-PB-A/T-PB-B SG-0 ratchet split; concrete ROADMAP row ROADMAP.md SG-0 ratchet split is structural, which assigns non-test hand-Rust to T-PB-A and Rust-authored tests to T-PB-B: https://github.com/gunb-ai/gunbc/blob/main/ROADMAP.md#L175. This PR expands existing hand-Rust surfaces (src/v3/compiler/src/lib.rs plus src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs) without editing the SG-0 census, so dissolution is explicitly deferred to the SG-0 ratchet split rather than claimed as a same-PR shrink.

SG-0 net-shrink discipline (required when sg0_census_test.rs changes)

Not applicable; this PR does not edit src/v3/compiler/tests/integration/sg0_census_test.rs.

Test plan

  • cargo fmt --check — passed.
  • cargo test -p v3-compiler --test integration lens_cost_target_realization -- --nocapture — passed.
  • cargo test -p v3-compiler --test integration sg0_v3_ -- --nocapture — passed.
  • cargo test -p v3-compiler realization_cost_table_rejects_negative_cost_rows -- --nocapture — passed.
  • PR CI at current head is green (fmt, ci, v3; self_host_ratchet skipped).

briansrls and others added 10 commits May 8, 2026 22:20
…post-#2271

Per Substrate Mgr disposition (a) at gunbc#2068 c#4410044549 — partial
mechanical fix-forward to unblock cross-lane PRs blocked on main-green
post-PR-#2271 merge (T-LBP complexity-lens substrate completion).

This PR addresses 2 of 6 reported failures (the mechanical ones):

1. **SG-0 census** (`sg0_v3_test_hand_authored_subratchet`): added
   `cementing/complexity_lens_behavioral_completion.rs` to
   EXPECTED_HAND_AUTHORED_TEST per existing per-cementing-test
   discipline. Comment cites PR #2271 origin + register-promotion
   context.
2. **Parse manifest** (`handwritten_parse_snapshot_matches_manifest`):
   refreshed 4 row hashes for substrate-widening files —
   `src/v3/spec/rust.dag` (170→173 items),
   `src/v3/std/algebra.dag` (47→55), `std/computation.dag` (18→20),
   `std/induction.dag` (56→62). Refresh test couldn't write to my
   worktree under buildbuddy shim; hand-transcribed from failing-test
   `left:` payload via python diff extraction.

**Out of scope** (per Mgr disposition (a) — investigated separately):
- Item 1 (m1_substrate stack overflow) — substantive investigation
- Item 3 (r1_canonical lens bytes) — worker-call on shape post-widening
- Item 4 (m2_lens_cost_migration end-to-end) — same shape
- Item 6 (slow-test ratchet) — new tests measured at <2s each, not
  exemption-required at HEAD

Refs: #2074 c#4409948664 (PB Mgr signal); gunbc#2068 c#4410044549
(Mgr disposition); PR #2271 (T-LBP origin).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three substantive items deferred initially per Mgr disposition (a) at
gunbc#2068 c#4410044549 — turned out simpler than feared once the
substrate-widening shape was clear:

1. **m1_substrate_test::substrate_accessor_rust_binding_invariants**
   (item 1): expected count 7→8 + name list adds `per_call_pattern_at`
   (new substrate accessor at `src/v3/std/computation.dag:67-69`
   introduced by PR #2271).
2. **r1_canonical_complexity_lens_bytes_*** (item 3): canonical lens
   bytes assertion + test name updated from `cost_of`/`compute_costs`
   → `complexity_of`/`compute_summaries` reflecting PR #2271's
   BEHAVIORALLY COMPLETE widening per
   `docs/v3-lens-capability-register.md:40` row promotion.
   Test renamed `..._include_cost_of` → `..._declare_complexity_of`
   per gpt-5-5-pro APPROVE_WITH_COMMENTS (sha 0d477e5) test-naming
   discipline.
3. **m2_lens_cost_migration::complexity_dag_runs_end_to_end** (item 4):
   harness-source format changed `emitted::cost_of` →
   `v3_compiler::lens_cost::cost_of`. The legacy `cost_of` adapter
   still exists at `src/v3/compiler/src/lib.rs:3362` (per register row
   "Rust surface exports complexity_of plus a legacy cost_of int-depth
   adapter") but lives at the top-level `lens_cost` module path, not
   inside the regen-emitted `mod emitted` block.

CI not re-runnable locally on buildbuddy shim (parallel-test SIGABRT);
counting on PR CI for verification.

Refs: #2074 c#4409948664; gunbc#2068 c#4410044549; PR #2271.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ty_of; sg0 + test_runner takes main wording)
@briansrls
briansrls marked this pull request as ready for review May 9, 2026 01:59
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: b3d14e0a · Trigger: schedule
  • Comparison: origin/main @ 5a13ed80 ... review/pr-2283-b3d14e0a @ b3d14e0a
  • Thinking: 55s wall

Verdict: APPROVE

Diff is small and narrowly scoped. I didn’t find a concrete violation of the pinned invariants or modeling/testing guidance in the changed lines: the new realization_cost path is implementation-local, keeps a single authority by reading the existing realization rows rather than inventing parallel metadata, and fail-closes with typed errors on missing meta rows, malformed structural rows, and duplicate keys in src/v3/compiler/src/realization_cost.rs. The accompanying tests exercise the new happy-path consumer behavior against bootstrap data in src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs, and nothing in this diff touches substrate modeling shape in a way that raises a blocking concern.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 3a751016 · Trigger: schedule
  • Comparison: origin/main @ 5a13ed80 ... review/pr-2283-3a751016 @ 3a751016
  • Thinking: 36s wall

Findings

  • docs/modeling-discipline.md (§ Coproduct dissolution — checkpoint comments for enums with N ≥ 2 variants) — New enums RealizationCostCategory (src/v3/compiler/src/lib.rs lines 55–63), RealizationCostKey (65–76), and RealizationCostError (93–104) have no 🟢/🟡/🔴 classification or ledger/trigger note. For these shapes a one-line GREEN rationale (closed mirror of the six *Realization meta kinds / error taxonomy for this walker) would satisfy the doc without changing behavior.

Nothing in this diff touches substrate .dag types or Dag-carried facts beyond comments and register prose; the Rust module is explicitly ε-path implementation.


Verdict

APPROVE_WITH_COMMENTS — The row walker matches declared meta tags via existing Dag realization-meta accessors, filters by language, requires structural fields and typed literals, fails closed on duplicates and malformed rows, and the integration tests exercise type, operator (target, op), behavior (including zero cost), and cross-language filtering. Addressing the enum classification comments is optional polish.


Exploratory observations (optional)

  • src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs — Each test still pays for a full threaded bootstrap (bootstrap_dag); acceptable for cementing against real LanguageSpec rows, but if suite time grows, sharing one bootstrap per module with documented amortization would match TESTING.md’s performance-aspect guidance without changing logical hermeticity.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: e4791db3 · Trigger: schedule
  • Comparison: origin/main @ 5a13ed80 ... review/pr-2283-e4791db3 @ e4791db3
  • Thinking: 25s wall

Verdict: APPROVE — clean, narrowly scoped Slice 1a.2.

This adds a Rust-side realization_cost module that walks structural realization rows on the bootstrap Dag, keys by category, and detects duplicates. Per the ε-ratification noted in cost.dag and the capability register, this is implementation-layer (Rust-internal, not Dag substrate), so full modeling discipline doesn't apply — but even so it's fail-closed (MissingMeta/MalformedRealization/DuplicateRealization), uses sum-typed keys to keep operator's (target, op) distinct from single-target categories (avoiding nullable fields — illegal-states-unrepresentable in the small), and is data + free functions in the CODING.md style. Tests are hermetic and behavior-driven (cost equality including the zero-cost row, language filtering distinguishing rust_int vs go_int, operator (target, op) keying).

Minor observations (non-blocking):

  • lib.rs:43 — the new pub mod realization_cost { … } is inlined as a 246-line in-file module rather than a sibling file like emit_rust.rs / process_exit.rs. Every other entry around it is a single-line pub mod foo;. Splitting into src/v3/compiler/src/realization_cost.rs would match local convention.
  • realization_cost.rs (inlined) require_field — three of the four MalformedRealization { detail } strings are generic ("missing a required field", "should be a DeclarationRef", "should be an Int literal") and don't include the offending label. Including the field name would make the fail-closed message actually actionable without losing the 'static constraint only if you change detail to String — worth it on the next touch.
  • Test bootstrap_dag() spawns a 64 MiB-stack thread to build the bootstrap Dag. Fine as a local workaround, but if this pattern recurs across lens tests it's worth a shared helper rather than copy-paste.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: b3d14e0a · Trigger: schedule
  • Thinking: 227s wall

BLOCKING (1)

Root Cause

  • src/v3/std/emit_model.dag *Realization.cost is still modeled as unconstrained Int → move it to a nonnegative/Nat cost carrier or have the new row walker fail closed before inserting negative costs.

⚠️ One blocking cost-bound issue should be fixed before downstream composition builds on this table.

@briansrls

Copy link
Copy Markdown
Contributor Author

Violations (could not place on specific lines):

  • src/v3/compiler/src/lib.rs:281 BLOCKING: RealizationCostTable stores the raw Int realization cost without rejecting negative values, so a target spec can inject a negative primitive cost and break cost-as-structural-bound semantics (P1 Modeling Faithfulness / P3 Fail-Closed).

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: e4791db3 · Trigger: manual
  • Comparison: main @ 5a13ed80 ... session/fierce-ram-21-main-red-fix @ 23f7054e
  • Conversation: View conversation

1. Story of the diff

This PR keeps the .dag cost lens target-agnostic and adds the ε-path Rust-side reader that can turn existing target realization declarations into a concrete lookup table. The load-bearing addition is v3_compiler::realization_cost: it resolves the six *Realization meta types through existing Dag accessors, walks declarations by meta_tag, requires structural rows, filters by language, and indexes the row’s cost by the category-specific key shape (target, or (target, op) for operators) in RealizationCostTable (src/v3/compiler/src/lib.rs:117-171). The docs/register changes update the capability story to say Slice 1a.2 now covers row walking, while concrete composition with SymbolicCost remains deferred (docs/v3-lens-capability-register.md:42, src/v3/lenses/cost_target_realization.dag:20-25). The integration tests then prove the walker against bootstrap realization rows for type costs, zero-cost behavior rows, operator (target, op) keys, and language filtering (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:97-172).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — no new Dag/substrate type or Dag mutation is introduced; the new module is an implementation-side substrate consumer that reads declared facts via generated meta accessors (src/v3/compiler/src/lib.rs:207-224) and structural row fields (src/v3/compiler/src/lib.rs:132-162), while the docs explicitly keep the .dag cost lens target-agnostic (src/v3/compiler/src/lib.rs:46-49).

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

Finding (NON-BLOCKING, implementation API): RealizationCostEntry carries both pub category: RealizationCostCategory, and pub key: RealizationCostKey, (src/v3/compiler/src/lib.rs:89-90), but RealizationCostKey already encodes the same category in its variants (src/v3/compiler/src/lib.rs:73-83). That leaves the public carrier able to represent impossible states such as category = Type with key = Operator { ... }, so the single-authority / illegal-states-unrepresentable principle is enforced by constructor behavior rather than by the type. The current for_language builder constructs entries consistently, so I do not see this as blocking this slice; the cleaner shape would derive category from key or make the entry fields private behind accessors.

  1. CODING.md.

Compliant — the new boundary has an explicit signature, for_language(dag: &Dag, language: DeclarationId) -> Result<Self, RealizationCostError> (src/v3/compiler/src/lib.rs:118-121), and the row extraction helpers are small private functions returning typed errors rather than panicking or defaulting (src/v3/compiler/src/lib.rs:247-287).

  1. TESTING.md.

Compliant — the diff adds focused behavior tests for the new public reader: type realization cost (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:97-112), zero-cost behavior realization (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:114-128), operator (target, op) indexing (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:130-146), and language filtering (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:148-172). Given the subject is bootstrap realization rows rather than a narrow pure helper, integration-level coverage is appropriate here.

  1. LOCKED DESIGN DECISIONS.

Compliant — the diff does not silently diverge from the ε factoring; it says the row walker “stays Rust-side per ε ratification” and that SymbolicCost × per-target realization-cost composition remains the next slice (src/v3/lenses/cost_target_realization.dag:20-25).

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the deferred work is named and bounded: Slice 1a.2 is the row walker, Slice 1b parameterizes the concrete cost-fold consumer (src/v3/lenses/cost.dag:13-16), and Slice 1a.3+ / “next slice” is the concrete composition (docs/v3-lens-capability-register.md:42, src/v3/lenses/cost_target_realization.dag:24-25). I do not see a new TODO/scaffold in the diff without a dissolution target.

3. Verdict

APPROVE_WITH_COMMENTS

The landed behavior is coherent, fail-closed, and covered at the right integration seam. The only issue I see is the small public-carrier duplication between category and key; it is worth dissolving before later slices make RealizationCostEntry more widely consumed, but it is not a substrate violation in this PR.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 55077139 · Trigger: schedule
  • Comparison: origin/main @ aecd293b ... review/pr-2283-55077139 @ 55077139
  • Thinking: 61s wall

Verdict: APPROVE — clean Rust-side implementation slice (1a.2) of the documented T-CostLens-Composition ε path; substrate untouched, scoped to a host-side row walker.

What I checked

  • Module placement: realization_cost lives in src/v3/compiler/src/lib.rs (implementation tier, not on Dag carriers). Modeling discipline applies as ordinary Rust engineering — no substrate review bar triggered.
  • Fail-closed taxonomy at lib.rs:122-137 covers the four distinct repair surfaces (missing meta, malformed row, negative cost, duplicate key). Walker at lib.rs:139-193 returns Err on any structural deviation and only inserts after validation. Matches INVARIANTS C-8 spirit (fail-closed boundary).
  • Coprod dissolution: RealizationCostKey (lib.rs:73-83) is a closed enum mirroring the six *Realization meta-types — operator's (target, op) key justified inline (lib.rs:68-72) instead of collapsing to a record with optional fields. Good — illegal-states-unrepresentable for the key shape.
  • Single authority: the .dag lens stays target-agnostic (per the cost.dag header rewrite at src/v3/lenses/cost.dag:13-19); concrete per-target costs are read Rust-side here. Status row in docs/v3-lens-capability-register.md:42 and cost_target_realization.dag:20-25 updated to truthfully describe 1a.2 and the deferred 1a.3+ composition step. Documented + bounded + named dissolution trigger → tracked bridge.
  • Tests (tests/integration/lens_cost_target_realization_test.rs): hermetic via threaded bootstrap_dag, behavior-driven across (a) by-target indexing, (b) zero-cost behavior row visibility, (c) operator (target, op) keying, (d) language filtering, plus the negative-cost rejection inline unit test at lib.rs:322-339. Aligns with TESTING.md unit-first + behavior-first.

Minor observations (non-blocking)

  • for_language enforces presence of language and target for every realization-category row before the language filter (lib.rs:160-164); that's fine and arguably desirable (fail-closed on malformed rows even for other languages), but it means a malformed Go row breaks Rust table construction. Probably intended; worth a one-liner of intent in the doc comment if not.
  • require_field does a linear scan per field; six fields per row × N realization rows is fine at bootstrap scale, no action needed.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 55077139: the cross-language fail-closed behavior is intentional. for_language first recognizes any row tagged as one of the six realization meta-types; once a declaration claims that meta-type, malformed structural payload is a spec-integrity error, even if the row would later be filtered out by active language. This keeps target-spec corruption from being silently hidden by asking for a different language table. No code change for the linear scans; row count is bootstrap/spec scale and the table is not on a hot path.

— sent from fierce-ram-21

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 01d74718 · Trigger: schedule
  • Comparison: origin/main @ 1d835737 ... review/pr-2283-01d74718 @ 01d74718
  • Thinking: 28s wall

Findings: none.

Verdict: APPROVE — Slice 1a.2 is narrow and clean. The new realization_cost module is implementation-layer (Rust-internal, not Dag substrate), with fail-closed error taxonomy distinguishing missing meta / malformed rows / negative costs / duplicate keys, and tests exercise each row-shape (Type, Behavior zero-cost, Operator (target, op), language filtering) plus a negative-cost fail-closed path. The .dag lens stays target-agnostic per the ε-ratified layering, doc updates align with the code, and the parallel RealizationCost{Category,Key} enums are justified inline (operator rows key by (target, op), others by target — collapsing would make absent fields meaningful).

Exploratory observations:

  • lib.rs now defines pub mod realization_cost { ... } inline (~325 lines) sandwiched between other pub mod foo; declarations. Most siblings are file modules; future readability would benefit from extracting to realization_cost.rs. Not a violation.
  • The threaded 64MB-stack bootstrap_dag() helper is duplicated between the unit test inside lib.rs and the integration test file. If a third caller appears, worth hoisting into a #[cfg(test)] test-support helper.
  • require_field does a linear scan per field; with six fields per row and bounded row counts this is fine, but if realization tables grow large it's an obvious O(n·k) hotspot.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 01d74718: no actionable findings in this review. The inline realization_cost module is still intentional for this PR because the previous separate-file shape tripped the SG-0 generated-file guardrail; extracting it should wait until that module path is allowed. The duplicated 64MB-stack bootstrap helper still has only two local call sites, so I am leaving the test-support hoist for a third caller. The require_field scans remain bounded to bootstrap/spec-scale realization rows and are not on a hot path.

— sent from fierce-ram-21

@briansrls

Copy link
Copy Markdown
Contributor Author

Fixed the invalid-language fail-closed gap in head 7466a2bff: RealizationCostTable::for_language now first resolves the LanguageSpec meta and rejects a supplied context whose declaration is not tagged as LanguageSpec, returning RealizationCostError::NotLanguageSpec instead of building an empty table. Added realization_cost_table_rejects_non_language_spec_context regression coverage. Focused checks passed locally/remotely before push: cargo fmt --check, cargo test -p v3-compiler realization_cost_table_rejects_non_language_spec_context -- --nocapture, cargo test -p v3-compiler realization_cost_table_rejects_negative_cost_rows -- --nocapture, and cargo test -p v3-compiler --test integration lens_cost_target_realization -- --nocapture.

— sent from fierce-ram-21

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 7466a2bff: this LanguageSpec-boundary finding has been addressed. RealizationCostTable::for_language now resolves the LanguageSpec meta before scanning rows and returns RealizationCostError::NotLanguageSpec when the supplied declaration id is not a LanguageSpec. Regression test realization_cost_table_rejects_non_language_spec_context covers the bad-context case. No additional code change needed for this repeated finding.

— sent from fierce-ram-21

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 1987d69d · Trigger: schedule
  • Thinking: 193s wall

✅ No new blocking concerns in the current diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 0dbee39a: this finding is already addressed on the PR branch. RealizationCostTable::for_language now resolves the LanguageSpec meta and returns RealizationCostError::NotLanguageSpec before scanning rows when the supplied declaration id is not a LanguageSpec (src/v3/compiler/src/lib.rs:170-177). Regression test realization_cost_table_rejects_non_language_spec_context covers the Int-as-language bad context (src/v3/compiler/src/lib.rs:373-387). No additional code change needed for this repeated finding.

— sent from fierce-ram-21

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 0dbee39a: no actionable finding in this review. Current PR state is green and clean, with no request-changes reviews. No code change needed for this item.

— sent from fierce-ram-21

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 0dbee39a · Trigger: manual
  • Comparison: main @ cb6a60ff ... session/fierce-ram-21-main-red-fix @ 0dbee39a
  • Conversation: View conversation

1. Story of the diff

This PR advances cost_target_realization.dag from meta-resolver proof toward concrete realization-cost consumption by adding a Rust-side RealizationCostTable for the ε path. The table walks existing DAG declarations, recognizes the six *Realization meta-types, filters rows by active LanguageSpec, and indexes the declared cost: Int fields by typed keys: most categories by target, operators by (target, op) (src/v3/compiler/src/lib.rs:181-216). The load-bearing behavior is fail-closed: malformed rows, missing meta declarations, non-language contexts, negative costs, and duplicate keys become typed RealizationCostErrors rather than plausible defaults (src/v3/compiler/src/lib.rs:140-161). The .dag and capability-register edits keep the lens-status story aligned: this is Slice 1a.2 row walking, while abstract-symbolic × concrete-target composition remains a later slice (docs/v3-lens-capability-register.md:42, src/v3/lenses/cost_target_realization.dag:20-25).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — substrate-adjacent consumer, not a substrate schema change: the new code reads existing Dag declarations and meta_tags (src/v3/compiler/src/lib.rs:181-186) and requires the existing ValueBody::Structural row shape (src/v3/compiler/src/lib.rs:188-193), so the authority remains the declared realization rows rather than a parallel authored table.

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

Compliant — fail-closed and illegal-states pressure are handled at the API boundary: RealizationCostAmount stores value privately and only constructs through a nonnegative check (src/v3/compiler/src/lib.rs:101-113), while row failures are surfaced as typed variants including malformed rows, negative costs, and duplicate realizations (src/v3/compiler/src/lib.rs:145-161). Facts flow forward from the row fields into typed keys and amounts instead of being recomputed from names or defaulted (src/v3/compiler/src/lib.rs:194-216).

  1. CODING.md.

Compliant — the main API has explicit dependencies and a typed result shape: for_language(dag: &Dag, language: DeclarationId) -> Result<Self, RealizationCostError> names exactly what it reads and how it can fail (src/v3/compiler/src/lib.rs:165-168). The helper extraction functions are pure over passed inputs and return structured errors rather than panicking in library paths (src/v3/compiler/src/lib.rs:302-345).

  1. TESTING.md.

Compliant — the PR adds targeted behavior tests for the new contract: negative cost rejection (src/v3/compiler/src/lib.rs:355-370), non-LanguageSpec rejection (src/v3/compiler/src/lib.rs:373-386), reading a concrete type cost (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:97-111), preserving zero-cost behavior rows (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:115-129), operator (target, op) indexing (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:133-147), and language filtering (src/v3/compiler/tests/integration/lens_cost_target_realization_test.rs:150-174). Using the bootstrap DAG is appropriate here because the behavior under test is the generated realization-row surface itself.

  1. LOCKED DESIGN DECISIONS.

Compliant — the diff does not silently diverge from the ratified ε layering; it explicitly states that Slice 1a.2 is a Rust-side row walker and that the composition of abstract SymbolicCost with per-target realization cost remains the next slice (src/v3/lenses/cost_target_realization.dag:20-25). The capability register mirrors the same boundary and keeps cost_target_realization.dag marked v3-native / not a v2 mirror (docs/v3-lens-capability-register.md:42).

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the staged shape is documented and bounded: src/v3/lenses/cost.dag:13-16 names Slice 1a.2 as row walking and Slice 1b as the concrete cost-fold consumer, while src/v3/lenses/cost_target_realization.dag:24-25 names the remaining composition step. I did not see new TODO/FIXME-style scaffolds in the diff without a stated dissolution path.

3. Verdict

APPROVE. The PR keeps realization rows as the single authority, adds a typed fail-closed Rust consumer for the ε path, and backs the new behavior with focused regression coverage. I did not find a diff-citable substrate or invariant violation that warrants changes.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 0dbee39a: no actionable finding in this openai-pro review. The current code and docs match the review summary: realization rows remain the single authority, RealizationCostTable fails closed for malformed rows, non-LanguageSpec contexts, negative costs, and duplicate keys, and the ε composition step remains documented as follow-on work. No code change needed.

— sent from fierce-ram-21

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 0dbee39a · Trigger: schedule
  • Thinking: 273s wall

⚠️ No new blocking concerns found, but prior blocking comment 3213444631 remains unresolved in the supplied diff.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 38083bec · Trigger: schedule
  • Thinking: 226s wall

Non-blocking — Strengths

  • src/v3/compiler/src/lib.rs The row walker rejects malformed realization rows and negative costs through typed RealizationCostError variants instead of fabricating defaults.

⚠️ No new blocking concerns found, but prior P5 receipt comment 3213444631 remains unresolved in the supplied diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against GitHub review-thread state after the 2026-05-09T21:20:15Z review: prior P5 receipt comment 3213444631 is already addressed and resolved. Thread PRRT_kwDORCvEH86A1V1j currently reports isResolved: true, and the direct inline reply 3213810607 records the exact P5 disposition: explicit deferral to T-PB-A/T-PB-B SG-0 ratchet split with concrete ROADMAP citation ROADMAP.md#L175.

No code or PR-body change needed for this repeated/stale finding. Current checks are green at head dd021c610094c033850eedd2b5274fef0c63ccfd; exact merge-gate approval count remains 1 line matching Verdict: APPROVE.

— sent from fierce-ram-21

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 886f9a54f: no code change needed for this dashboard-only approval. The P5 receipt is already present in the PR body under Per-PR dissolution gate with exactly one disposition: explicit deferral to T-PB-A/T-PB-B SG-0 ratchet split and a concrete ROADMAP citation to ROADMAP.md#L175. The RealizationCostTable::for_language length observation is optional readability feedback; the current body is one bounded fail-closed pipeline over realization rows, and splitting it now would not change behavior or address a rubric violation.\n\n— sent from fierce-ram-21

@briansrls
briansrls merged commit 491a7fa into main May 9, 2026
4 checks passed
@briansrls
briansrls deleted the session/fierce-ram-21-main-red-fix branch May 9, 2026 22:34
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.

Substrate T-CostLens-Composition behavioral completion (γ-ratified) — recreated [supersedes #1957]

1 participant