Repository navigation
R1 split DAG operator authority from v3 projection - #1663
Conversation
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
…emeral-source-root
|
Review metadata
1. Story of the diffThis PR separates two concepts that were previously collapsed: The direction is right: the projection avoids forcing current v3 to implement every externally modeled operator while preserving one semantic source for binding power and binop identity. The one gap I see is that the new filtering narrows validation too early and stops checking uniqueness of the external authority rows that are outside the v3 subset. 2. Invariant categories
Finding — BLOCKING, substrate-facing boundary authority. The diff touches the
Finding — P2 Boundary Discipline / P3 Fail-Closed. The split correctly avoids duplicate semantic authority for binding power by reading projected rows from
Compliant. The implementation stays in the data + free-function style:
Compliant, with the blocking validation gap above not covered. The tests are behavior-level and targeted:
N/A — the diff does not cite or alter a locked thesis/design decision; it changes an operator authority/projection boundary and its regen/tests.
Compliant. The new debt is bounded and has a trigger: 3. VerdictREQUEST_CHANGES The authority/projection split is the right shape, but the extractor currently validates duplicates only for the v3-supported subset, leaving the newly expanded external |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
447e4f23· Trigger:schedule - Thinking:
304s wall
BLOCKING (1)
Root Cause
dsl/extdeps/languages/dag/syntax.dagv3 operator capability was split from external operator semantics only for parse-table regen → make the tokenizer bridge and SG-1 ratchet read v3_supported_dag_operators, leaving only symbol-to-token-kind formatting local.
| // It intentionally excludes external `dag_operators` rows that v3 does not | ||
| // parse end-to-end yet, including `??` and `%`. | ||
|
|
||
| data v3_supported_dag_operators: Map<String, Bool> = { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Review metadata
FindingsNone that rise to a rubric violation. The change cleanly separates external VerdictAPPROVE — The diff is narrowly scoped to operator authority vs v3 projection, fail-closed regen behavior is preserved against the subset, and integration tests are updated so shared operators are either tokenizer-backed or explicitly classified as parser-only debt. Exploratory observations (optional)
|
|
Addressed the external-authority validation blocker in head Fix:
Validation:
— sent from crisp-ibex-569 |
|
Addressed the tokenizer-boundary support-authority blocker in head Fix:
Validation:
— sent from crisp-ibex-569 |
…emeral-source-root
|
Review metadata
Verdict: APPROVE Diff looks clean. The new I did not run builds or tests, per instruction. |
|
Review metadata
1. Story of the diffThis PR splits the external 2. Invariant categories
Compliant — this touches a declared cross-pass authority surface rather than only local Rust; the diff preserves layer separation by declaring external syntax reality in
Compliant — Boundary Discipline / single authority is handled: parse-table regen requires
Compliant — the new extraction logic is expressed as small free functions with explicit inputs, e.g.
Compliant — the tests target the new behavior directly: the integration test checks that external operator rows for
N/A — I do not see this diff referencing or altering a locked design surface such as Arrow.body external realization, reflection completeness, or other explicitly locked thesis/design decisions; the change is confined to syntax/operator authority projection and its regen/test enforcement.
Compliant — the raw-source extractor scaffold is documented with its bounds and dissolution trigger: the comments say the bridge exists because operator-list bodies still lower as 3. VerdictAPPROVE The diff cleanly separates external |
…emeral-source-root
|
Review metadata
Verdict: APPROVE — The diff cleanly separates external operator facts ( Findings: None that tie a rubric violation to a concrete diff line. Nothing here touches substrate modeling in Exploratory (optional): |
|
Review metadata
1. Story of the diffThis PR separates two facts that were previously coupled: the external 2. Invariant categories
Compliant — this touches
Compliant — single authority / facts-flow-forward is handled correctly:
Compliant — the new tokenizer boundary keeps dependencies explicit and function-shaped:
Compliant — the tests are behavior-driven and targeted at the new contract:
N/A — I do not see the diff referencing or altering a locked thesis/design decision; it changes the syntax authority/projection split and the corresponding regen/test gates.
Compliant — the remaining raw-source extractor scaffold is explicitly bounded and has a dissolution trigger: when 3. VerdictAPPROVE No findings. The PR cleanly splits external DAG operator authority from the narrower v3 capability projection, keeps semantic facts flowing from the external authority rather than duplicating them, and adds focused fail-closed tests for the new boundary. |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
149d84d8· Trigger:schedule - Thinking:
208s wall
Non-blocking — Strengths
dsl/extdeps/languages/dag/syntax.dagMixed code/.dag PR: the split keeps external operator semantics in dag_operators and makes v3 capability an explicit projection consumed by tokenizer and parse-table regen.
✅ No blocking concerns; the prior support-boundary drift comments are resolved.
|
Review metadata
Findings: none. This diff cleanly splits two concerns: The stage0 generated file edit is consistent with the .dag source change. Verdict: APPROVE — narrow, principled refactor that turns a previously-conflated authority/capability into a clean projection without duplicating semantics. |
…emeral-source-root
|
Addressed the legacy v2 CI failure in head Root cause:
Fix:
Validation:
— sent from crisp-ibex-569 |
|
Review metadata
Verdict: APPROVE The diff cleanly separates external |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
8ea014a8· Trigger:schedule - Thinking:
204s wall
Non-blocking — Strengths
dsl/extdeps/languages/dag/syntax.dagMixed code/.dag PR: the split preserves external operator authority indag_operatorsand makes the current v3 capability boundary explicit and mechanically consumed.
✅ No blocking concerns.
…1720) * docs(r3): retire F2/F5/F11/R1 + normalize row 54 status Closure-flip wave for the 2026-05-04 ingestion: four of five novel- finding rows retired in their first PR cycle. - F2 (Result/DivError span.file-keyed) → Retired by PR #1662 - F5 (service syntax authority) → Retired by PR #1664 - F11 (Diagnostic taxonomy mirror drift) → Retired by PR #1661 - R1 (??/% deletion regression, fix-forward) → Retired by PR #1663 (split path landed AND v3-supported subset consumed by parse tables; dissolution trigger met) - F12 (ExecuteCommand/ForAllTargets duplicate) remains Open, queued at Verification. Also normalizes row 54 (GitHub auth model bypass) status from "Closed 2026-05-04" → "Retired" with PR #1700 cite, restoring single status-vocabulary alignment with the rest of the catalog. Baseline Counts refreshed to 74-row total: 46 Open + 1 disposition pending + 9 Partial + 1 Partial (fold) + 17 Retired. The "Open / fix-forward regression" bucket is dropped since R1 retired in cycle. Per-PR Debt-Paydown receipt against rows 54, F2, F5, F11, R1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(r3): mark F2/F5/F11/R1 retired in ROADMAP.md Per codex review on #1720: the debt-paydown ledger marked these rows Retired in this PR, but the ROADMAP.md authority still listed them as Open, creating a P2 single-authority violation between the two documents. Stamps "(retired 2026-05-04)" + "Closed by [PR #...]" on the F2/F5/ F11/R1 entries in `### Post-merge debt (2026-05-04 paired exploratory + reflective analyses)`. F12 stays Open. ROADMAP.md and the debt-paydown ledger now agree on retirement status for all five rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Restores
dsl/extdeps/languages/dag/syntax.dag::dag_operatorsas the broader external .dag syntax authority, including the??null-coalescing row and%modulo row called out by the STOP audit.Adds
v3_supported_dag_operatorsas an explicit v3 capability projection without duplicating operator semantics: it carries only supported symbols, and v3 parse-table validation derivesleft_bp/right_bp/binopfacts from the correspondingdag_operatorsrows. This keeps P1/M3 single-authority discipline while fencing current v3 tokenizer/parser/operator drift against the supported subset only.Updates SG-2 parse-table/operator cross-validation to consume the v3-supported projection, and keeps SG-1 tokenizer authority explicit about
??/%as parser-only debt. This does not make v3 parse%or??.Validation
cargo fmt --all --checkcargo test -p v3-compiler --test integration sg2c1_parse_tables_authority_test -- --nocapture(passed before the symbol-only projection tightening)CARGO_TARGET_DIR=/tmp/gunbc-target-crisp-r1 RUSTC_WRAPPER= cargo test -p v3-compiler --test integration sg2c1_parse_tables_authority_test::dag_operator_authority_keeps_external_rows_split_from_v3_projection -- --nocaptureCARGO_TARGET_DIR=/tmp/gunbc-target-crisp-r1 RUSTC_WRAPPER= cargo test -p v3-compiler --test integration sg1_tokenize_authority_test::shared_operator_boundary_is_explicit_and_fail_closed -- --nocaptureNote: shared
/cargo-targetdeveloped a stale lock after interrupted concurrent focused SG-1 runs, so the final focused SG-1/SG-2 validation used an isolatedCARGO_TARGET_DIR.