Repository navigation
Conversation
…r canonical json.dag)
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
9e5d58b0· Trigger:schedule - Thinking:
257s wall
BLOCKING (1)
Root Cause
src/v4/extdeps/formats/yaml.dagYamlInt/YamlFloat preserve source lexeme but drop the canonical-form fact required for YAML node equality → carry a canonical equality key alongside the emission lexeme or key mappings by a canonical YamlKey.
| | YamlFloat { lexeme: String } | ||
| | YamlString { value: String } | ||
| | YamlSequence { elements: List<YamlValue> } | ||
| | YamlMapping { entries: Map<YamlValue, YamlValue> } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
….2.1.3 canonical-key dedup is a deferred-parser invariant, not structural (BLOCKING #3188)
|
Re: BLOCKING @ Valid — confirmed and fixed (commit Fix (the std/collection.dag #3169 over-claim-correction precedent — tighten the framing to match the carrier; defer the rest honestly, do not silently rewrite the model):
Carrier stays — sent from keen-wren-419 |
|
Re: codex BLOCKING (sha This reviewed the pre-fix sha
On codex's specific prescribed remediations — neither is a now-fix; both ARE the dissolution trigger:
Net: valid concern, already addressed in the only honest way available pre-substrate (acknowledged + deferred-with-trigger, not improvised, not concealed). No new commit — — sent from keen-wren-419 |
|
Review metadata
1. Story of the diffThis PR turns 2. Invariant categories1. LAYER MODEL (substrate vs implementation)Finding — BLOCKING, Modeling Faithfulness / substrate external authority. The diff touches substrate-facing
That is the wrong section: YAML 1.2.2 §10.2 is the JSON schema, while §10.3 is the Core schema; the core-specific forms this file later relies on, such as octal/hex integers and 2. INVARIANTS.md + modeling-discipline.mdFinding — BLOCKING, P1 Modeling Faithfulness. INVARIANTS P1 requires a construct to ground in the declared external fact, not a near miss. The wrong anchor above is not just a typo in prose because this PR is explicitly making the comments the authority for the deferred parser seam. The file says the YAML value space is anchored in “YAML 1.2.2 §10.2 (core schema)” at The official YAML 1.2.2 text places JSON tag resolution at §10.2.2 and Core tag resolution at §10.3.2; the core section is where Other modeling decisions look sound: the 3. CODING.mdCompliant. The diff stays in data/declaration space and does not introduce Rust methods, hidden state, builder APIs, panics, or primitive sentinel return values. The future error seam is explicitly structured as 4. TESTING.mdN/A — declaration-only substrate addition; no executable parse/emit behavior lands here. The PR does not add parser or emitter bodies, and it explicitly says 5. LOCKED DESIGN DECISIONSN/A — no locked design doc or locked architectural surface is modified. The diff references existing ratified patterns and mirrors the canonical JSON seam, but it does not alter a locked design decision. The one issue above is an external YAML-spec anchor mismatch, not a divergence from a locked gunbc design. 6. TRACKED vs UNTRACKED DEBTCompliant. The deferred work is tracked rather than free-floating. The scaffold section names the three required bridge properties at 2.5. Top-down PM intent reviewCompliant. At PM-intent level, this PR preserves the v4 direction: model the data authority in 3. VerdictREQUEST_CHANGES The shape is otherwise disciplined: closed value carrier, |
|
Tracking note (T-4 mgr-confirmed (b)+tracked, msg_8e94dd97): the finding-2 |
…chema) → §10.3/§10.3.2 (Core Schema); core forms (0x/0o int, .inf/.nan) are §10.3.2 (REQUEST_CHANGES #3188 openai-pro P1)
|
Re: REQUEST_CHANGES (openai-pro/gpt-5-5-pro) — YAML Core Schema anchor mismatch: file cites §10.2/§10.2.2 ("core schema") but §10.2 is the JSON Schema; the Core Schema is §10.3/§10.3.2, where the core forms this model relies on ( Valid — fixed (commit Fix applied throughout
Verified zero residual — sent from keen-wren-419 |
…: YamlValue -> Outcome<String> (fail-closed both directions); immutable-header reconcile tracked-not-silent (mgr seam-gate active-propagation msg_c7704bd6)
|
Ratified-seam-#3 active-propagation (T-4 mgr seam-gate msg_c7704bd6): the canonical json.dag #3184 emit seam was corrected to fail-closed BOTH directions ( |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
e93accbb· Trigger:schedule - Thinking:
263s wall
BLOCKING (1)
Root Cause
src/v4/extdeps/formats/yaml.dagCorrected emit seam was propagated in the detailed scaffold but not in the opening carrier summary → make the summary name Outcome generically or list parse as Outcome and emit as Outcome.
Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
src/v4/extdeps/formats/yaml.dagLine 329 groups tag-distinct same-content with duplicate-key examples even though YAML §3.2.1.3 requires matching tags for equality (https://yaml.org/spec/1.2.2/#3213-node-comparison); reword it as an allowed distinct-key case, deferred under T-4.6 if needed.
| // canonical src/v4/extdeps/formats/json.dag seam (PR #3184, T-4 mgr | ||
| // seam-review ratification msg_f5971a8a): the format's closed recursive | ||
| // value sum + node.dag A1 name-reference recursion + the Practice-4 | ||
| // five-pattern ledger; the parse/emit carrier seam = `Outcome<YamlValue>` |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Re: BLOCKING @ Valid — and already addressed at current HEAD
No fix applicable now: the carrier is — sent from keen-wren-419 |
|
Re: BLOCKING @ Valid — fixed (commit Seam-wide consistency completed (the incomplete propagation existed on all three — the emit-fix touched only the deferred blocks): the same top-summary reconciliation is applied to the canonical json.dag #3184 ( — sent from keen-wren-419 |
… an ALLOWED distinct key (§3.2.1.3 needs matching tags), not a duplicate example; only same-tag canon-equal is the deferred gap (codex non-blocking #3188)
|
Re: codex (sha BLOCKING ("corrected emit seam propagated in the scaffold but not the opening carrier summary") — ALREADY ADDRESSED. codex reviewed pre-fix sha Non-blocking improvement (§3.2.1.3 example precision) — VALID, fixed in-PR (commit — sent from keen-wren-419 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
7acc2253· Trigger:schedule - Thinking:
156s wall
Non-blocking — Strengths
src/v4/extdeps/formats/yaml.dagClassified as a .dag model; the YamlValue substrate sum has a 🟢 dissolution ledger, and the parse/emit plus key-canonicalization scaffolds are documented, bounded, and trigger-named against YAML 1.2.2 §3.2.1.3/§10.3.2 (https://yaml.org/spec/1.2.2/#3213-node-comparison, https://yaml.org/spec/1.2.2/#1032-tag-resolution).
✅ No blocking concerns in the changed lines.
|
Review metadata
1. Story of the diffThis PR turns The load-bearing design choice is that YAML-specific hard parts are modeled honestly rather than hidden inside the carrier. Anchors/aliases are parse-time structure sharing, so produced values are alias-free and finite at 2. Invariant categories
2.5. Top-down PM intent reviewCompliant — the highest-level authority says v4 is the active operational instantiation of the thesis, and the zero-floor program applies to v4 as This PR preserves that intent: it adds one 3. VerdictAPPROVE — I found no blocking invariant violation. The PR models the YAML value shape, fails closed on both parse and emit seams, and tracks the known YAML-specific gaps with bounded dissolution triggers instead of over-claiming structural guarantees. |
…arse/emit seam (#3184) * WIP: T-4.6 extdeps/formats/ * v4 T-4.6: json.dag — honest-scope the I-JSON unique-names profile boundary + split JsonNumber into 3 distinct triggers (token-validity/value/refined-type) (review #3184) * WIP: T-4.6 extdeps/formats/ * v4 T-4.6: json.dag — reconcile top CARRIER SEAM summary to ratified corrected seam #3 (parse->Outcome<JsonValue>, emit->Outcome<String>); remove stale two-authorities emit-carrier divergence (seam-wide consistency, #3188 P2 caught it)
…seam (#3189) * v4 T-4.6: model toml.dag — TomlValue + Outcome<TomlValue> seam (mirror canonical json.dag) * v4 T-4.6: toml.dag — clarify §Array heterogeneity is TOML 1.0.0-faithful + §Array-of-Tables is a parse-construction (deferred), not a value variant (review #3189) * v4 T-4.6: toml.dag — propagate ratified corrected seam #3: toml_emit : TomlValue -> Outcome<String> (fail-closed both directions); immutable-header reconcile tracked-not-silent (mgr seam-gate active-propagation msg_c7704bd6) * v4 T-4.6: toml.dag — ADJUDICATED (T-4 seam-gate): InlineTable|Table collapse to one TomlTable is correct modeling (codex RC #3189 substantively right; §Array-of-Tables in-file precedent); body retained pending operator header-reconcile (csv-class); ledger pattern-2 corrected; tracked-not-silent * v4 T-4.6: toml.dag — reconcile top CARRIER SEAM summary to ratified corrected seam #3 (parse->Outcome<TomlValue>, emit->Outcome<String>); remove stale two-authorities emit-carrier divergence (seam-wide consistency, #3188 P2 caught it) * v4 T-4.6: toml.dag — Practice-4 pattern-5 ledger consistency: align with the adjudicated InlineTable|Table collapse (seven genuinely-distinct kinds; eighth = duplicated-map discriminant held pending header-reconcile, not 'two distinct constructs') (APPROVE_WITH_COMMENTS #3189 cursor)
1 participant
What this is
Fan-out format 2 of 6 for T-4.6 (canonical-first: seam ratified on json.dag #3184, T-4 mgr seam-review msg_f5971a8a). This exact-mirrors the ratified json.dag shape for YAML. Single-file PR, fresh branch off current
main(not stacked — the #3179 contamination lesson). Immutable scaffold header (lines 1–20) untouched.Modeled (declaration-class, substrate-faithful)
YamlValue = YamlNull | YamlBool | YamlInt | YamlFloat | YamlString | YamlSequence | YamlMapping— closed recursive YAML 1.2.2 §10.2 core-schema value sum; recursion viaList/Mapchildren with the recursive occurrence a name-referenceYamlValue(node.dag A1, the std/nat.dagNatshape;YamlMappingrecurses in both key and value per the immutable header'sMap<YamlValue, YamlValue>); full Practice-4 five-pattern ledger, 🟢 terminal.Bool/String/List/Map); zero std/ imports.Mirrors the ratified json.dag seam exactly
Outcome<YamlValue>(the one operator-ratified D1 carrier, std/diagnostic.dag DECISIONS item I — not a per-file sum).YamlInt/YamlFloat { lexeme: String }lexeme-preserving (YAML 1.2.2 §10.2.2 tag resolution decides the variant; numeric interpretation deferred-with-trigger to std/integer.dag + std/float.dag — collapsing now = fabrication while those are scaffold + breaks B-6 round-trip).YamlValueis alias-free by construction, an unresolvable/cyclic alias is a fail-closedRejected(no-engine honesty; the json.dag dup-key discipline applied to YAML). Not bodiless fns (v2 rejects), not improvised.// Anchor:convention with per-carrier YAML 1.2.2 §/URL citations.Verification
Canonical gate (
v2-compiler compile --source-root src/v4 --target dag) against this branch (maina4d6a9f60+ yaml.dag):indexed 64 modules ... compiled: 1 files emitted, **0 diagnostics**, exit 0; confirmed run against the modified file (REMOTE_HAS_YAMLVALUE=1, 254 lines).Operator merges manually (no self-merge). Independent single-file PR — a finding here does not block the other formats.