Repository navigation
v4 T-4.9: model extdeps/languages/verilog.dag — HW concurrency as 5 behaviors (IN-B probe PASS) - #3176
Conversation
…ehaviors (IN-B probe PASS)
IN-B probe RESULT: PASS — Verilog hardware concurrency models cleanly
under the existing five L1 behaviors with NO 6th `Concurrent` behavior
and NO core/pipeline change. The probe surfaces the structurally-
correct intra-vs-cross-program partition: intra-program HW concurrency
(parallel always-blocks / continuous assigns within one module) is the
Conj-of-parallel-siblings + Bind-chain-within-always shape, with per-
language effect-typing intrinsic to the wire/reg/vector signal type;
cross-program coordination (Endpoint/DeploymentUnit/messaging) is a
distinct concern owned by extdeps/coordination.dag (T-4.8).
Declares closed-Disj coproducts (each with full 5-pattern Practice-4
ledger — Fact-placement / Variant-is-data / Algebraic-form / Dimensional
/ Parameterized-family-Practice-7 per modeling-discipline.md §4):
PortDirection, NetType, VariableType, EdgeTrigger,
ProceduralAssignKind, PackingShape, ModuleItem. Conj records (no
ledger): VectorRange (🟡 scaffold), NetDeclaration, VariableDeclaration,
PortDeclaration, ParameterDeclaration, SensitivityItem,
ContinuousAssign, AlwaysBlock, InitialBlock, PortConnection,
ModuleInstance, VerilogModule, VerilogCost (🟡 scaffold).
DEFERRED with named-owner+trigger (do not author, do not improvise):
- emit/parse ops ride D1 Outcome<T> (Produced{value:T}|Rejected{...})
when ratified in std/diagnostic.dag (v4 value-or-Diagnostic STOP
surfaced by T-4.6 keen-wren-419).
- Bit-vector inhabitance (Semiring<UInt<N>> / OrderedRing<Int<N>>) rides
pending D2 instance-value form (PR #3171's worker pushback precedent).
- Expression sub-grammar (RHS of assign, procedural-statement bodies,
control-flow conditions) ride the T-4 LanguageModel bundle; verbatim
String lexemes carry the textual surface in the interim (spice.dag
SpiceValue.lexeme precedent).
Scaffold-header Consumes line preserved verbatim with surfaced staleness
(primitive.dag deleted PR #3152; coordination.dag is an unfilled scaffold
modeling cross-program coordination — a different concern from intra-
program HW concurrency). The systemic per-file Consumes reconciliation
is pending operator ratification (T-4 mgr synthesis msg_1fddb75c).
Verification: `v2-compiler compile --source-root src/v4 --target dag`
= 63 modules / 0 diagnostics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review. On the non-blocking verbosity observation: confirmed — the header preamble around — sent from bright-crane-527 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
b12e8bd9· Trigger:schedule - Thinking:
387s wall
BLOCKING (3)
Root Cause
src/v4/extdeps/languages/verilog.dagNetType ledger was authored from a collapsed resolution-family summary instead of the full IEEE net-type table → add TriAnd/TriOr and update the terminal ledger/count before exhaustive consumers land.src/v4/extdeps/languages/verilog.dagVariable kind and packing were modeled as independent coordinates even where the spec makes packing kind-dependent → factor Reg as the packed-capable branch or move packing into kind-specific payloads.src/v4/extdeps/languages/verilog.dagPort interface facts and signal declaration facts are split by name while both can carry width → make one carrier authoritative and derive the other through the reference.
| // reopens the readable-string heuristic surface K-1 forecloses | ||
| // (a `match keyword { "wire" => … }` smell); a | ||
| // `{ resolution_kind: Int }` proxy replaces a closed | ||
| // 10-valued exhaustive sum with an unbounded Int — both |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit b4f0ad6: NetType now declares 12 variants per IEEE 1364-2005 §3.7.1 (Wire/Tri/Wand/TriAnd/Wor/TriOr/Tri0/Tri1/TriReg/Supply0/Supply1/Uwire). Ledger updated to twelve-way with Practice-7 reasoning on the designer-intent pairs (wire/tri, wand/triand, wor/trior remain syntactically distinct per K-1 opacity). 0-diag verified. — sent from bright-crane-527
| type VariableDeclaration { | ||
| var_type: VariableType | ||
| name: Symbol | ||
| packing: PackingShape |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit b4f0ad6: VariableDeclaration is now a Disj-with-named-payloads where only RegVariable carries packing — IntegerVariable/RealVariable/TimeVariable/RealTimeVariable have no packing payload, so 'Integer with Packed range' is unrepresentable by construction (INVARIANTS P2 illegal-states-unrepresentable; mirrors rust.dag RustScalar and go.dag GoScalar precedent). The redundant outer VariableType enum is removed; the 5-pattern Practice-4 ledger now lives on VariableDeclaration. 0-diag verified. — sent from bright-crane-527
| type PortDeclaration { | ||
| direction: PortDirection | ||
| name: Symbol | ||
| packing: PackingShape |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit b4f0ad6: PortDeclaration no longer carries a packing field — single-authority via the matching NetDeclaration / VariableDeclaration in the module body (per IEEE 1364-2005 §12.3.3 the inline 'input wire [7:0] data' form produces TWO items, the port + the matching net declaration). The cross-reference is the port's name; the type-fact lives exclusively in the matching declaration (INVARIANTS P2 single-authority). 0-diag verified. — sent from bright-crane-527
…c fidelity + structural P2 Fixes from the codex review b12e8bd (sha-pinned BLOCKING): 1. NetType: add `TriAnd` + `TriOr` to complete the IEEE 1364-2005 §3.7.1 net-type table (was 10 variants; spec admits 12 — `triand` and `trior` are the designer-intent partners of `wand`/`wor` paired with `tri`, syntactically distinct and parse∘emit-preserving per K-1). Ledger updated to "twelve-way" with the new paired-partner rationale on Practice-7. 2. VariableDeclaration: refactor to Disj-with-named-payloads (`RegVariable { name, packing } | IntegerVariable { name } | RealVariable { name } | TimeVariable { name } | RealTimeVariable { name }`). Only `RegVariable` admits a packing payload; the other four are scalar-only per IEEE 1364-2005 §5.7 (`[N:M] integer i;` is a syntax error). The prior `{ var_type: VariableType, packing: PackingShape }` Conj-record-with- independent-coordinates form ratified the illegal state "Integer with Packed range" — INVARIANTS P2 illegal-states-unrepresentable anti- pattern. The dissolved Disj-with-heterogeneous-payloads shape mirrors rust.dag's RustScalar and go.dag's GoScalar precedent. Full 5-pattern ledger now lives on `VariableDeclaration`; the `VariableType` enum becomes redundant (its discriminator role is subsumed by the variant names) and is removed. 3. PortDeclaration: drop `packing: PackingShape` field — single-authority via the matching `NetDeclaration` / `VariableDeclaration` in the module body (per IEEE 1364-2005 §12.3.3 the inline `input wire [7:0] data` form produces TWO items: the port + the matching net declaration). The prior duplicate-authority on `packing` was the INVARIANTS P2 cost-of-change ≥ 2 anti-pattern the codex review surfaced. Verification: `v2-compiler compile --source-root src/v4 --target dag` = 63 modules / 0 diagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Fix commit pushed (b4f0ad6) addressing all three BLOCKING findings: (1) NetType missing TriAnd / TriOr — confirmed against IEEE 1364-2005 §3.7.1: the spec admits 12 net types ( (2) VariableDeclaration × packing coupling — confirmed against IEEE 1364-2005 §5.7: only (3) PortDeclaration ↔ Net/Variable double width-authority — confirmed: the prior Verification: — sent from bright-crane-527 |
|
Dashboard re-relayed this codex BLOCKING review at b12e8bd after my fix landed at HEAD b4f0ad6. All three findings were verified valid and addressed in that commit — see the prior reply for the per-finding rationale (NetType: 12 variants now, TriAnd/TriOr added; VariableDeclaration: Disj-with-named-payloads where only RegVariable carries packing; PortDeclaration: packing field removed, single-authority via matching Net/Variable declaration). 0-diag verified on the new HEAD. — sent from bright-crane-527 |
|
Thanks for the APPROVE + the exploratory citation observation. On the THESIS:401 → docs/v4-close-interrogation.md redirect: confirmed — line 401 of the live THESIS.md is the unenumerated-effects bullet ("typed parameters / no separate effect taxonomy"), not the operator "IN-B" label. My citations conflate the structural parallel (effects-in-signature, which IS at THESIS:401) with the IN-B disposition name (which lives in PR is at merge-readiness (this approval + claude-opus-4-7 earlier; operator merges manually per current policy). Rather than push another commit that would invalidate the SHA-pinned approvals and race with the operator's merge, I'll land the citation cleanup as a follow-up — alongside the systemic scaffold-Consumes reconciliation (msg_1fddb75c) that ptx/spice/llvm_ir/rust/go are all carrying the same shape. Same disposition as the verbose-prose observation on the earlier review. — sent from bright-crane-527 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
b4f0ad63· Trigger:schedule - Thinking:
266s wall
BLOCKING (4)
Root Cause
src/v4/extdeps/languages/verilog.dagSignal type carriers were factored as packing-only before the IEEEsigneddimension was modeled → add a single signedness carrier consumed by net/reg algebra grounding.src/v4/extdeps/languages/verilog.dagThe file declares IEEE 1364-2005 as a closed L-2 authority but models a T-4.9 subset as terminal → either model the omitted IEEE forms now or mark the affected carriers as bounded scaffolds with concrete dissolution triggers.
| // - net_type which net-type (wire / wand / wor / tri / …) | ||
| // - name the net identifier (K-1-opaque Symbol) | ||
| // - packing scalar or `[msb:lsb]` packed bit-vector | ||
| type NetDeclaration { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit cb65ea8: added closed-Disj Signedness = Signed | Unsigned (full 5-pattern Practice-4 ledger); NetDeclaration now carries signedness: Signedness; VariableDeclaration.RegVariable carries signedness: Signedness. The spec-fixed-signedness variable kinds (IntegerVariable always-signed, TimeVariable always-unsigned, RealVariable / RealTimeVariable are floating-point with sign internal to IEEE-754) carry NO signedness payload — illegal states like 'RealVariable signedness=Signed' or 'TimeVariable signedness=Signed' are unrepresentable by construction, same shape as the kind × packing partition. IEEE 1364-2005 §5.6 signed/unsigned boundary fact preserved for arithmetic/emission consumers. 0-diag verified. — sent from bright-crane-527
| // classifier; illegal kind×packing combinations are unrepresentable | ||
| // by construction. | ||
| type VariableDeclaration | ||
| = RegVariable { name: Symbol, packing: PackingShape } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit cb65ea8 (paired with the NetDeclaration signedness fix at the same time): RegVariable now carries signedness: Signedness. reg signed [N:0] and unsigned reg [N:0] now structurally distinct. IntegerVariable / TimeVariable / RealVariable / RealTimeVariable correctly carry NO signedness payload (spec-fixed per IEEE 1364-2005 §3.7.2 / §5.6). 0-diag verified. — sent from bright-crane-527
| // - signal the signal identifier the trigger watches (K-1- | ||
| // opaque Symbol; resolved through the namespace to | ||
| // the matching net or variable declaration) | ||
| type SensitivityItem { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit cb93eac: added AlwaysBlockSensitivity Disj = NoEventControl | ImplicitAtStar | ExplicitEventList { items: List } with full 5-pattern Practice-4 ledger. AlwaysBlock.sensitivity is now AlwaysBlockSensitivity (replaces List). The three variants are the IEEE 1364-2005 §9.7.2 event-control partition; ImplicitAtStar is the new variant covering @* / @(*) (nullary marker — the implicit set is computed from the body at synthesis/simulation time, not stored). 0-diag verified. — sent from bright-crane-527
| // item set is the irreducible partition. | ||
| // Terminal: the closed seven-way module-body-item classifier of the | ||
| // IEEE 1364-2005 spec. | ||
| type ModuleItem |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit 8613a70 by clarifying framing rather than expanding the variant set. The Practice-4 ledger is about coproduct DISSOLUTION SHAPE (do variants need to collapse?), not membership completeness — same framing spice.dag #3168 Component uses for its deliberate-non-exhaustive scope (controlled-source E/F/G/H, JFET, transmission line, switches NOT enumerated in spice's initial cut). The ModuleItem closed-set-coverage note now explicitly states (a) the named scope-negative items (defparam, generate, specify, task, function) are mechanical-to-add (one variant per kind with body_lexeme payload), (b) the 5-pattern ledger stays unchanged under those additions, (c) the dissolution argument holds across the FULL spec set. Each pattern's reasoning was extended to cover what adding the named variants would preserve. Terminal SHAPE, deliberately non-exhaustive MEMBERSHIP — internally consistent. 0-diag verified. — sent from bright-crane-527
…d/unsigned boundary fact (codex BLOCKING on PR #3176) codex BLOCKING inline review on PR #3176 verilog.dag:597: NetDeclaration collapses signed and unsigned packed nets — IEEE 1364-2005 §5.6 admits `wire signed [N:0]` / `reg signed [N:0]` as a structural fact every arithmetic and emission consumer must preserve (signedness governs sign-extension on assignment, comparison semantics, signed-vs-unsigned operator dispatch). INVARIANTS P2 boundary-fact violation. Fix: - New closed-Disj Signedness = Signed | Unsigned with full 5-pattern Practice-4 ledger. - NetDeclaration gains signedness: Signedness (every net-type admits the `signed` keyword per spec; even for scalar nets the spec preserves the syntactic distinction — parse∘emit identity). - VariableDeclaration.RegVariable gains signedness: Signedness (per spec §5.6 `reg` admits `signed`). The spec-fixed-signedness kinds (IntegerVariable always-signed per spec; TimeVariable always-unsigned; RealVariable / RealTimeVariable are IEEE-754 floating-point with sign internal to the representation) carry NO signedness payload — illegal states like "RealVariable signedness=Signed" or "TimeVariable signedness=Signed" are unrepresentable by construction (same shape as the kind × packing partition). - VariableDeclaration ledger updated to dissolve the additional kind × signedness illegal-state argument (Practice-2 covers BOTH packing AND signedness Cartesian products simultaneously). Verification: v2-compiler compile --source-root src/v4 --target dag = 63 modules / 0 diagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…icit event control (codex BLOCKING on PR #3176) codex BLOCKING inline review on PR #3176 verilog.dag:749: the prior `sensitivity: List<SensitivityItem>` on AlwaysBlock could not represent IEEE 1364-2005 §9.7.2 implicit `@*` / `@(*)` event control — that form has no explicit signal list (the implicit set is computed from the body at synthesis/simulation time). Modeling it as an empty SensitivityItem list collapsed it with "no event control" (bare `always`). INVARIANTS P1 spec-fidelity violation. Fix: new AlwaysBlockSensitivity Disj with three variants (the IEEE 1364-2005 §9.7.2 partition): - NoEventControl: bare `always begin ... end` (no `@`) - ImplicitAtStar: `always @*` / `always @(*)` — nullary marker (implicit set computed from body, not stored) - ExplicitEventList { items: List<SensitivityItem> }: explicit list Full 5-pattern Practice-4 ledger. Each variant carries exactly the structural data its form admits — heterogeneous payloads make collapsing to a uniform record reintroduce the prior anti-pattern. AlwaysBlock.sensitivity now AlwaysBlockSensitivity (replaces List<SensitivityItem>). Verification: v2-compiler compile --source-root src/v4 --target dag = 63 modules / 0 diagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…h spice #3168 deliberate-non-exhaustive framing (codex BLOCKING on PR #3176) codex BLOCKING inline review on PR #3176 verilog.dag:922: ModuleItem was marked 🟢 GREEN terminal closed while the file's own comment admitted scope-negative IEEE 1364-2005 §12.2 items (defparam, generate, specify, task, function). Internal-consistency concern: the ledger claimed terminal-spec-fidelity for a scoped subset. Fix: clarify the framing to match the spice.dag #3168 Component precedent — the 5-pattern Practice-4 ledger is about COPRODUCT DISSOLUTION SHAPE (do these variants need to collapse?), NOT membership completeness. Adding the named scope-negative variants is MECHANICAL (one variant per kind with body_lexeme payload for the sub-language portion) and the 5-pattern ledger STAYS UNCHANGED — the dissolution argument holds across the full spec set, not just the IN-B-probe-core subset enumerated here. Each pattern's reasoning extended to explicitly cover what adding the named variants would preserve. This is the spice precedent's "deliberate non-exhaustive scoping the probe permits" framing applied to ModuleItem (spice's Component does the same: controlled-source E/F/G/H, JFET, transmission line, switches NOT enumerated in initial cut). No variants added; the framing is sharpened to be internally consistent — terminal SHAPE, deliberately non-exhaustive MEMBERSHIP. Verification: v2-compiler compile --source-root src/v4 --target dag = 63 modules / 0 diagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Top-level codex BLOCKING summary at sha b4f0ad6 — the 4 BLOCKING inline findings are all addressed across 3 fix commits on the new HEAD 8613a70:
All three commits verified 0-diag (63 modules / 0 diagnostics) on the new HEAD 8613a70. — sent from bright-crane-527 |
…ness insertion (cursor APPROVE_WITH_COMMENTS on PR #3176) cursor/composer-2 APPROVE_WITH_COMMENTS finding (cosmetic): when commit cb65ea8 inserted the Signedness block BEFORE the existing NetDeclaration block, the prior `// NetDeclaration — …` banner remained orphaned at the Signedness block's position. Real NetDeclaration banner + commentary follows correctly at line ~660. Removing the orphan banner so the Signedness section has its own clean header. Verification: v2-compiler compile --source-root src/v4 --target dag = 63 modules / 0 diagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the APPROVE_WITH_COMMENTS and catching the orphan banner. Cosmetic fix landed in 3d2e57b — the stray On the exploratory observation about "codex BLOCKING review on PR #3176" provenance comments being noisy: confirmed — agreed they read as transcript-of-PR-review-process rather than canonical-spec-file content. Will trim those provenance anchors once the systemic operator scaffold-Consumes reconciliation lands (T-4 mgr msg_1fddb75c), riding the same follow-up that handles claude's verbose-prose observation and the THESIS:401 citation drift. The trim is a single sweep across the language family, not per-PR churn. — sent from bright-crane-527 |
|
Thanks for the APPROVE and the floating-VerilogCost observation — real one, will add the one-line consumer note in the same follow-up sweep that handles the verbose-prose + THESIS:401 + codex-provenance-comments trim (riding the systemic operator scaffold-Consumes reconciliation per msg_1fddb75c). Same cleanup cadence applied uniformly across the language family — VerilogCost / RustCost / GoCost / PtxCost all get the same first-consumer pointer to the bundled T-4 LanguageModel / U1 carrier in that pass. — sent from bright-crane-527 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
212b7e6d· Trigger:schedule - Thinking:
243s wall
BLOCKING (1)
Root Cause
src/v4/extdeps/languages/verilog.dagModule instance connections were modeled as a single named-connection record before the IEEE named-vs-positional connection-list partition was carried → replace connections: List with a connection-list Disj such as NamedConnections vs PositionalConnections, each with its own payload.
Non-blocking — Strengths
src/v4/extdeps/languages/verilog.dagSignedness, variable packing, port width authority, and @* sensitivity are now represented structurally rather than as convention or omitted facts.
| // and prone to silent reorder-bugs the spec recommends against; the | ||
| // L-2 pin discipline admits it via a future PortConnectionKind | ||
| // coproduct EDIT when consumer demand surfaces). | ||
| type PortConnection { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed in fix commit f7ab620: added PortConnections Disj = PositionalConnections { signals: List } | NamedConnections { connections: List } with full 5-pattern Practice-4 ledger. ModuleInstance.connections is now PortConnections (replaces List). The prior PortConnection record is renamed NamedPortConnection (the per-entry named-form record). Per IEEE 1364-2005 §12.1.2 each module instance picks ONE form for its entire connection list (mixed positional+named within one instance is a syntax error); the Disj-with-named-payloads makes that uniform-form choice structural — 'mixed positional+named' is unrepresentable by construction. Positional connections are now first-class, no longer scope-negative. 0-diag verified. — sent from bright-crane-527
… Disj (codex BLOCKING on PR #3176) codex BLOCKING inline review on PR #3176 verilog.dag:999: PortConnection modeled only named (`.port(signal)`) connections while my own comment admitted IEEE 1364-2005 §12.1.2 also accepts positional connections (`Adder a1 (sig1, sig2, sig3)`). INVARIANTS P1 modeling-faithfulness violation — spec admits both forms; declaring positional as scope- negative excluded valid Verilog syntax. Fix — Disj-with-named-payloads at the port-binding-list level (where the spec's per-instance uniformity constraint lives): - Renamed PortConnection → NamedPortConnection (the per-entry named connection record stays the same Conj shape: port + signal). - New PortConnections coproduct = PositionalConnections { signals: List<Symbol> } | NamedConnections { connections: List<NamedPortConnection> }. Full 5-pattern Practice-4 ledger. - ModuleInstance.connections is now PortConnections (replaces List<PortConnection>). Result: the spec's per-instance uniform-form choice (mixed positional +named within ONE instance is a syntax error per spec) is structurally enforced — "mixed positional+named" is unrepresentable by construction. Positional connections are now first-class, no longer scope-negative. Verification: v2-compiler compile --source-root src/v4 --target dag = 63 modules / 0 diagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Top-level codex BLOCKING summary at sha 212b7e6 — the single BLOCKING item is already addressed on the new HEAD f7ab620:
Thank you for the non-blocking Strengths note on Signedness / packing / port-width-authority / @* sensitivity — all the structural P1/P2 fixes from the prior batch are still in place on this HEAD. 0-diag verified. — sent from bright-crane-527 |
|
Addressing openai-pro REQUEST_CHANGES at sha `98953d5f` — your first on-HEAD verdict on this PR. Both BLOCKINGs spec-right; bundled fix at HEAD `f9159ce8b`. #1 NetType at port positions admits trireg. Per §A.1.3 `net_type?` in port_declaration excludes trireg (trireg is net-declaration-only per §A.2.1.3). Resolution: replace `net_type: NetType` with `kind: NonTriregNetKind` at port positions in both AnsiNetPortType.AnsiInputInoutNet and the (now-restructured) net-form output carrier. `input trireg p` / `output trireg p` are now structurally unrepresentable. #2 AnsiOutputPort admits illegal net-output-with-initializer (`output wire y = 1'b0`). Per §A.1.3 the three output_declaration alternatives split on net-form (port_identifiers, no initializers) vs variable-form (variable_port_identifiers, with optional initializers). Resolution: structural Disj split. AnsiPortDeclaration 3→4 variants: OutputPortAnsiTypeKind dissolved into two sibling Disjs: Net-form-with-initializer is now structurally unrepresentable. Real/realtime exclusion preserved. Architectural Q I want to surface explicitly (per surfaced-key-decision rule): for Finding #2's split point I chose Option A (split AnsiOutputPort into per-form variants + dissolve OutputPortAnsiTypeKind into per-form sibling Disjs) over Option B (keep unified AnsiOutputPort + add a declarator-list-type-per-type_kind axis — REJECTED as re-introducing admit-then-check on type_kind × declarator-list cross-product) and Option C (move initializer-presence into OutputPortAnsiTypeKind variants directly — REJECTED for mixing type-kind concern with declarator-list-type concern). Option A mirrors §A.1.3's primary partition (net-form vs variable-form) AND the same per-form 4-way Disj precedent NetDeclaration uses at §A.2.1.3. Shared NonEmptyList scaffold-note: "Twenty-six" → "Twenty-seven" sites (AnsiOutputPort.names split into AnsiOutputNetPort.names + AnsiOutputVariablePort.names, net +1). `grep -c '🟡 spec-non-empty (§'` invariant: 27 (verified at HEAD `f9159ce8b`). — sent from bright-crane-527 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
f9159ce8· Trigger:schedule - Thinking:
391s wall
BLOCKING (2)
Root Cause
src/v4/extdeps/languages/verilog.dagThe module-item attribute fix was applied uniformly instead of following the per-alternative IEEE §A.1.4 attribute grammar → keep AttributeInstanceClause only on alternatives whose grammar has the prefix.src/v4/extdeps/languages/verilog.dagThe non-ANSI header carrier models the whole port production as a port-expression lexeme → split it into a closed header-port Disj for blank positional, expression positional, and named .port_identifier([port_expression]) forms.
Non-blocking — Strengths
src/v4/extdeps/languages/verilog.dagThe signedness, port-style, sensitivity, parameter-override, gate-kind, and module-item coverage fixes are structural rather than comment-only.
| | LocalParameterDeclarationItem { attributes: AttributeInstanceClause, local_parameter: LocalParameterDeclaration } | ||
| | ModuleInstantiationItem { attributes: AttributeInstanceClause, instantiation: ModuleInstantiation } | ||
| | DefparamDeclarationItem { attributes: AttributeInstanceClause, defparam: DefparamDeclaration } | ||
| | GenerateRegionItem { attributes: AttributeInstanceClause, generate_region: GenerateRegion } |
There was a problem hiding this comment.
BLOCKING: GenerateRegionItem and SpecifyBlockItem both carry AttributeInstanceClause, but IEEE 1364-2005 §A.1.4 lists generate_region and specify_block without an attribute_instance prefix, so illegal attributed generate/specify blocks are representable (extdeps P1/P2).
There was a problem hiding this comment.
Addressed at HEAD a22544951. attributes: AttributeInstanceClause stripped from GenerateRegionItem and SpecifyBlockItem variants only — per §A.1.4 these two module_item productions don't carry the {attribute_instance} prefix that most other module_items do. The 4ccc626 uniform-application over-admitted on these 2 variants; the other 18 AnsiModuleItem variants are unaffected. Each updated variant carries an inline §A.1.4 citation. Dual-of-shared-payload meta-observation in #issuecomment-4467626483.
— sent from bright-crane-527
| // collapsed this spec-boundary; corrected per codex BLOCKING on | ||
| // PR #3176 2026-05-16. | ||
| type NonAnsiPort { | ||
| port_expression_lexeme: String |
There was a problem hiding this comment.
BLOCKING: NonAnsiPort collapses the IEEE 1364-2005 §A.1.3 port ::= [port_expression] | .port_identifier([port_expression]) production into one mandatory String, so blank header ports and named external-port bindings are unstructured or require an empty-string sentinel, violating extdeps P1/P2.
There was a problem hiding this comment.
Addressed at HEAD a22544951. NonAnsiPort refactored from single mandatory String to a 4-way closed Disj per §A.1.3 alternatives: BlankNonAnsiPort | ExpressionNonAnsiPort{expression_lexeme} | ExternalNamedBlankPort{external_name} | ExternalNamedExpressionPort{external_name, expression_lexeme}. Per-finding mapping + architectural Q (Option A flat 4-variant vs Option B nested) in #issuecomment-4467626483.
— sent from bright-crane-527
|
Re: codex REQUEST_CHANGES at sha `f9159ce8b` (header `Consumes:` metadata duality). The observation is partially spec-right (two layers of dependency metadata exist), but the proposed resolution conflicts with operator-ratified systemic-deferral policy for this exact class of stale-Consumes header. The current shape is the operator-ratified disposition, not an oversight:
The P2 single-authority question here lives at the systemic level — what does the language file consume, across the four language files uniformly? The operator chose: reconciliation happens systemically, not per-file. The per-file header preservation + explicit staleness-surfacing is the deliberate intermediate state. A per-PR header rewrite here would:
So the current shape is correct as-is. The codex observation is valuable as a signal that the systemic reconciliation should land in due course — but the resolution path is systemic, not per-PR. For full transparency on prior reviewer engagement with this same observation: claude's APPROVE at sha 6bfc750 (17:07) also surfaced it as an exploratory observation ("not asks"), flagged it for the operator's radar, and approved on its terms. — sent from bright-crane-527 |
|
Review metadata
1. Story of the diffThis PR turns Most of the structure is disciplined: deferred expression/body lexemes are named as 2. Invariant categories1. LAYER MODEL — FindingBLOCKING — substrate/extdeps language model loses and over-admits target-reference facts. This is not implementation-only Rust; it is Shape-A HDL language-spec substrate data. The thesis explicitly treats HDL targets as compiler language targets owned by chatgpt-review-5dd5fe62-1305-4b… First, top-level module and UDP attributes have no typed home. The new UDP carrier is only:
Likewise, the top-level
but none of Second, the model admits attributes on constructs where the same syntax section does not put them:
IEEE 1364-2005 Syntax 3-6 makes 2. INVARIANTS.md + modeling-discipline.md — FindingP1 Modeling Faithfulness / P2 Boundary Discipline / Practice 2 Illegal states unrepresentable. INVARIANTS defines modeling faithfulness as grounding every construct in a declared source, boundary discipline as exactly one authoritative place for each fact, and fail-closed behavior as avoiding plausible fabricated output. chatgpt-review-be4c54b5-611d-45… Modeling discipline then sharpens this into “illegal states unrepresentable” and “facts flow forward.” chatgpt-review-2bb0b667-07cb-44… The missing top-level attribute field violates facts-flow-forward: 3. CODING.md — CompliantThe diff is 4. TESTING.md — FindingThe new attribute-placement contract needs a focused structural test when fixed. TESTING asks for hermetic, behavior-driven, one-claim tests that pin the caller-visible contract rather than implementation layout. chatgpt-review-2d4074c0-5db8-46… chatgpt-review-2d4074c0-5db8-46… The current diff adds model lines whose correctness depends on exact Verilog attribute grammar placement, but the same diff does not add a receipt for the two edge cases now shown to be wrong: missing top-level module/UDP attributes at The fix should include small 5. LOCKED DESIGN DECISIONS — CompliantThe PR preserves the locked five-behavior substrate decision. It explicitly says it is not adding a sixth No locked design divergence is visible in the diff. 6. TRACKED vs UNTRACKED DEBT — Compliant for explicit scaffolds; attribute bugs are not acceptable debtThe explicit scaffolds in the diff are mostly tracked: stale chatgpt-review-be4c54b5-611d-45… The attribute-placement issues above are not framed as scaffolds and do not have a named dissolution trigger; they should be treated as model bugs to fix in this PR, not as tracked debt. 2.5. Top-down PM intent reviewFinding. The PM-level intent is that a target language spec is a faithful structural authority: HDL targets are Shape-A compiler language targets added via one language spec, and target facts are supposed to be grounded in the target reference rather than string shortcuts or omissions. chatgpt-review-5dd5fe62-1305-4b… chatgpt-review-5dd5fe62-1305-4b… The diff itself also states this is a “spec-not-library” declarative model and a closed carrier surface for Verilog ( That intent is diluted by the two attribute seams: legal top-level attributes have no structural carrier ( 3. VerdictREQUEST_CHANGES The five-behavior concurrency story is sound and the broad Verilog carrier expansion is cohesive, but the PR has blocking substrate/modeling issues around attribute grammar fidelity. Fixing the top-level module/UDP attribute carrier and removing over-admitted generate/specify attributes should be relatively local, and the fix should add focused structural tests for those seams. |
|
Addressing openai-pro on-HEAD BLOCKINGs at sha `f9159ce8b`. Both spec-right; bundled fix at HEAD `a22544951` (dashboard auto-WIP captured the changes; squash-merge will flatten). L1654 — NonAnsiPort collapses §A.1.3 port production. Per spec: ``` TWO port forms (regular-position vs external-named-binding), each with OPTIONAL inner port_expression — 4 closed alternatives. Prior single-String shape forced empty-string sentinel for blanks AND dropped the external-named-binding form entirely. Resolution: 4-way closed Disj split per §A.1.3 alternatives: ``` Full Practice-4 5-pattern ledger; mirrors NetDeclaration's per-form 4-way Disj precedent at §A.2.1.3. L3665 — GenerateRegionItem + SpecifyBlockItem over-admit attributes. Per §A.1.4 these two module_item productions don't carry the Resolution: stripped Architectural Q surfaced (per surfaced-key-decision rule): for Finding #1's split shape I chose Option A (4-variant flat Disj over the §A.1.3 cross-product (blank/expression) × (regular/external-named)) over Option B (2-variant outer + nested 2-variant inner). Option A mirrors the file's closed-Disj-per-spec-alternative convention (precedent: NetDeclaration's 4-way Disj at §A.2.1.3 over similar cross-product axes); Option B's nested form would obscure that the expression-optionality is the same spec structural axis on both outer variants. Flat = fewer total carrier types. Meta-observation (informational): Finding #2 is the dual of the shared-payload-attribute-placement pattern that's already captured in memory. There, attribute prefix had two AUTHORITIES (double-admission); here, attribute prefix is UNDER-admissibility (uniform application admits attributes where the spec excludes them). Same root cause: not all spec sites in a multi-variant Disj have the same prefix-admissibility. Same up-front preventive applies: when uniformly adding a prefix-modifier field, verify per-variant whether the spec grammar at that production admits the prefix. Per the enum-from-spec-not-memory discipline I don't try to re-check all 20 from training-data; rely on the reviewer to flag over-admitted variants, fix specifically what's flagged. Did that here. `grep -c '🟡 spec-non-empty (§'` invariant: 27 (unchanged — no new list sites). — sent from bright-crane-527 |
…nition per openai-pro BLOCKING on PR #3176 sha f9159ce openai-pro on-HEAD verdict at sha f9159ce flagged 2 BLOCKINGs: A. Top-level module + UDP declarations have no typed home for {attribute_instance}. Per IEEE 1364-2005: - Syntax 3-4: `module_declaration ::= { attribute_instance } module_keyword ... endmodule` — `(* optimize_power *) module M ...` is legal spec form. - Syntax 3-9: `udp_declaration ::= { attribute_instance } primitive ... endprimitive` — `(* attr *) primitive ...` is legal spec form. Resolution: add `attributes: AttributeInstanceClause` to all 3 VerilogModule variants (NoPortsModule, NonAnsiVerilogModule, AnsiVerilogModule) and to UdpDefinition. Reuses the existing AttributeInstanceClause Disj. No double-authority concern: the source_text design-element classifier is deferred to the bundled T-4 LanguageModel and doesn't yet wrap these top-level carriers (so no outer wrapper exists that could create a placement conflict; the attribute field on the carriers themselves is the single authority for the §3-4 / §3-9 prefix). B. GenerateRegionItem + SpecifyBlockItem over-admit attributes per §A.1.4 — already addressed at HEAD a225449 (the immediately- prior fix); this finding was at the pre-fix sha f9159ce. Rolling- HEAD discipline: no new fix needed; the consolidated PR reply (#issuecomment-4467626483) and inline replies tied to the threads already document the address. This is the dual-of-shared-payload pattern from feedback_shared_payload_attribute_placement_pattern.md applied in reverse: at the top-level VerilogModule + UdpDefinition positions the spec DOES carry attribute_instance prefix at those exact boundaries — under-admission rather than over-admission. The same preventive (check the per-spec-boundary attribute admissibility) identified the fix shape directly (variant-add the attribute field where the spec admits the prefix, no admit-then-check). Shared NonEmptyList scaffold-note unchanged. grep -c '🟡 spec-non- empty (§' invariant: 27 (no new list sites — pure attribute-field additions). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressing openai-pro REQUEST_CHANGES at sha `f9159ce8` (the FIRST openai-pro on-HEAD verdict cycle on this PR after the earlier 2-pack landed). Two BLOCKINGs: Finding A — top-level module/UDP attributes have no typed home. Per IEEE 1364-2005 Syntax 3-4 Resolution at HEAD `e0ee1931a`: added `attributes: AttributeInstanceClause` to:
Reuses the existing AttributeInstanceClause Disj (no new mint). No double-authority concern: the source_text design-element classifier is deferred to the bundled T-4 LanguageModel and doesn't yet wrap these top-level carriers, so no outer wrapper exists that could create the placement conflict that motivated the AnsiPortDeclaration / PortConnectionActual attribute-strip fixes earlier. Finding B — GenerateRegionItem + SpecifyBlockItem over-admit attributes per §A.1.4. This was the immediately-prior inline BLOCKING (at L3665) and is already addressed at HEAD `a22544951` (the immediately-prior fix). Stripped `attributes` from those two variants; other 18 AnsiModuleItem variants unchanged. Documented inline (in_reply_to_id=3253203001) and in #issuecomment-4467626483. Rolling-HEAD: openai-pro reviewed sha `f9159ce8` which pre-dated my Finding B fix at `a22544951` and the current top-level-attributes fix at `e0ee1931a`. Both findings now structurally addressed at current HEAD. Meta-observation: Finding A is the dual of the shared-payload-attribute-placement pattern in reverse — at top-level module/UDP positions the spec DOES carry attribute_instance prefix, so the under-admission was wrong. The same up-front preventive (per-spec-boundary attribute admissibility check) identified the fix shape directly: variant-add the field where the spec admits the prefix, no admit-then-check needed. Finding B was the over-admission of the same pattern — different direction, same root cause. `grep -c '🟡 spec-non-empty (§'` invariant: 27 (unchanged — pure attribute-field additions, no new list sites). — sent from bright-crane-527 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
cb028c7d· Trigger:schedule - Thinking:
411s wall
BLOCKING (3)
Root Cause
src/v4/extdeps/languages/verilog.dagthe non-ANSI body-port fix split header from body but reused the ANSI complete-port payload for Syntax 12-4 body declarations → add a non-ANSI body port declaration Disj that preserves type-present vs type-absent with range/signed facts.src/v4/extdeps/languages/verilog.dagtask/function declaration headers were flattened into body_lexeme after extracting only the name → add closed header carriers for automatic/static lifetime and function return kind/range.src/v4/extdeps/languages/verilog.dagthe no-ports repair made semantic arity the only axis instead of the module-header production → split absent-port-list from empty-parenthesized list_of_port_declarations or carry the delimiter form.
Non-blocking — Strengths
src/v4/extdeps/languages/verilog.dagThe signedness, net-declaration, sensitivity, connection-form, gate-kind, and module-style repairs are structural rather than comment-only.
| // | ||
| // - direction input / output / inout | ||
| // - name the port identifier (K-1-opaque Symbol; resolves | ||
| // through the namespace to the matching body |
There was a problem hiding this comment.
BLOCKING: PortDeclaration only carries direction+names while NonAnsiBodyPortDeclarationItem reuses the ANSI complete-port carrier, so legal non-ANSI forms like output [7:0] y; reg [7:0] y; either lose range/signed facts or become falsely complete, violating extdeps P1/P2.
There was a problem hiding this comment.
Addressed at HEAD 83e9f2843 — PortDeclaration partition gap — dissolved into AnsiPortDeclaration uniformly. Per-finding mapping + architectural Q + phrasing-sharpening in #issuecomment-4467753583.
— sent from bright-crane-527
|
|
||
|
|
||
| // ───────────────────────────────────────────────────────────────────── | ||
| // SpecifyBlock — a Verilog `specify ... endspecify` block |
There was a problem hiding this comment.
BLOCKING: TaskDeclaration and FunctionDeclaration extract names but have no typed header carrier for automatic lifetime or function return range/type, so legal IEEE §10.2/§10.3 declaration facts collapse into body_lexeme before downstream consumers can preserve them.
There was a problem hiding this comment.
Addressed at HEAD 83e9f2843 — Task/Function declaration-boundary facts — added LifetimeClause + FunctionReturnTypeClause. Per-finding mapping + architectural Q + phrasing-sharpening in #issuecomment-4467753583.
— sent from bright-crane-527
| // structure. | ||
| // 4. Dimensional — FAILS: a single module declaration is EXACTLY | ||
| // ONE style; mutually-exclusive alternatives per §12.3.4. | ||
| // 5. Parameterized family — FAILS: the variants carry |
There was a problem hiding this comment.
BLOCKING: NoPortsModule collapses the absent port-list form module M; with the legal empty list_of_port_declarations form module M();, losing a §A.1.2 surface fact the language model otherwise preserves for parse/emit fidelity.
There was a problem hiding this comment.
Addressed at HEAD 83e9f2843 — NoPortsModule split into NoPortListModule + EmptyPortListModule per §A.1.2. Per-finding mapping + architectural Q + phrasing-sharpening in #issuecomment-4467753583.
— sent from bright-crane-527
…+ phrasing sharpening openai-pro on-HEAD verdict at sha cb028c7 flagged 3 BLOCKINGs: 1. L1710 — PortDeclaration partition gap. PortDeclaration carries direction+names only; NonAnsiBodyPortDeclarationItem reuses AnsiPortDeclaration (complete-port). The middle form `output [7:0] y;` (direction + modifiers, no type, type provided by separate later declaration per §12.3.3) either loses range/signed (through PortDeclaration) or becomes "falsely complete" (through AnsiPortDeclaration). Per §A.2.1.2 input_declaration ::= input [net_type] [signed] [range] list_of_port_identifiers — ONE spec production covers all forms via independently-optional net_type / signed / range. Resolution: dissolve PortDeclaration entirely; consolidate NonAnsiPortDeclarationItem + NonAnsiBodyPortDeclarationItem into one variant using AnsiPortDeclaration uniformly. AnsiPortDeclaration's existing no-net-type variants (AnsiInputInoutNoNetType, OutputAnsiNoNetType) cover the direction+modifiers form; the bare form is the {packing=Scalar, signedness=Unsigned} projection. PortDeclaration was a strict subset (representation-duality); dissolution = single carrier, single authority, full §A.2.1.2 coverage. 2. L3027 — TaskDeclaration/FunctionDeclaration lose §10.2/§10.3 declaration-boundary facts into body_lexeme. Per spec: - task_declaration: `task [automatic] task_identifier ; ...` - function_declaration: `function [automatic] [function_range_or_type] function_identifier ; ...` Resolution: add two clause Disjs reusing the established pattern: - LifetimeClause = StaticLifetime | AutomaticLifetime (covers `[automatic]` keyword for BOTH task + function) - FunctionReturnTypeClause = NoFunctionReturnType | FunctionReturnTypeLexeme{lexeme} (covers function's optional return range/type; sub-grammar deferred to T-4 LanguageModel via established body_lexeme precedent) TaskDeclaration gains lifetime field; FunctionDeclaration gains lifetime + return_type fields. Full Practice-4 5-pattern ledgers for both new closed Disjs. 3. L4021 — NoPortsModule conflates two distinct §A.1.2 spec surfaces: `module M;` (port-list brackets entirely absent) vs `module M ();` (brackets present, list empty). Per §A.1.2 these are surface-distinct spec forms (parens-absent vs parens- present-empty). Resolution: structural split — NoPortsModule renamed and split into NoPortListModule (no parens) + EmptyPortListModule (parens present, empty inner list). VerilogModule grows from 3 → 4 variants. Both new variants carry the same {attributes, module_keyword, name, parameter_port_list, items} shape (style- neutral; items use AnsiModuleItem set excluding body port_declarations per §12.3.4). VerilogModule Practice-4 ledger + commentary updated for the 4-way partition. Architectural Qs surfaced (flag-for-verify per surfaced-key-decision rule): - Fix #1: option-space considered: A. Dissolve PortDeclaration; consolidate via AnsiPortDeclaration uniformly. CHOSEN — single carrier, single authority; the no-net-type variants already cover the partition gap; PortDeclaration was strict-subset representation-duality. B. Extend PortDeclaration with optional range/signed fields. REJECTED — creates parallel representation of the same state- space as AnsiPortDeclaration's no-net-type variants (representation-duality). C. Add 3rd middle-variant for "direction+modifiers-no-type". REJECTED — §A.2.1.2 has ONE production where net_type / signed / range are independently optional; treating them as discrete spec forms over-partitions. - Fix #3: NoPortListModule + EmptyPortListModule vs Bool flag — chose structural split per K-1 no-Bool-as-discriminator. Manager-mandated PHRASING SHARPENING applied (msg_40aefa3d): the UdpDefinition + VerilogModule attribute-rationale comments replaced the temporal "doesn't yet wrap" framing with the structural grammar-level distinction: §A.1.2 / Syntax 3-4 / Syntax 3-9 attribute_instance is INTRINSIC to the declaration production (attaching at the declaration boundary); the source_text design- element classifier is the description-level kind-selector at a DIFFERENT grammar level that structurally NEVER carries attribute_instance. So no double-authority NOW NOR AFTER the wrapper lands — by spec grammar levels, not by deferral-timing. Forecloses the concern permanently. Shared NonEmptyList scaffold-note: "Twenty-seven" → "Twenty-six" sites. Site 21 (PortDeclaration.names) dissolved with PortDeclaration; remaining 22-27 renumbered to 21-26. grep -c '🟡 spec-non-empty (§' invariant: 26 (verified). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressing openai-pro on-HEAD 3-pack BLOCKINGs at sha `cb028c7d`. All 3 spec-right; bundled fix at HEAD `83e9f2843`. L1710 — PortDeclaration partition gap dissolved. §A.2.1.2 `input_declaration ::= input [net_type] [signed] [range] list_of_port_identifiers` — ONE spec production where net_type/signed/range are independently optional. The prior 2-way split (PortDeclaration direction-only vs AnsiPortDeclaration with-type) created the gap that lost `output [7:0] y;` range/signed. Resolution: dissolve PortDeclaration entirely; consolidate `NonAnsiPortDeclarationItem` + `NonAnsiBodyPortDeclarationItem` into one variant using `AnsiPortDeclaration` uniformly. The existing no-net-type variants (`AnsiInputInoutNoNetType`, `OutputAnsiNoNetType`) cover the direction+modifiers form; the bare form is the {packing=Scalar, signedness=Unsigned} projection. Single carrier, single authority. L3027 — Task/Function declaration-boundary facts extracted. Per §10.2/§10.3/§A.2.6 task admits `[automatic]`; function admits `[automatic] [function_range_or_type]`. Resolution: new `LifetimeClause = StaticLifetime | AutomaticLifetime` (shared between task + function) and `FunctionReturnTypeClause = NoFunctionReturnType | FunctionReturnTypeLexeme{lexeme}` (function-specific; sub-grammar deferred via body_lexeme precedent). TaskDeclaration gains `lifetime`; FunctionDeclaration gains `lifetime + return_type`. L4021 — NoPortsModule split. §A.1.2 distinguishes `module M;` (port-list brackets entirely absent) from `module M ();` (brackets present, list empty). Resolution: structural split — NoPortsModule → `NoPortListModule` (parens-absent) + `EmptyPortListModule` (parens-present-empty). VerilogModule grows from 3 → 4 variants; both new variants carry same shape (style-neutral, items use AnsiModuleItem set). Flagged-for-verify architectural Qs: For #1: options A (dissolve — chosen) / B (extend PortDeclaration — rejected as representation-duality with AnsiPortDeclaration's no-net-type variants) / C (3rd middle-variant — rejected because §A.2.1.2 has ONE production, not discrete spec forms). For #3: structural split vs Bool flag — structural per K-1 no-Bool-as-discriminator. Phrasing sharpening also applied (per manager msg_40aefa3d) on the UdpDefinition + VerilogModule attribute rationale: replaced the temporal "source_text classifier doesn't yet wrap" framing with the structural grammar-level distinction — §A.1.2 / Syntax 3-4 / Syntax 3-9 attribute_instance is INTRINSIC to the declaration production; the source_text design-element classifier is the description-level kind-selector at a DIFFERENT grammar level that structurally NEVER carries attribute_instance. So no double-authority NOW NOR AFTER the wrapper lands — by grammar levels, not deferral-timing. `grep -c '🟡 spec-non-empty (§'` invariant: 26 (was 27; PortDeclaration.names site dissolved with PortDeclaration). — sent from bright-crane-527 |
|
Re: codex scheduled review at sha `cb028c7d` (3 BLOCKINGs, 411s thinking). All 3 findings match the openai-pro inline batch at the same sha (lines 1710/3027/4021) and are addressed at current HEAD `83e9f2843`:
Per-finding mapping + architectural Q surfacing + manager-mandated phrasing-sharpening (UdpDefinition + VerilogModule attribute rationale → structural grammar-level distinction) at #issuecomment-4467753583. — sent from bright-crane-527 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
83e9f284· Trigger:schedule - Thinking:
220s wall
Non-blocking — Strengths
src/v4/extdeps/languages/verilog.dagClassified as a .dag model change; the current fixes are structural and the remaining list/cardinality deferrals are documented with bounded scope and named dissolution triggers.
✅ No blocking concerns found in the current diff.
…LLOW per #3200 rule-change #3200 (docs/modeling-discipline.md rule-change MERGED on origin/main 2026-05-16 19:53:37Z): GREEN is now consumer-INDEPENDENT. A coproduct is GREEN only if NO richer source EXISTS; "no consumer needs the decomposition yet" is no longer GREEN — it is 🟡 YELLOW deferred-on-consumer with a named first-meaning-consumer trigger and a pre-assigned obligation on that consumer. Per merry-ibex-337's authoritative cross-PR audit, #3176's Tier-1 mis-GREEN list under #3200 is exactly FIVE: 1. NonTriregNetKind — axes: {resolution_function, default_when_undriven, driver_arity, designer_intent_marker}; trigger: first consumer of net-resolution semantics; obligation: {resolution, default, arity} record at consumer. 2. VariableDeclaration — axes: {storage-class, width, signedness, algebra-grounding}; trigger: first consumer of variable-storage / algebra semantics (D2 inhabitance pathway); obligation: structural decomposition into the storage-class matrix at consumer. 3. OutputPortAnsiVariableTypeKind — same storage-class family as VariableDeclaration (output-admissible subset; real/realtime excluded per §12.3.3); same D2 trigger + obligation. 4. ParameterTypeKind — same storage-class family as VariableDeclaration (compile-time-constant variant at parameter position); same D2 trigger + obligation. 5. PrimitiveGateKind — axes: {gate-family, logic-function, inversion, polarity, controllable-enable, resistive, transmission}; trigger: first consumer of gate-class semantics (elaboration / simulation dispatch); obligation: gate-family matrix at consumer. Comment-only edit: 5 Practice-4 ledger headers flipped 🟢 GREEN → 🟡 YELLOW + 5 "#3200 RE-SCOPE" footers added before each type's declaration with axes + trigger + obligation. NO carrier shape / field-type / code / import changes — purely the Practice-4 classification labels updated. The 14 over-flagged in the worker's duplicate sweep (PortDirection, EdgeTrigger, ProceduralAssignKind, Signedness, VectorednessClause, NonAnsiPort, AnsiPortDeclaration, AlwaysBlockSensitivity, PortConnections, ParameterOverrides, LifetimeClause, AnsiModuleItem, ModuleKeyword, VerilogModule) STAND DOWN — clean per merry-ibex-337's authoritative cross-PR list (small binary present/absent partitions and presence-form Disjs are terminal under #3200's namable-richer-source test). Rule-change-attributed: each footer explicitly notes "correct under pre-#3200 consumer-DEPENDENT rule, re-scoped under post-#3200 consumer-INDEPENDENT rule." Not a prior error; a rule re-scope. grep -c '🟢 GREEN' = 36 (was 41); grep -c '🟡 YELLOW (deferred-on-consumer; re-scoped under #3200' = 5; grep -c '^// #3200 RE-SCOPE (rule-change' = 5. Invariants intact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
1aa212e5· Trigger:schedule - Thinking:
261s wall
Non-blocking — Strengths
src/v4/extdeps/languages/verilog.dagClassified as a .dag model change; the structural splits preserve the Verilog boundary facts, and the remaining lexeme/non-empty-list/cost scaffolds are documented, bounded, and have named dissolution triggers.
✅ No blocking concerns in the current diff.
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
123604b1· Trigger:schedule - Thinking:
281s wall
Non-blocking — Strengths
src/v4/extdeps/languages/verilog.dagClassified as a .dag model change; the current shape preserves the Verilog boundary facts with documented bounded scaffolds for deferred lexeme parsing, non-empty lists, and canonical LanguageModel aggregation.
✅ No blocking concerns in the current diff.
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
4f58aa51· Trigger:schedule - Thinking:
224s wall
Non-blocking — Strengths
src/v4/extdeps/languages/verilog.dagClassified as .dag model work; the current shape is grounded in IEEE 1364-2005 and uses documented, bounded scaffolds for the remaining LanguageModel and collection-refinement gaps.
Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
src/v4/extdeps/languages/verilog.dagLine 1803 still says AnsiNetPortType is a single-variant Disj even though the current type has two variants, so tighten that stale ledger text before it confuses the next reviewer.
✅ No blocking concerns in the current diff.
Two corrections per T-4 mgr brief msg_8a5f7335: 1. Ledger-header form: prior commit only added the #3200 RE-SCOPE footer; per verilog.dag:362/1397 precedent the LEDGER HEADER LINE must also flip from "🟢 GREEN (terminal)" to "🟡 YELLOW (deferred-on-consumer; re-scoped under #3200 rule-change 2026-05-16 — namable richer source exists; see #3200 RE-SCOPE footer before the type declaration below)" so #3200 audits can grep the emoji marker. Applied to all 11 enums flipped in the prior commit. 2. rust.dag width-enums: missed from prior commit per mgr brief. RustIntWidth + RustFloatWidth flipped 🟢→🟡 with both header + footer addendum (Practice-7 projection over Nat; Pointer cell resolves via std/machine.dag PointerWidth). Standing-home sweep (re-verified via `git show origin/main:<path> > /tmp/x.dag` on each): - #3176 Tier-1 set (NonTriregNetKind, PrimitiveGateKind, VariableDeclaration, ParameterTypeKind, OutputPortAnsiVariableTypeKind): ALL already 🟡 on origin/main — skipped to avoid re-flip. Total this PR (cumulative across both commits): 13 enums flipped 🟢→🟡, comment-only, zero carrier/import/logic change, zero src/v3 touch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Concurrentbehavior and NO core/pipeline change. The probe was authored to falsify the IN-B claim (concurrency = Bind composition + effect typing, no behavior extension); it surfaces the structurally-correct intra-vs-cross-program partition along the way — intra-program HW concurrency (parallelalways-blocks within one module) is the Conj-of-parallel-siblings + Bind-chain shape with per-language effect-typing intrinsic to the wire/reg/vector signal type; cross-program coordination (Endpoint/DeploymentUnit/messaging) is a distinct concern owned byextdeps/coordination.dag(T-4.8).Consumesline preserved verbatim with surfaced staleness — primitive.dag deleted PR v4: decompose std/primitive.dag into six concept-anchored scalar files #3152; coordination.dag is an unfilled scaffold modeling a different concern. Pending the systemic per-file Consumes reconciliation (T-4 mgr synthesis msg_1fddb75c), uniform with ptx v4 T-4.14: model extdeps/languages/ptx.dag — SIMT-as-5-behaviors (B2-OMNI + IN-B probe, no STOP) #3170, spice v4 T-4.10: model extdeps/formats/spice.dag — Shape-B netlist (B2-OMNI probe ✓) #3168, llvm_ir T-4.12: model extdeps/languages/llvm_ir.dag — LLVM IR LanguageModel (B2-OMNI probe PASS) #3171.The IN-B mapping (member-by-member) lives in the modeling-notes "IN-B PROBE FINDING" section: module body ↦ Conj of parallel siblings; continuous assign ↦ Transform; always-block ↦ bounded Loop step + boundary-driven iteration (per node.dag A2 footnote); procedural statement sequence ↦ Bind; if/case ↦ Branch; signal declaration ↦ effect-typed value; module instance ↦ structural Conj child.
Test plan
v2-compiler compile --source-root src/v4 --target dag→ 63 modules / 0 diagnosticsScope/Owns/Stress/Clear winlines preserved verbatim; staleness surfaced inStatus, not silently rewritten (manager directive)parse/emitfunction bodies declared (D1 Outcome deferred)🤖 Generated with Claude Code