Skip to content

F5: deactivate v3 service parser authority - #1664

Merged
briansrls merged 107 commits into
mainfrom
session/stern-bee-22-coproduct-wire-contract
May 4, 2026
Merged

briansrls merged 107 commits into
mainfrom
session/stern-bee-22-coproduct-wire-contract

Conversation

@briansrls

@briansrls briansrls commented May 4, 2026 •

Copy link
Copy Markdown
Contributor

Choice

Deactivate v3 parser authority. This PR does not land ServiceBlock parsing/lowering; it keeps the shared/v2 DAG syntax fact for existing service declarations, while removing the unsupported v3 token/parse-table/parser dispatch that recognized top-level service only to hard-error.

What changed

  • Kept service live in the shared dsl/extdeps/languages/dag/syntax.dag authority consumed by v2/extdeps.
  • Removed KwService from the v3 token kind generation and top-level item dispatch tables.
  • Removed the generated/parser scaffold that recognized top-level service only to hard-error in v3.
  • Preserved service as an ordinary identifier in v3 name position.
  • Regenerated tokenizer, parse tables, parser, bootstrap/generated projections, and v2 stage0 after the main merge.
  • Updated SG-2 parse-table authority tests to ratchet the v3 deactivated top-level state and identifier-position behavior.

Validation

  • cargo fmt --all --check
  • CARGO_TARGET_DIR=target/stern-bee-22-stage0 cargo test -p v2-compiler-tests ci_freshness -- --ignored --nocapture
  • CARGO_TARGET_DIR=target/stern-bee-22-validate cargo test -p v3-compiler --test integration sg2c1_parse_tables_authority_test -- --nocapture
  • CARGO_TARGET_DIR=target/stern-bee-22-validate cargo test -p v3-compiler handwritten_parse_snapshot_matches_manifest -- --nocapture

briansrls added 30 commits May 3, 2026 14:30
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: b7cae7b8 · Trigger: schedule
  • Comparison: origin/main @ 7b415f1e ... review/pr-1664-b7cae7b8 @ b7cae7b8
  • Thinking: 105s wall

Findings

  • src/v2/stage0/src/extdeps_languages_dag_syntax.rs (post-change ~49–67, ~109–119, ~169–256, file ends ~275) — The checked-in “generated” module no longer matches the authoring module dsl/extdeps/languages/dag/syntax.dag on the same branch: dag_item_forms / dag_keyword_set drop the service rows even though syntax.dag still declares them (syntax.dag ~35–40, ~66–68), dag_operators drops the ?? (NullCoalesce) and % (Mod) rows even though syntax.dag still lists them (~101–117), and pub fn v3_supported_dag_operators() is removed while syntax.dag still defines data v3_supported_dag_operators (~127–132). That breaks P2: Boundary Discipline (declaration vs realization drift) and re-touches the R1 ?? / % authority story documented in ROADMAP.md / docs/debt/r3-debt-paydown-ledger-2026-05-02.md (full external operator table + explicit v3 subset). This looks like accidental damage during the service work, not an intentional, .dag-authored narrowing.

Verdict

REQUEST_CHANGES — The v3-facing deactivation of top-level service (parse_tables.dag, tokenize.dag, parse_parser_body.txt, tests, regen) is internally consistent with E-8 / fail-closed intent for unsupported surface syntax, but the v2 stage0 extdeps_languages_dag_syntax.rs edit must be corrected so it stays a faithful projection of syntax.dag (regen from the real v2 emitter, or revert that file to match origin/main unless syntax.dag is changed in the same PR to match).

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head 4436296. The blocking shared-syntax/stage0 findings were valid on the intermediate heads, but are now addressed: dsl/extdeps/languages/dag/syntax.dag keeps the shared/v2 service item form and keyword, and src/v2/stage0/src/extdeps_languages_dag_syntax.rs now mirrors that plus the existing ??/% operator rows and v3_supported_dag_operators projection. The v3-specific token/table/parser surfaces remain deactivated for top-level service until ServiceBlock support lands.\n\nValidation run locally on current head: cargo fmt --all --check; CARGO_TARGET_DIR=target/stern-bee-22-stage0 cargo test -p v2-compiler-tests ci_freshness -- --ignored --nocapture; CARGO_TARGET_DIR=target/stern-bee-22-validate cargo test -p v3-compiler --test integration sg2c1_parse_tables_authority_test -- --nocapture; CARGO_TARGET_DIR=target/stern-bee-22-validate cargo test -p v3-compiler handwritten_parse_snapshot_matches_manifest -- --nocapture.\n\n— sent from stern-bee-22

@briansrls briansrls changed the title F5: deactivate unsupported service syntax authority F5: deactivate v3 service parser authority May 4, 2026
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 44362960 · Trigger: schedule
  • Comparison: origin/main @ 671b3602 ... review/pr-1664-44362960 @ 44362960
  • Thinking: 28s wall

APPROVE — clean, narrow deactivation of v3's service keyword authority.

The change consistently removes KwService across the v3 substrate (tokenize.dag, parse_tables.dag) and its generated artifacts (tokenize_generated.rs, parse_tables_generated.rs, parse_generated.rs), drops the now-unreachable parse_service_item scaffold, and updates both authority tests (top_level_item_kw_rows_* and soft_keyword_ident_rows_*) plus the corpus manifest hashes for the two regenerated .dag files. The fail-closed behavior swaps cleanly: service Llm {} now errors via the standard top-level-item dispatch path because service tokenizes as Ident("service"), and the renamed test (...is_deactivated_until_service_block_ast_lands) asserts exactly that message and the corresponding soft-keyword test asserts the new Ident tokenization. The v2 stage0 extdeps_languages_dag_syntax.rs / dsl/extdeps/languages/dag/syntax.dag edits are pure alphabetical reorderings of ServiceBody/ResourceBody — no semantic change at the v2 layer, which is the correct seam since v2 still recognizes service.

No invariant violations observed in the diff. The scaffold removal even improves coproduct discipline (one fewer dead variant in ItemDispatchKind). The deactivation has a clearly named dissolution trigger ("until the ServiceBlock AST/lowerer slice lands"), which the test names and inline comments echo — tracked-debt, not a hidden scaffold.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: eec0f677 · Trigger: schedule
  • Comparison: origin/main @ 522fa3d3 ... review/pr-1664-eec0f677 @ eec0f677
  • Thinking: 53s wall

Findings: None. The diff removes KwService from the shared TokenKind taxonomy and tokenizer, drops the top_level_kw_service / soft_keyword_ident_service table rows, removes the dedicated parse_service_item arm, and routes top-level service … through ordinary Ident("service") + top_level_item_dispatch → None, which still produces a ParseError with an explicit expected-keyword list (parse_parser_body.txt around the updated parse_item match and None arm). That stays fail-closed (P3 / modeling-discipline §1) rather than silently accepting or fabricating surface structure. Consolidating lexical + table + parser authority on “no dedicated service keyword until the feature lands” aligns with boundary / single-authority discipline (P2) for this implementation seam, not a second competing keyword path.

Tests and SG-2c pins (sg2c1_parse_tables_authority_test.rs) were updated so row coverage and behavior match the new authority; parse_corpus_manifest.txt hash lines track the edited .dag sources per existing integration discipline.

Verdict: APPROVE — Narrowly scoped deactivation of v3 top-level service parser/token authority, consistent with the referenced docs; no concrete invariant or testing-discipline violation visible in the diff.

Exploratory (optional): soft_keyword_ident_service_still_parses_in_name_position asserts Ident("service") via format!("{:?}", t.kind); if TokenKind’s Debug ever changes shape, the test could flake without a semantic change. Low priority if you ever touch that test style.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: f7f92c73 · Trigger: schedule
  • Comparison: origin/main @ 522fa3d3 ... review/pr-1664-f7f92c73 @ f7f92c73
  • Thinking: 56s wall

Findings

  • NON-BLOCKING (TESTING.md — avoid implementation-detail assertions): In soft_keyword_ident_service_still_parses_in_name_position, new checks compare format!("{:?}", t.kind) to string literals such as "Ident(\"service\")" (src/v3/compiler/tests/integration/sg2c1_parse_tables_authority_test.rs, around lines 571–579 in the updated file). That pins TokenKind’s Debug layout, which TESTING.md calls out as brittle (same bucket as asserting on internal stringification). Prefer matches!(t.kind, TokenKind::Ident(ref s) if s == "service") (or an existing test helper) so the test keys off the public TokenKind shape, not Debug.

Verdict: APPROVE_WITH_COMMENTS

The diff consistently withdraws v3’s KwService / top-level service dispatch and the service soft-keyword row so the lexer and parse_tables.dag no longer treat service as a reserved form the parser cannot represent—top-level service still fails closed via the ordinary parse_item None arm (parse_parser_body.txt ~186–192), and the authority/ratchet tests were updated to match. Nothing here violates fail-closed, boundary discipline, or the modeling-doc scaffold rules in a way that needs a blocking fix; only the new Debug-string assertions are worth tightening when someone touches that test next.

@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: 44362960 · Trigger: schedule
  • Thinking: 309s wall

Non-blocking — Strengths

  • src/v3/compiler/tests/integration/sg2c1_parse_tables_authority_test.rs The v3 service deactivation is ratcheted by tests covering top-level rejection and identifier-position parsing.

✅ No blocking concerns.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: ef53b5d4 · Trigger: schedule
  • Comparison: origin/main @ b4203b5a ... review/pr-1664-ef53b5d4 @ ef53b5d4
  • Thinking: 76s wall

Verdict: APPROVE

The diff consistently removes KwService from the v3 tokenizer/parser dispatch path while leaving service-domain carrier files alone. I don’t see a concrete violation of the pinned modeling, coding, or testing discipline in the changed lines. Tests were not run, per review instructions.

@briansrls
briansrls merged commit b8c9821 into main May 4, 2026
4 checks passed
@briansrls
briansrls deleted the session/stern-bee-22-coproduct-wire-contract branch May 4, 2026 22:55
briansrls added a commit that referenced this pull request May 5, 2026
…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>
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.

1 participant