enforce_violation_routing_landed

[briansrls]

Auto-opened by session-dashboard for session neat-dove-545.
Pushing to session/neat-dove-545 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

• Title describes the change (not the session id or branch).
• PR body summarises what and why (replace the TODO below).
• Tests run: name the command (e.g. npm test, cargo test) and the result.
• If this closes a work item, the body contains a Closes #N directive.
• No commits on this branch are surprises (no fork/cherry-pick I did not make).
• No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

• TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

[briansrls]

Director conformance read — would-approve.

Read against INVARIANTS.md (P1 modeling-faithfulness + C-8 fail-closed) + design-lens-application-surface §10 step 2 + gate #91 thesis.

Conformance citations:

1. Gate Untangle refactor: restore behavioral keywords, add obligation invariants #91 consumer-side landing is the canonical Slice B shape: per §1.8 row 91 'CONSUMER_LANDED requires the fold-pass consumer per design doc §10 step 2 — deferred to Slice B'. This PR is that fold-pass consumer for the complexity-enforce-violation path. Substrate routing surface (from PR Substrate T-LAS Slice A: 88-91 carrier + routing landings #2145) is now consumed; the substrate-vs-consumer pair is complete.

2. P1 modeling-faithfulness: consumer READS the authored diagnostic_severity field from the EnforcedApplication declaration rather than hardcoding 'Error' at the consumer tier. The substrate is the authority; the fold pass is faithful to that authority. Adding a new DiagnosticSeverity variant in substrate would naturally extend the routing (today fails closed; future ratchet extends behavior).

3. C-8 fail-closed discipline preserved across 3 boundary classes:

   • Missing diagnostic_severity field → emit ParseError 'missing diagnostic_severity'
   • Wrong type (not a variant value) → emit ParseError 'must be a DiagnosticSeverity variant value'
   • Wrong variant (not Error) → emit ParseError 'must be Error (INVARIANTS C-8; fail-closed discipline)'

   Each boundary class is named in the diagnostic message; reviewer can verify substrate drift won't silently degrade routing.

4. No fail-open class introduced: the substrate currently constrains DiagnosticSeverity to a single Error variant, so the wrong-variant arm is unreachable today. But the consumer code is structured to handle additional variants by fail-closing — when a future PR adds a Warning variant, this consumer's behavior is explicit and the change requires intentional substrate + consumer co-update.

5. Internal-error path: if DiagnosticSeverity disjunction can't be looked up OR lacks the Error constructor, consumer emits 'internal error' diagnostics rather than silently falling through. Defensive against substrate-tier drift; reviewable signal.

6. Architectural alignment with design §3 (enforce-mode violation routing through DiagnosticSeverity) + lens_application.dag substrate (DiagnosticSeverity variant + EnforcedApplication.diagnostic_severity field). No invented intermediate types; all substrate is pre-declared.

7. Net +86 lines in single file (94+/8-): all additive. The 8-line removal is the prior hardcoded ParseError construction replaced by the routed enforced_violation_diagnostic helper. Concentrated change; small blast radius.

No INVARIANTS violations found. The fold-pass consumer is exactly the shape the substrate routing surface was designed to enable. Worth promoting gate #91 from DECLARED to CONSUMER_LANDED in §1.8 on land.

— sent from zesty-bear-812

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-5-thinking
• Commit: 280401bd · Trigger: manual
• Comparison: main @ e35bc1a0 ... session/neat-dove-545 @ 280401bd
• Conversation: View conversation

---

1. Story of the diff

This PR changes src/v3/compiler/src/enforced_lens_application.rs so complexity-budget enforcement no longer emits the violation diagnostic directly. Instead, when an EnforcedApplication violates its budget, the compiler reads the authored diagnostic_severity field and routes the violation through a new helper, enforced_violation_diagnostic, which currently accepts only DiagnosticSeverity::Error and maps it to Diagnostic::ParseError (src/v3/compiler/src/enforced_lens_application.rs:175-201, :208-260). That is the right shape for the landed slice: the routing decision is tied to the modeled field, but the steady-state accepted severity remains fail-closed and compile-failing.

The load-bearing issue is that the new code first discovers the DiagnosticSeverity declaration by scanning declarations for a name and source-file suffix, then silently returns if that authority is missing (src/v3/compiler/src/enforced_lens_application.rs:71-81). Because this function is the enforcement pass, that return can suppress the entire enforcement check rather than reporting a malformed/missing modeled authority.

2. Invariant categories

1. LAYER MODEL (substrate vs implementation).

Finding — BLOCKING. This is implementation Rust, not a substrate-type addition, but it consumes a substrate-declared fact. The new lookup makes DiagnosticSeverity absence a silent no-op: else { return; } at src/v3/compiler/src/enforced_lens_application.rs:80. For a substrate-backed enforcement path, missing modeled authority must fail closed, not disable enforcement. INVARIANTS P3 says every path succeeds fully or fails with a typed diagnostic, and C-8 is the canonical fail-closed compilation rule. chatgpt-review-cf159455-2f43-4a…

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

Finding — BLOCKING. Principle: fail-closed / facts flow forward. The helper itself mostly honors C-8 by returning Diagnostic::ParseError for malformed severity shapes and non-Error constructors (src/v3/compiler/src/enforced_lens_application.rs:208-260), but the declaration lookup added just before enforcement returns silently when DiagnosticSeverity is unavailable (src/v3/compiler/src/enforced_lens_application.rs:71-81). Modeling discipline explicitly treats silent failure/absence as a violation: failures should go through diagnostics, not disappear as None/return. chatgpt-review-467e14c5-6f4f-4d…

1. CODING.md.

Compliant. The new enforced_violation_diagnostic is a free function over explicit inputs (&Dag, declaration id, field value, message, span) and returns a structured Diagnostic, matching the data + functions style; no hidden global state or method accretion is introduced.

1. TESTING.md.

Finding — NON-BLOCKING if the fail-closed bug is fixed with an existing covered path; otherwise BLOCKING. The diff adds no test file or test hunk. The changed behavior has several meaningful branches: missing diagnostic_severity, malformed non-variant severity, non-Error constructor, and successful budget-violation routing. TESTING.md asks new tests to name the interface and behavior and to pin behavior rather than implementation; this is exactly the kind of enforcement behavior that should get a focused regression. chatgpt-review-01471593-2d95-44…

1. LOCKED DESIGN DECISIONS.

Compliant. The PR does not alter a locked design doc. Its runtime behavior is aligned with the thesis-level direction that correctness dimensions are structural facts and validation reads modeled structure, not ad hoc behavioral checks. chatgpt-review-d3699412-0c0f-44…

1. TRACKED vs UNTRACKED DEBT.

Compliant. No new TODO, scaffold, bridge, or temporary file is introduced in the diff. The added module comment states the current single-variant policy and gate intent rather than creating a new tracked-debt surface (src/v3/compiler/src/enforced_lens_application.rs:1-11).

2.5. Top-down PM intent review

Finding — BLOCKING. PM-level intent is that complexity/correctness dimensions are structural facts validated by the compiler, with compile-time proofs closing by reading structure; the thesis also names suboptimal-complexity contract violation as an R1 impossible-bug class. chatgpt-review-d3699412-0c0f-44…

The diff’s intended routing supports that by reading diagnostic_severity, but src/v3/compiler/src/enforced_lens_application.rs:80 turns the absence of the DiagnosticSeverity authority into “do not run this pass.” A worker following this implementation faithfully could land a compiler that accepts a program with a violated enforceable budget whenever that declaration lookup fails, which dilutes the PM intent from “structural enforcement” to “best-effort enforcement depending on an unchecked lookup.”

3. Verdict

REQUEST_CHANGES. The routing helper is directionally right, but the new declaration lookup has a silent fail-open path on the exact enforcement mechanism this PR is supposed to harden. Fix DiagnosticSeverity lookup failure to emit a typed diagnostic/fail closed, then add a focused regression for the routing and malformed/missing severity cases.

[briansrls]

Review metadata

• Provider / model: openai-pro / gpt-5-5-pro
• Commit: 6b40f032 · Trigger: manual
• Comparison: main @ 987d6986 ... session/neat-dove-545 @ 6b40f032
• Conversation: View conversation

---

1. Story of the diff

This PR narrows src/v3/compiler/src/enforced_lens_application.rs around Gate #91: instead of every complexity-budget enforcement violation directly becoming a Diagnostic::ParseError, violations now read the authored EnforcedApplication.diagnostic_severity field and route through enforced_violation_diagnostic. The implementation adds a substrate lookup for DiagnosticSeverity from lens_application.dag, fails closed if that substrate row is missing, then checks the value on each violating enforced application before emitting the budget-exceeded diagnostic. The added unit tests cover the missing-substrate case, a wrong severity constructor, and a non-variant severity value. The shape is directionally right for single-authority routing, but the new value checker does not fully validate the variant payload it accepts.

2. Invariant categories

1. LAYER MODEL (substrate vs implementation).

Compliant with caveat — this diff does not add or reshape a substrate type on Dag; it is implementation-side Rust consuming an existing substrate field. The consumer now resolves DiagnosticSeverity from lens_application.dag at src/v3/compiler/src/enforced_lens_application.rs:27 and refuses to continue when that substrate row is missing at src/v3/compiler/src/enforced_lens_application.rs:100, which is the right layer direction.

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

Finding — Fail-Closed / malformed value validation. The helper documents that “unknown constructors or malformed values fail closed” at src/v3/compiler/src/enforced_lens_application.rs:232, but the actual match discards the variant payload with .. at src/v3/compiler/src/enforced_lens_application.rs:258 and then accepts solely on constructor equality at src/v3/compiler/src/enforced_lens_application.rs:267. That means a malformed FieldValue::Variant { constructor: Error, payload: non_empty } is treated as a valid DiagnosticSeverity::Error and returns the normal violation diagnostic at src/v3/compiler/src/enforced_lens_application.rs:276. The fix should validate the payload shape for the Error constructor — currently likely empty/nullary — and fail closed if it does not match.

1. CODING.md.

Compliant — the new behavior is factored into small free functions rather than methods or hidden state: diagnostic_severity_substrate_disj at src/v3/compiler/src/enforced_lens_application.rs:27, attach_missing_diagnostic_severity_substrate_diagnostic at src/v3/compiler/src/enforced_lens_application.rs:40, and enforced_violation_diagnostic at src/v3/compiler/src/enforced_lens_application.rs:234.

1. TESTING.md.

Finding — missing regression for the malformed payload path. The added tests cover missing substrate resolution at src/v3/compiler/src/enforced_lens_application.rs:489, wrong constructor at src/v3/compiler/src/enforced_lens_application.rs:523, and non-variant values at src/v3/compiler/src/enforced_lens_application.rs:560, but they do not cover the malformed Error constructor with an invalid/non-empty payload that the checker currently accepts because of src/v3/compiler/src/enforced_lens_application.rs:258. Add a focused unit test for FieldValue::Variant { constructor: error_ctor, payload: vec![...] } expecting a fail-closed diagnostic.

1. LOCKED DESIGN DECISIONS.

N/A — the diff does not edit locked design docs or alter a locked substrate decision; it implements the existing Gate #91 routing contract inside one Rust module.

1. TRACKED vs UNTRACKED DEBT.

N/A — no new TODO, scaffold, temporary bridge, or added hand-authored file path appears in the diff. The “today the single substrate variant” wording at src/v3/compiler/src/enforced_lens_application.rs:4 describes current policy rather than introducing a deferred migration.

2.5. Top-down PM intent review

Finding. The PR’s own stated intent is that Gate #91 routes budget-violation diagnostics through the authored diagnostic_severity field and remains compile-fail / fail-closed at src/v3/compiler/src/enforced_lens_application.rs:3. The implementation mostly does that, but the payload-discarding match at src/v3/compiler/src/enforced_lens_application.rs:258 semantically dilutes “fail-closed” by allowing a malformed Error variant value to pass as if it were the authored severity. That is small to repair, but it matters because the PR exists specifically to land violation-routing enforcement.

3. Verdict

REQUEST_CHANGES. The routing mechanism is mostly in the right place and the tests are close, but the new enforcement boundary currently accepts a malformed DiagnosticSeverity::Error value instead of failing closed. Tighten payload validation and add the missing unit test, and this should be ready.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 453b4651 · Trigger: schedule
• Thinking: 507s wall

---

✅ The diff routes enforce-mode violations through DiagnosticSeverity and preserves fail-closed behavior; no blocking concerns.