feat(patches): make model customizations configurable and harden opusplan1m patching#571
Closed
liafonx wants to merge 3 commits intoPiebald-AI:mainfrom
Closed
feat(patches): make model customizations configurable and harden opusplan1m patching#571liafonx wants to merge 3 commits intoPiebald-AI:mainfrom
liafonx wants to merge 3 commits intoPiebald-AI:mainfrom
Conversation
…ogic for better clarity and functionality.
Contributor
📝 WalkthroughWalkthroughAdds a new misc setting Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as MiscView UI
participant Config as Config System
participant Patches as Patch Engine
participant FS as FileSystem
User->>UI: Toggle enableModelCustomizations
UI->>Config: updateConfigFile(settings)
Config->>FS: write config
FS-->>Config: success
Config-->>UI: ack
Note over Patches,Config: On next patch run
Patches->>Config: read settings.misc.enableModelCustomizations
alt enabled
Patches->>Patches: apply model-customizations
Patches->>Patches: apply show-more-items-in-select-menus
Patches->>FS: write patched files
else disabled
Patches->>FS: skip those patches
end
FS-->>Patches: complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/patches/opusplan1m.test.ts (1)
17-35: Add one explicit failure-path assertion for unmatched input.Current tests cover successful transforms, but a malformed/non-matching input case would lock in expected
nullbehavior and prevent silent regressions.As per coding guidelines, `**/*.test.{ts,tsx}`: Test edge cases and error conditions in test files.✅ Suggested test addition
describe('writeOpusplan1m', () => { + it('returns null when required anchors are missing', () => { + const output = writeOpusplan1m('function z(){return A;}'); + expect(output).toBeNull(); + }); + it('keeps output syntactically valid for wrapped selector returns', () => { const output = writeOpusplan1m(createInput(true));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/opusplan1m.test.ts` around lines 17 - 35, Add a test that covers the unmatched/malformed input path by calling writeOpusplan1m with an input that will not match any selectors (e.g., a modified createInput(...) or a deliberately malformed object) and assert the explicit failure behavior: check the return is null/falsey (expect(output).toBeNull() or expect(output).toBeFalsy()) and/or that attempting to compile with new Function(output!) throws when output is present but invalid; place this as a new it(...) near the existing tests referencing writeOpusplan1m and createInput so the regression for unmatched input is validated.src/patches/opusplan1m.ts (2)
111-113: Remove newly added inline explanatory comments in helper bodies.These comment additions should be dropped to stay aligned with repository rules.
Proposed cleanup
- // Pattern matches: if (VAR === "opusplan") return "<some text>"; - // Keep this strict enough to avoid consuming adjacent tokens. @@ - // Replace the full branch instead of appending to preserve statement boundaries. @@ - // Capture groups: - // 1=fullMatch, 2=conditionVar (K), 3=optional wrapper fn (v1A), 4=listVar (A), 5=funcName (Mm3) @@ - // Preserve the optional wrapper call (e.g. v1A(...)) so generated code remains valid - // for both wrapped and bare-array return forms.As per coding guidelines: “Do not add comments unless explicitly requested.”
Also applies to: 126-127, 212-213, 226-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/opusplan1m.ts` around lines 111 - 113, Remove the newly added inline explanatory comments that were inserted inside helper bodies (e.g., the comment block immediately above the const pattern declaration and the similar inline comments added inside the helper functions referenced later), leaving only code and necessary JSDoc or requested comments; update the helper implementations (including the declaration of pattern and the other helper functions that contain those explanatory comments) by deleting those comment lines so the file complies with the “no inline comments unless requested” guideline.
111-114: Regex matching is still too permissive for minified-target patching.Both updated patterns still allow broad whitespace-flex matching, which increases false-positive risk in patch selection. Tighten these to minified-token forms.
Proposed tightening
- const pattern = - /(if\s*\(\s*([$\w]+)\s*===\s*"opusplan"\s*\)\s*return\s*"[^"]*";)/; + const pattern = /(if\(([$\w]+)==="opusplan"\)return"[^"]*";)/; - const pattern = - /(if\s*\(\s*([$\w]+)\s*===\s*"opusplan"\s*\)\s*return\s*(?:([$\w]+)\()?\[\s*\.\.\.([$\w]+)\s*,\s*([$\w]+)\(\)\s*\]\)?;)/; + const pattern = + /(if\(([$\w]+)==="opusplan"\)return(?:([$\w]+)\()?\[\.\.\.([$\w]+),([$\w]+)\(\)\]\)?;)/;Based on learnings: in
src/patches/, regexes should be strict and match minified original Claude Code format rather than formatted variants.Also applies to: 212-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/opusplan1m.ts` around lines 111 - 114, The current regex in src/patches/opusplan1m.ts (const pattern) is too permissive because it allows broad \s* and formatted variants; replace it with a strict minified-token pattern that only matches minified Claude Code like if(<ident>==="opusplan")return"<text>"; (e.g., require no optional spacing and use a JS identifier token class for the variable instead of ([\$\w]+)), and apply the same tightening to the other similar regex(es) referenced around lines 212-215 so patch selection only matches minified originals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/patches/opusplan1m.test.ts`:
- Around line 17-35: Add a test that covers the unmatched/malformed input path
by calling writeOpusplan1m with an input that will not match any selectors
(e.g., a modified createInput(...) or a deliberately malformed object) and
assert the explicit failure behavior: check the return is null/falsey
(expect(output).toBeNull() or expect(output).toBeFalsy()) and/or that attempting
to compile with new Function(output!) throws when output is present but invalid;
place this as a new it(...) near the existing tests referencing writeOpusplan1m
and createInput so the regression for unmatched input is validated.
In `@src/patches/opusplan1m.ts`:
- Around line 111-113: Remove the newly added inline explanatory comments that
were inserted inside helper bodies (e.g., the comment block immediately above
the const pattern declaration and the similar inline comments added inside the
helper functions referenced later), leaving only code and necessary JSDoc or
requested comments; update the helper implementations (including the declaration
of pattern and the other helper functions that contain those explanatory
comments) by deleting those comment lines so the file complies with the “no
inline comments unless requested” guideline.
- Around line 111-114: The current regex in src/patches/opusplan1m.ts (const
pattern) is too permissive because it allows broad \s* and formatted variants;
replace it with a strict minified-token pattern that only matches minified
Claude Code like if(<ident>==="opusplan")return"<text>"; (e.g., require no
optional spacing and use a JS identifier token class for the variable instead of
([\$\w]+)), and apply the same tightening to the other similar regex(es)
referenced around lines 212-215 so patch selection only matches minified
originals.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/defaultSettings.tssrc/patches/index.tssrc/patches/modelCustomizationsToggle.test.tssrc/patches/modelSelector.tssrc/patches/opusplan1m.test.tssrc/patches/opusplan1m.tssrc/tests/config.test.tssrc/types.tssrc/ui/components/MiscView.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/patches/opusplan1m.ts (2)
111-111: Minor: Identifier pattern style differs from coding guideline recommendation.The patterns use
[$A-Za-z_][\w$]*for identifier matching:const pattern = /(if\(([$A-Za-z_][\w$]*)==="opusplan"\)return"[^"]*";)/;The coding guideline recommends using
[$\w]+for identifier matching in patch patterns. While[$A-Za-z_][\w$]*is technically more accurate for JS identifiers (requiring a non-digit first character), the simplified[$\w]+form is sufficient for minified code and provides consistency across patch files.This is a minor style consideration and doesn't affect correctness.
As per coding guidelines: "Use
[\$\\w]+instead of\\w+for identifier matching in regex patterns to include$for React refs"Also applies to: 211-211
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/opusplan1m.ts` at line 111, The regex stored in the variable pattern uses [$A-Za-z_][\w$]* for identifier matching; update the pattern to use the simplified coding-guideline form [\$\w]+ so it matches identifiers (including $) consistently across patches—replace occurrences of [$A-Za-z_][\w$]* in the pattern variable (and the second occurrence noted in this file) with [\$\w]+ while preserving the surrounding regex structure and escaping.
110-125: Description wording is inconsistent with other patches in this file.The description here uses the pattern
"X in plan mode, else Y":"Opus 4.6 in plan mode, else Sonnet 4.6 (1M context)"While lines 223-224 and 271-272 use
"Use X in plan mode, Y otherwise":"Use Opus 4.6 in plan mode, Sonnet 4.6 (1M context) otherwise"Consider aligning the wording style across all user-facing descriptions for consistency.
✏️ Suggested fix for consistent wording
const replacement = - `if(${varName}==="opusplan")return"Opus 4.6 in plan mode, else Sonnet 4.6";` + - `if(${varName}==="opusplan[1m]")return"Opus 4.6 in plan mode, else Sonnet 4.6 (1M context)";`; + `if(${varName}==="opusplan")return"Use Opus 4.6 in plan mode, Sonnet 4.6 otherwise";` + + `if(${varName}==="opusplan[1m]")return"Use Opus 4.6 in plan mode, Sonnet 4.6 (1M context) otherwise";`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/opusplan1m.ts` around lines 110 - 125, The user-facing description strings in patchDescriptionFunction are inconsistent with other patches; update the replacement assigned in patchDescriptionFunction so both return strings follow the same phrasing style (e.g., "Use Opus 4.6 in plan mode, Sonnet 4.6 otherwise") — specifically change the two literals currently produced in the replacement (the entry for ${varName}==="opusplan" and ${varName}==="opusplan[1m]") to the consistent wording ("Use Opus 4.6 in plan mode, Sonnet 4.6" and "Use Opus 4.6 in plan mode, Sonnet 4.6 (1M context) otherwise") so they match the other patches' style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/patches/opusplan1m.ts`:
- Line 111: The regex stored in the variable pattern uses [$A-Za-z_][\w$]* for
identifier matching; update the pattern to use the simplified coding-guideline
form [\$\w]+ so it matches identifiers (including $) consistently across
patches—replace occurrences of [$A-Za-z_][\w$]* in the pattern variable (and the
second occurrence noted in this file) with [\$\w]+ while preserving the
surrounding regex structure and escaping.
- Around line 110-125: The user-facing description strings in
patchDescriptionFunction are inconsistent with other patches; update the
replacement assigned in patchDescriptionFunction so both return strings follow
the same phrasing style (e.g., "Use Opus 4.6 in plan mode, Sonnet 4.6
otherwise") — specifically change the two literals currently produced in the
replacement (the entry for ${varName}==="opusplan" and
${varName}==="opusplan[1m]") to the consistent wording ("Use Opus 4.6 in plan
mode, Sonnet 4.6" and "Use Opus 4.6 in plan mode, Sonnet 4.6 (1M context)
otherwise") so they match the other patches' style.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/patches/opusplan1m.test.tssrc/patches/opusplan1m.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/patches/opusplan1m.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds configurable model customizations in TUI/config and hardens
opusplan[1m]patching.Why
opusplan1mbecause malformed patched minified JS can break Claude startup/runtime after apply.Key changes
settings.misc.enableModelCustomizationsand Misc toggle.model-customizations+show-more-items-in-select-menusby that setting.opusplan1mreplacement logic for wrapped/unwrapped returns.Validation
npx vitest run src/patches/opusplan1m.test.tspnpm lintvitest runpassed.Reviewer checklist
enableModelCustomizationswork end-to-end.true/false/unset).opusplan1mpatch still applies on current minified Claude JS shapes.nulland does not emit invalid output.Summary by CodeRabbit
New Features
Improvements
Tests