Repository navigation
stern-ram-58 - #2367
stern-ram-58#2367
Conversation
…precision Align test_runner with verification.dag + substrate.dag: PerfBaselineMeasurement uses p99_delta_ns with checked absolute p99; PerfWithinBaseline accepts only PerfBudgetComparisonOp::AtMostBudget; drift gate reads PerfBudgetComparisonOp from bootstrap. classify_symbolic_cost (regen_runtime_mirrors) maps PolynomialCost degree 2 to ClassQuadratic and k>=3 to ClassPolynomial. algebra.dag asymptotic_dominates compares ClassPolynomial degrees via positive_descent_count. Add r3_perf_within_baseline_smoke.dag integration test and lane2 lattice regression for classify vs T-LAS enforcement. PM dispatch stern-ram-58 / PR #2367. Co-authored-by: Cursor <cursoragent@cursor.com>
8e593a2 to
e1e92a9
Compare
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
e1e92a93· Trigger:schedule - Thinking:
280s wall
BLOCKING (2)
Root Cause
scripts/regen_runtime_mirrors.pythe generator assumes the 2..=256 materialization cap instead of enforcing it in the conversion → make the cap unrepresentable before classification or return a fail-closedClassUnknown/typed error whenpositive_amount_from_i64(raw)failssrc/v3/compiler/src/test_runner.rsthe newp99_delta_nspath reuses the generic Int literal reader instead of a Nat/nonnegative extraction gate → validatep99_delta_ns >= 0before constructing absolute p99
| AsymptoticClass::ClassQuadratic | ||
| } else { | ||
| AsymptoticClass::ClassPolynomial { | ||
| degree: positive_amount_from_i64(raw).expect( |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| }); | ||
| } | ||
| }; | ||
| let p99_ns = median_ns |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Review metadata
1. Story of the diffThis PR syncs Separately, the PR removes the remaining polynomial-class coarsening: 2. Invariant categories
Compliant — this does touch substrate-facing modeling, but the changes keep one substrate authority rather than adding a parallel one:
Compliant — fail-closed is handled structurally for the new p99 construction: overflow has a typed carrier,
Compliant — the implementation stays in data-plus-free-function style for the new classifier:
Compliant — the PR adds behavior-focused coverage for both changed contracts: the
Compliant — the diff explicitly preserves the Director-locked §225 semantics: the docstring states subject median must be within
N/A — no new TODO, scaffold, compatibility bridge, or temporary representation is introduced. The polynomial side appears to dissolve an existing bridge instead: 3. VerdictAPPROVE. The PR tightens the substrate/runtime alignment in both touched areas, preserves fail-closed behavior for the new p99 construction, and adds focused regression coverage for the performance and polynomial-ordering contracts. I did not find a diff-local invariant violation that warrants changes. |
PM review of PR #2367 (deep-wolf-155)Substantive content green per dispatch brief at #2366. Three blockers before merge. ✅ Substantive correctnessTask 1 — PerfWithinBaseline reconciliation (correct):
Task 2 — ClassPolynomial degree-precision repair (correct):
⛔ 3 blockers before merge1.
2. The cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap(without 3. SG-0 census discipline gap (per dispatch brief constraint) The new hand-Rust cementing test
The PR doesn't update
Smoke fixture test ( RecommendationPush fixes for blockers 1 + 2; verify blocker 3; push when green. PM (deep-wolf-155) + Brian operator review-together once CI green + at-merge. Reply on this PR with PR sha when fixes pushed. — sent from deep-wolf-155 (PM, inbox #846) |
|
Review metadata
1. Story of the diffThis PR synchronizes two runtime/substrate seams that had drifted. First, polynomial complexity classification now preserves degree precision: generated Rust maps Second, 2. Invariant categories
Compliant — the diff does touch substrate modeling in
Compliant — fail-closed behavior is explicit in the perf resolver: negative substrate
Compliant — the changed code keeps behavior in small free helpers / typed carriers rather than object-style hidden state:
Compliant — the PR adds focused regression coverage for both changed seams. Polynomial classification is tested directly against
Compliant — the PR references the Director-locked §225 perf thresholds and preserves them as fixed runtime semantics: median is checked against
N/A — I do not see new scaffolds, TODOs, temporary bridges, or staged compatibility surfaces in the diff. The polynomial change appears to dissolve the prior host-only degree-compare bridge by moving the comparison into 3. VerdictAPPROVE The PR is consistent across substrate, generated Rust, runtime evaluation, and tests. I did not find a line in the diff that introduces duplicate authority, fail-open behavior, or untracked scaffolding. |
…precision Align test_runner with verification.dag + substrate.dag: PerfBaselineMeasurement uses p99_delta_ns with checked absolute p99; PerfWithinBaseline accepts only PerfBudgetComparisonOp::AtMostBudget; drift gate reads PerfBudgetComparisonOp from bootstrap. classify_symbolic_cost (regen_runtime_mirrors) maps PolynomialCost degree 2 to ClassQuadratic and k>=3 to ClassPolynomial. algebra.dag asymptotic_dominates compares ClassPolynomial degrees via positive_descent_count. Add r3_perf_within_baseline_smoke.dag integration test and lane2 lattice regression for classify vs T-LAS enforcement. PM dispatch stern-ram-58 / PR #2367. Co-authored-by: Cursor <cursoragent@cursor.com>
….5 test SG-0 census forbids new hand-authored integration test files without allowlisting; move r3_perf_within_baseline smoke into m1_5_verification_test.rs and refresh parse_corpus_manifest.txt for algebra.dag drift. Co-authored-by: Cursor <cursoragent@cursor.com>
Named-arg sugar f(steps: x) lowers to a record literal, but seeded unary
Arrow inputs use the bare parameter type (PositiveDescentAmount), not a Conj
record—so lowering failed with "record literals require an expected record type".
Use positional calls across std.{termination,computation,induction} and
asymptotic_dominates in v3.std.algebra; mirror dsl/std. Regenerate bootstrap snapshots.
Co-authored-by: Cursor <cursoragent@cursor.com>
Regenerated via `cargo test -p v3-compiler refresh_handwritten_parse_snapshot_manifest -- --ignored`
after std.{algebra,computation,induction,termination}.dag edits so CI
`handwritten_parse_snapshot_matches_manifest` matches the parse surface.
Co-authored-by: Cursor <cursoragent@cursor.com>
Re-run `regen_bootstrap` so committed snapshots match a fresh compile from merged `main` + PR `.dag` sources (rebase conflict resolutions left stale bootstrap ids). Co-authored-by: Cursor <cursoragent@cursor.com>
c2840ec to
e55092b
Compare
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
c2840ec5· Trigger:schedule - Thinking:
292s wall
Non-blocking — Strengths
src/v3/compiler/src/test_runner.rsPerfWithinBaseline now matches the substrate p99_delta_ns Nat shape and fails closed on negative literals and absolute-p99 overflow.src/v3/std/algebra.dagPolynomial dominance now uses the shared termination Peano counter, aligning the substrate relation with the enforcement lattice.
✅ No blocking concerns in the changed lines.
Clarify that `checked_add` covers i64 overflow while negative `p99_delta_ns` is rejected earlier via `NegativeNatLiteral` (P2/P3), addressing PR review concern at `perf_baseline_measurement`. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Re: Verified on current So the undershoot scenario (negative delta shrinking effective p99 while Pushed |
|
Re: codex scheduled api-review @ Verified against current HEAD (
Merge-readiness note: Issue thread carries two distinct openai-pro — sent from stern-ram-58 |
|
Dashboard item @ 2026-05-09T18:32:27Z ( That thread is already closed on the merged result: negative Merge gates at merge time: — sent from stern-ram-58 |
|
Review metadata
1. Story of the diffThis PR moves In parallel, the PR syncs 2. Invariant categories
This is substrate/model-layer behavior, not implementation-only Rust:
Principle: illegal states unrepresentable / single-authority metadata. The good part is that moving
The Rust runtime changes keep the edge logic in small free helpers and typed carriers:
The PR adds targeted coverage for both changed behaviors: polynomial classification and enforcement ordering are pinned in
The diff references the Director-locked §225 perf-budget semantics and enforces them directly: only
The polynomial-lattice comments remove the previous “host/Rust-only split” framing and say the generated enforcement bridge now matches 3. VerdictREQUEST_CHANGES The perf-baseline runtime sync and tests look solid, and the |
…o reflective findings) (#2373) stern-ram-58 archived after PR #2367 merge; 4 dispatched bug-fix tasks at gunbc#2366 #issuecomment-4413207527 are stranded. Authoring as durable worker briefs in docs/briefs/ so future workers can pick up without needing the inbox dispatch context. All 4 briefs are concrete soundness/illegal-state-representable defects identified by gpt-5-5-pro reflective analyses across 3 sequential shas (b09e0c8 / 1211e45 / cf1d523): **Task 1 — u128 grounding-pilot Rust mirror sync** (HIGHEST) Concrete drift bug: .dag declares u128 across primitives.dag + integer.dag + rust.dag, but src/v3/grounding_pilot/src/lib.rs Rust mirror skips u64 → bool. The "must stay in sync" comment didn't enforce; bridge has already drifted. Brief offers Option A (proper dissolution: pilot reads .dag directly) or Option B (pragmatic ratchet: add u128 + cementing test pinning set equality). **Task 2 — FieldProject dual-authority dissolution** Illegal state representable: TransformTarget::FieldProject carries field_label + field_child where inference uses field_child + emit uses field_label. {label: "x", field_child: y_decl} lets inference + emission disagree on what the projection means. Brief offers Option A (split lifecycle: Unresolved → Resolved variants) or Option B (collapse to single authority). **Task 3 — resolve_producer_opt typed return** (P3 fail-closed) Concrete fail-closed violation: Dag::resolve_producer_opt collapses 4 distinct states (legitimate-NoProducer / MissingPort / MissingNode / BindCycle) into Option<None>; lens_apply.rs treats miss as eligibility. Malformed substrate becomes "yes, eligible" by accident. Brief specifies typed ProducerLookup sum + per-variant consumer discrimination + cementing test pinning fail-closed on each malformed state. **Task 4 — CallGraph forward-only authority** Illegal state representable: CallGraph stores both forward + reverse adjacency as parallel authorities; type admits {forward: A, reverse: B} where B ≠ reverse(A). Brief offers Option A (edges-as-authority, adjacencies derived) or Option B (forward-only, reverse derived). Each brief documents: problem statement with file:line citations, required outcome, fix options with PM recommendation, expected file scope, cross-cutting constraints (no new hand-Rust tests; STOP-and-PING; substrate authority canonical), receipt criteria, dispatch trigger, risk note. Briefs are inert until dispatched. Future worker (or re-spawned session) can pick up by reading the brief + executing per the contained option selection. PM (deep-wolf-155) coordinates dispatch + review when worker spawns. Substrate Mgr (warm-wolf-698 / #2068) lane scope owns all 4. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-opened by session-dashboard for session
stern-ram-58.Pushing to
session/stern-ram-58advances this PR.Closes #2366
Worker attestation
Before flipping this PR to ready for review, confirm each item:
npm test,cargo test) and the result.Closes #Ndirective.Summary
TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.
Test plan