-
Notifications
You must be signed in to change notification settings - Fork 65
Add a patch to make thinking blocks italic and dim again #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughA new patch utility is introduced to transform React code, wrapping a "Thinking…" label in styled Box and Text components. The patch is integrated into the existing patch pipeline in the index file. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/patches/thinkingLabel.ts (1)
81-81: Consider adding a guard for the identifiers array.The non-null assertion on
location.identifiers![2]is currently safe becausefindThinkingLabelLocationalways populates the array when returning non-null. However, for defensive coding, you could add a guard:🔧 Optional defensive check
+ if (!location.identifiers || location.identifiers.length < 3) { + console.error('patch: thinkingLabel: missing identifiers from location'); + return null; + } + - const thinkingTextVar = location.identifiers![2]; + const thinkingTextVar = location.identifiers[2];
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/patches/index.tssrc/patches/thinkingLabel.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/patches/index.ts (1)
src/patches/thinkingLabel.ts (1)
writeThinkingLabel(56-101)
src/patches/thinkingLabel.ts (1)
src/patches/index.ts (5)
LocationResult(73-77)findBoxComponent(424-450)findTextComponent(407-419)getReactVar(258-314)showDiff(91-129)
🔇 Additional comments (4)
src/patches/index.ts (2)
66-66: LGTM!The import follows the established pattern used throughout the file.
609-611: LGTM!The patch integration follows the established pattern used by other patches. The placement after
writeThinkingVisibilityis logical since both patches relate to thinking UI styling.src/patches/thinkingLabel.ts (2)
18-51: LGTM!The implementation follows codebase conventions:
- Uses
[$\w]+pattern for identifiers as recommended in the patch-writing notes- Error handling with
console.erroris consistent with other component finders- The 200-char search window is a reasonable heuristic for minified code proximity
83-98: LGTM!The replacement construction and string manipulation are correct. The
showDiffcall matches the expected signature from the index module.
Resolves #353 (comment)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.