[#4638] fix(frontend): Preserve string type when view mode is Text#4658
[#4638] fix(frontend): Preserve string type when view mode is Text#4658IshaanAggrawal wants to merge 1 commit into
Conversation
|
@IshaanAggrawal is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
|
dosubot
Bot
added
the
size:S
|
✅ Thanks @IshaanAggrawal! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon. |
github-actions
Bot
added
the
incomplete-pr
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughRemoved primitive inference: ChangesTextEditor primitive inference removal
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
github-actions
Bot
removed
the
incomplete-pr
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-entity-ui/src/testcase/TestcaseDrillInFieldRenderer.tsx (1)
251-260:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMarkdown path misses
viewModepropagation, so coercion still occurs.On Line 253,
TextEditoris rendered for markdown mode, butviewModeis not passed. BecausehandleChange(Lines 129-135) only preserves raw strings whenviewMode === "text" || viewMode === "markdown", this branch still runsinferPrimitiveFromText, breaking the markdown requirement.Suggested fix
if (viewMode === "markdown") { return ( <TextEditor editorId={editorId} value={value} displayValue={displayValue} markdown onChange={onChange} readOnly={!editable} + viewMode={viewMode} /> ) }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f4df9985-b966-4384-91eb-bdd9cd947f9e
📒 Files selected for processing (1)
web/packages/agenta-entity-ui/src/testcase/TestcaseDrillInFieldRenderer.tsx
IshaanAggrawal
force-pushed
the
fix/4638-text-mode-no-type-coercion
branch
from
01b7a58 to
e343953
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-entity-ui/src/testcase/TestcaseDrillInFieldRenderer.tsx (1)
223-227:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInline comment no longer matches implementation.
This block says native typing is preserved via
inferPrimitiveFromTextinTextEditor.handleChange, but that path was removed. Please update/remove the comment to avoid future misreads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 730b35dd-536e-4346-bf01-7500f8253148
📒 Files selected for processing (1)
web/packages/agenta-entity-ui/src/testcase/TestcaseDrillInFieldRenderer.tsx
| const handleChange = useCallback( | ||
| (next: string) => onChange(inferPrimitiveFromText(next)), | ||
| (next: string) => onChange(next), | ||
| [onChange], | ||
| ) |
There was a problem hiding this comment.
Fallback coercion behavior was unintentionally removed.
handleChange now always emits strings, which breaks the documented fallback behavior when viewMode is unset (auto-coercion should still occur). This changes runtime behavior beyond text/markdown and can regress type chips/state for default mode.
Suggested fix
-import {parseCodeString, toCodeString} from "./codeFormat"
+import {inferPrimitiveFromText, parseCodeString, toCodeString} from "./codeFormat"
...
- const handleChange = useCallback(
- (next: string) => onChange(next),
- [onChange],
- )
+ const handleChange = useCallback(
+ (next: string) => {
+ const preserveAsString = viewMode === "text" || viewMode === "markdown"
+ onChange(preserveAsString ? next : inferPrimitiveFromText(next))
+ },
+ [onChange, viewMode],
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleChange = useCallback( | |
| (next: string) => onChange(inferPrimitiveFromText(next)), | |
| (next: string) => onChange(next), | |
| [onChange], | |
| ) | |
| const handleChange = useCallback( | |
| (next: string) => { | |
| const preserveAsString = viewMode === "text" || viewMode === "markdown" | |
| onChange(preserveAsString ? next : inferPrimitiveFromText(next)) | |
| }, | |
| [onChange, viewMode], | |
| ) |
IshaanAggrawal
force-pushed
the
fix/4638-text-mode-no-type-coercion
branch
from
e343953 to
f96576e
Compare
2 participants
Summary
What changed:
TextEditornow respects the activeviewMode. When the user explicitly selects "Text" or "Markdown" from the dropdown, it passes the raw string through without coercion.Why was this change needed: Typing values like
Falseor5in Text mode was incorrectly mutating the variable type.What problem does it solve: Fixes an issue in the playground test case drawer where type chips would improperly update to
booleanornumberwhen the user intended for them to stay as strings.Root cause:
TextEditor.handleChangeinTestcaseDrillInFieldRenderer.tsxwas unconditionally callinginferPrimitiveFromText()on every keystroke, regardless of the view mode. Fixes #4638Verified locally
viewMode === "text"andviewMode === "markdown", all inputs (e.g.False,5,true) remain strings.falsebecomes boolean,5becomes number).##Demo
QA follow-up
False. The type chip should staystringand the stored value should be"False".5,true,0. All should remain strings.false. The type chip should update toboolean.5. The type chip should update tonumber.falseor5should still auto-detect as boolean/number.