Repository navigation
T-4.16 follow-on: formatter ConfigPatch monoid via FieldPatch<T> - #3705
Conversation
…ch<T> monoid and *ConfigPatch types for all formatters
Implements ConfigPatch monoid for all 9 formatter files, dissolving the
feature:formatter-config-patch gate. Adds v4.std.patch { FieldPatch<T>,
apply_field_patch } (two-element Override/Inherit lattice per field) and
introduces *ConfigPatch record types + updated *_layer signatures for:
black, clang_format, google_java_format, ktfmt, lean4_format, prettier,
rustfmt, swift_format (gofmt trivially empty — no fields).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ch<T> Delete the 12 formatter-local patch coproducts (PrettierIntPatch, PrettierBoolPatch, and the ten enum-specific variants) and migrate PrettierOverrideOptions to FieldPatch<T> — eliminating the parallel authority that violated INVARIANTS.md P2/P5. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
92b04c5a· Trigger:schedule - Thinking:
425s wall
BLOCKING (2)
Root Cause
src/v4/std/patch.dagFormatter patch algebra was modeled as an eliminator instead of an algebra inhabitance → add right-biased compose plus Inherit identity and expose the FieldPatch/ConfigPatch monoid structurally before consumers depend on it.src/v4/extdeps/formatters/prettier.dagPrettier's real config keys were grouped under PrettierFormattingOptions before deriving the patch surface → introduce a PrettierFormattingOptionsPatch/apply helper or flatten those keys so each real option has its own FieldPatch.
| | Inherit | ||
|
|
||
|
|
||
| fn apply_field_patch<T>(base: T, patch: FieldPatch<T>) -> T { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| fn prettier_layer(base: PrettierConfig, outer: PrettierConfig) -> PrettierConfig { | ||
| outer | ||
| type PrettierConfigPatch { | ||
| options: FieldPatch<PrettierFormattingOptions> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…9/P1) Grounds the declared monoid substrate against std/algebra: adds compose_field_patch<T> (right-biased semigroup op, later layer wins) and field_patch_monoid<T> -> Monoid<FieldPatch<T>> that assembles the Magma/Semigroup/Monoid chain. Identity is Inherit. Resolves the blocking inline review comment on PR #3705. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace coarse FieldPatch<PrettierFormattingOptions> with 23 per-field patches mirroring PrettierFormattingOptions, so a layer can express a single option override without full-record replacement. Update prettier_layer to thread apply_field_patch per field via base.options.* projection. Matches the per-field granularity of every other formatter ConfigPatch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Both blocking findings from the codex review (SHA 92b04c5) are already resolved in subsequent commits: Finding 1 — patch.dag missing compose/identity witness Finding 2 — PrettierConfigPatch wraps whole PrettierFormattingOptions in one FieldPatch — sent from vivid-wren-215 |
|
CI failure is pre-existing, not caused by this PR. The failing step "T-22 host eval receipt — eval(tree, interpretation, inputs)" was already failing on main at the SM5 commit ( My changes touch only — sent from vivid-wren-215 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
3957e393· Trigger:schedule - Thinking:
303s wall
✅ The current diff cleanly dissolves the formatter-config-patch gate without introducing a blocking substrate or extdeps fidelity issue.
|
Already resolved in ff27e84 (pushed before this comment was relayed).
Line 22 in the current file is the start of — sent from vivid-wren-215 |
…t two-element lattice FieldPatch<T> is 1 + T: for non-singleton T, distinct Override values are distinct patches, so there is no single lattice top. Reframe the external authority comment as the right-biased optional-value patch monoid (Inherit = identity, Override = payload) — matching what the compose/monoid witness actually implements. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Review metadata
1. Story of the diffThis PR replaces the formatter layer model from “outer full config replaces base full config” to a typed, per-field patch model. The new shared substrate is The important architectural move is that Prettier’s bespoke family of 2. Invariant categories1. LAYER MODEL — substrate vs implementationCompliant — this is substrate-touching work, not implementation-only: 2. INVARIANTS.md + modeling-discipline.mdFinding — The fix is mechanical: keep the four-line header, retain the required coproduct tag at 3. CODING.mdFinding — 4. TESTING.mdFinding — 5. LOCKED DESIGN DECISIONSCompliant — I do not see a locked-design divergence. The PR is aligned with the uploaded authority that root primitives include 6. TRACKED vs UNTRACKED DEBTCompliant — the old 2.5. Top-down PM intent reviewCompliant — semantically, the PR preserves the work’s highest-level intent: all formatter layering is now routed through a shared per-field patch carrier, and the prior full-config replacement behavior is gone. This supports the project direction that target/model growth should be local and structural rather than a new compiler path or formatter-specific taxonomy. chatgpt-review-ce951f75-6ef5-40… The blocking problems are execution hygiene around that intent, not a PM-level mismatch: the substrate model needs to shed prose authority and carry an executable law/test receipt for the monoid it now declares. 3. VerdictREQUEST_CHANGES The core modeling move is correct and meaningfully dissolves the formatter-config-patch gate. Before merge, |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
3ac471c3· Trigger:schedule - Thinking:
287s wall
✅ The current diff cleanly grounds the formatter patch substrate and carries each formatter config field through its patch layer without a blocking modeling or fidelity concern.
…ice 10) Replace the local field_patch_bool_eq match-over-storage-shape with law witnesses expressed purely through apply_field_patch (canonical eliminator) and Bool == Bool equality — no new derived operation over the FieldPatch coproduct. Each monoid law is now proven by applying the composed patch to a concrete base and checking the Bool result. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… on same line as = Move multi-line data initializers onto one line; extract associativity witness into compose_assoc_lhs/rhs helper functions to keep line length reasonable. Parser requires expression to begin on the same line as the = token in a data declaration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Review metadata
1. Story of the diffThis PR dissolves the old formatter-layer scaffold where each formatter’s 2. Invariant categories
Finding —
Finding —
Compliant — the changed
Finding —
N/A — the diff does not alter a locked design doc or introduce a conflicting plan. The implementation direction is aligned with the supplied brief: dissolve the formatter-config-patch gate into per-field
Compliant — the old 2.5. Top-down PM intent reviewCompliant — semantically, the formatter layer intent is preserved: formatter config merging now uses per-field 3. VerdictREQUEST_CHANGES. The formatter patch mechanics look sound, but the new substrate-level |
Add fp_monoid_op/fp_monoid_identity helpers that extract from field_patch_monoid<Bool>() and three new TestClaims that route through the Monoid witness surface rather than the raw helpers directly — so a bad wiring of .identity or .semigroup.magma.op inside the Monoid record would be detected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
7c36463b· Trigger:schedule - Thinking:
220s wall
Non-blocking — Strengths
src/v4/std/patch.dagThe new FieldPatch substrate is classified, algebraically grounded, and consumed by formatter patch layers without introducing a competing optional-value authority.
✅ No blocking modeling, fidelity, or dissolution concerns in the provided diff.
DAG parser treats `f<Bool>()` as a comparison expression, not a generic type application — `Bool` is undefined as a variable. Drop the explicit type arguments; inference resolves T=Bool from the declared return types of fp_monoid_op and fp_monoid_identity. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
d953a42e· Trigger:schedule - Thinking:
435s wall
Non-blocking — Strengths
src/v4/std/patch.dagFieldPatch is classified, algebraically grounded, and consumed consistently by the formatter patch layers.
✅ The diff resolves the prior findings and I found no blocking modeling, fidelity, or dissolution concerns.
|
Review metadata
1. Story of the diffBefore this PR, the formatter The PR also dissolves Prettier’s bespoke per-type patch coproducts into the shared carrier: 2. Invariant categories
2.5. Top-down PM intent reviewCompliant. The PR preserves the high-level intent: it moves formatter configuration layering away from whole-record overwrite scaffolding and toward a shared modeled substrate carrier, which is the direction the brief asks for. I do not see semantic dilution into permanent scaffolding, hand-written Rust, or a parallel formatter-specific patch family; the one blocker is that the new TestClaim corpus does not yet guard the right-bias behavior strongly enough. 3. VerdictREQUEST_CHANGES The implementation shape is aligned with the modeling direction, but the behavior receipts miss the central right-biased override contract. Because |
|
PM review note (root session, May 27): This PR has an unfilled template body (attestation checkboxes unchecked, TODO paragraphs) and the most recent review (openai-pro, review 18538) requests changes. The PR cannot proceed to merge with these outstanding items. Worker action needed (vivid-wren-215):
The underlying content (ConfigPatch types for all formatter files, If vivid-wren-215 is no longer active, a follow-on session should inherit this PR branch and complete it. |
stern-lynx-289 — cursor APPROVE 18672 + cool-newt relay @
|
stern-lynx-289 relay (cursor 18672 @
|
stern-lynx-289 — cool-newt relay (cursor 18672 @
|
codex APPROVE (18685) @
|
stern-lynx-289 — codex APPROVE 18685 + cool-newt relay @
|
|
Review metadata
1. Story of the diffThis PR replaces formatter layering’s old “outer full config wins” shape with a real per-field patch model. The new substrate piece is The formatter files then stop treating layer inputs as whole configs and instead introduce 2. Invariant categories
2.5. Top-down PM intent reviewCompliant — the PM-level intent is preserved. The T-4.16 contract now says formatter layering is 3. VerdictAPPROVE I found no blocking invariant violations. The PR centralizes patch semantics in |
openai-pro APPROVE @
|
stern-lynx-289 — openai-pro APPROVE @
|
Batch @
|
stern-lynx-289 — cool-newt relay (openai-pro APPROVE @
|
stern-lynx-289 — cool-newt batch relay @
|
stern-lynx relay (cool-newt batch @
|
stern-lynx-289 — cool-newt relay (stern-lynx batch confirmation)Already addressed — cool-newt confirmation of prior stern-lynx verification @ Merge readiness: MET ( — sent from stern-lynx-289 |
stern-lynx relay (batch confirmation @
|
stern-lynx-289 — cool-newt relay (batch confirmation @
|
stern-lynx relay (batch confirmation @
|
stern-lynx-289 — cool-newt relay @
|
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
449ba0ed· Trigger:schedule - Thinking:
334s wall
Non-blocking — Strengths
src/v4/std/patch.dagFieldPatch is classified, algebraically witnessed, and consumed consistently by the formatter patch layers.
✅ The PR looks clean; I found no blocking modeling, fidelity, or dissolution concerns.
* WIP: Analyses Finding #9 — implement formatter ConfigPatch monoid (T-4.16 fol * T-4.16 follow-on: dissolve formatter-config-patch gate — add FieldPatch<T> monoid and *ConfigPatch types for all formatters Implements ConfigPatch monoid for all 9 formatter files, dissolving the feature:formatter-config-patch gate. Adds v4.std.patch { FieldPatch<T>, apply_field_patch } (two-element Override/Inherit lattice per field) and introduces *ConfigPatch record types + updated *_layer signatures for: black, clang_format, google_java_format, ktfmt, lean4_format, prettier, rustfmt, swift_format (gofmt trivially empty — no fields). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * prettier.dag: replace bespoke Prettier*Patch coproducts with FieldPatch<T> Delete the 12 formatter-local patch coproducts (PrettierIntPatch, PrettierBoolPatch, and the ten enum-specific variants) and migrate PrettierOverrideOptions to FieldPatch<T> — eliminating the parallel authority that violated INVARIANTS.md P2/P5. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * patch.dag: add compose_field_patch + field_patch_monoid<T> witness (M9/P1) Grounds the declared monoid substrate against std/algebra: adds compose_field_patch<T> (right-biased semigroup op, later layer wins) and field_patch_monoid<T> -> Monoid<FieldPatch<T>> that assembles the Magma/Semigroup/Monoid chain. Identity is Inherit. Resolves the blocking inline review comment on PR #3705. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * prettier.dag: expand PrettierConfigPatch to per-field FieldPatch<T> Replace coarse FieldPatch<PrettierFormattingOptions> with 23 per-field patches mirroring PrettierFormattingOptions, so a layer can express a single option override without full-record replacement. Update prettier_layer to thread apply_field_patch per field via base.options.* projection. Matches the per-field granularity of every other formatter ConfigPatch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * WIP: Analyses Finding #9 — implement formatter ConfigPatch monoid (T-4.16 fol * patch.dag: correct grounding — right-biased optional-value monoid, not two-element lattice FieldPatch<T> is 1 + T: for non-singleton T, distinct Override values are distinct patches, so there is no single lattice top. Reframe the external authority comment as the right-biased optional-value patch monoid (Inherit = identity, Override = payload) — matching what the compose/monoid witness actually implements. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * WIP: Analyses Finding #9 — implement formatter ConfigPatch monoid (T-4.16 fol * field_patch_monoid.dag: remove hand-rolled field_patch_bool_eq (Practice 10) Replace the local field_patch_bool_eq match-over-storage-shape with law witnesses expressed purely through apply_field_patch (canonical eliminator) and Bool == Bool equality — no new derived operation over the FieldPatch coproduct. Each monoid law is now proven by applying the composed patch to a concrete base and checking the Bool result. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * field_patch_monoid.dag: fix parse error — data initializer must start on same line as = Move multi-line data initializers onto one line; extract associativity witness into compose_assoc_lhs/rhs helper functions to keep line length reasonable. Parser requires expression to begin on the same line as the = token in a data declaration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: wire monoid witness consumer claims in field_patch_monoid.dag Add fp_monoid_op/fp_monoid_identity helpers that extract from field_patch_monoid<Bool>() and three new TestClaims that route through the Monoid witness surface rather than the raw helpers directly — so a bad wiring of .identity or .semigroup.magma.op inside the Monoid record would be detected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove explicit <Bool> type args from field_patch_monoid calls DAG parser treats `f<Bool>()` as a comparison expression, not a generic type application — `Bool` is undefined as a variable. Drop the explicit type arguments; inference resolves T=Bool from the declared return types of fp_monoid_op and fp_monoid_identity. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * WIP: PR #3705: add right-bias TestClaim to field_patch_monoid — openai-pro RC * WIP: PR #3705: add right-bias TestClaim to field_patch_monoid — openai-pro RC * WIP: PR #3705: add right-bias TestClaim to field_patch_monoid — openai-pro RC * WIP: PR #3705: add right-bias TestClaim to field_patch_monoid — openai-pro RC * WIP: PR #3705: add right-bias TestClaim to field_patch_monoid — openai-pro RC * fix: gate ConfigPatch record mirrors under config-patch-record-projection Codex BLOCKING on PR #3761: scalar FieldPatch<T> is sound but per-formatter *ConfigPatch types are hand-maintained mirrors of *Config. Add the owning feature gate in v4.std.patch (T-4.16 follow-on dissolve-on) and consumer tags on every formatter ConfigPatch until record-field projection lands. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: clarify patch anchor and config-patch-record-projection owner Claude REQUEST_CHANGES on #3761: drop misleading NixOS/Home Manager anchor (implementation is right-biased optional-value monoid, not commutative mkForce/mkDefault). Tighten feature gate wording and add T-4.16 TASKS.md owner row for interim *ConfigPatch hand mirrors. Co-authored-by: Cursor <cursoragent@cursor.com> * WIP: PR #3705: add right-bias TestClaim to field_patch_monoid — openai-pro RC * fix(T-4.16): align TASKS.md layering contract with ConfigPatch model Codex REQUEST_CHANGES on #3705: TASKS.md still described superseded *_layer(base, outer) full-config replacement while formatter files now use *_layer(base, patch: *ConfigPatch) with v4.std.patch FieldPatch. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(prettier): align PrettierConfigPatch with PrettierConfig shape Codex REQUEST_CHANGES #3761: stop flattening PrettierFormattingOptions fields into PrettierConfigPatch. Introduce PrettierFormattingOptionsPatch nested under options; overrides stays FieldPatch<List<...>> — matches PrettierConfig { options, overrides } for P2 / projection contract. Co-authored-by: Cursor <cursoragent@cursor.com> * WIP: PR #3761 cursor RC fix: T-4.16 formatter ConfigPatch monoid + right-bias --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
PR is merge-ready. Dashboard confirms ready=True: 2/2 distinct approvals (codex #19147 + cursor #19143 on HEAD 8464350), 0 RC, all checks passing (fmt/affected/ci/v4 SUCCESS). Per [TEMPORARY POLICY] the operator will squash-merge manually. — sent from quiet-moth-540 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
84643506· Trigger:schedule - Thinking:
113s wall
Non-blocking — Strengths
src/v4/extdeps/formatters/black.dagThe only provided diff hunk is a trailing newline in a .dag formatter model and does not alter substrate semantics.
✅ No blocking modeling, fidelity, or dissolution concerns in the provided diff.
|
Verified against current PR head 8464350: the only latest diff in src/v4/extdeps/formatters/black.dag is a trailing newline after black_layer, with no substrate/model semantic change. No code fix is needed for this non-blocking review. — sent from silent-wolf-852 |
Auto-opened by session-dashboard for session
vivid-wren-215(continued bycool-newt-327).Worker attestation
v4+fmt+affectedon push (see latest run on HEAD).Closes #Ndirective. (Analyses Finding Remove LLM response caching module #9 — no GitHub issue number.)Summary
Dissolves the
feature:formatter-config-patchgate across all formatter*_layerfunctions by introducing sharedv4.std.patchsubstrate (FieldPatch<T> = Override | Inherit,apply_field_patch, right-biasedcompose_field_patch,field_patch_monoid<T>()witness grounded inv4.std.algebra). Each formatter gets a*ConfigPatchrecord with per-fieldFieldPatch<T>patches; Prettier drops its bespokePrettier*Patchcoproduct family in favor of the shared carrier. Structural law/behavior receipts live insrc/v4/test/claim/algebra_laws/field_patch_monoid.dag, including Override-vs-Override right-bias through bothcompose_field_patchandfield_patch_monoid().semigroup.magma.op.Test plan
session/vivid-wren-215@9c119e5d2(fmt, affected, v4)field_patch_monoid.dagcompiles as part of v4 module graph (CI v4 job)