UN-2742 [FIX] Show profile name instead of LLM name in Output Analyzer tabs#2030
UN-2742 [FIX] Show profile name instead of LLM name in Output Analyzer tabs#2030athul-rs wants to merge 1 commit into
Conversation
Profiles encode user intent (same model can differ by LLMWhisperer mode, chunking, etc.), so the profile name is the meaningful label when comparing outputs. The LLM model name stays available as a tooltip on each tab. Applies to the shared tab strip used by the Output Analyzer and the main combined-output view (JsonView), and the same pattern in OutputForDocModal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbit
WalkthroughTwo output components add Ant Design ChangesAdapter Model Tooltips
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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 docstrings
🧪 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 |
athul-rs
requested review from
chandrasekharan-zipstack,
hari-kuriakose and
jaseemjaskp
|
|
| Filename | Overview |
|---|---|
| frontend/src/components/custom-tools/combined-output/JsonView.jsx | Added Tooltip import; tab label priority flipped from llm_model |
| frontend/src/components/custom-tools/output-for-doc-modal/OutputForDocModal.jsx | Same Tooltip-wrapping pattern applied consistently; tab selection (handleTabChange) is keyed on index, unchanged by the label swap |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[adapterData array] --> B{profile_name truthy?}
B -- Yes --> C[Tab label = profile_name]
B -- No --> D[Tab label = llm_model]
C --> E{llm_model defined?}
D --> E
E -- Yes --> F[Tooltip shows llm_model on hover]
E -- No --> G[No tooltip rendered]
F --> H[Tab rendered in strip]
G --> H
Reviews (1): Last reviewed commit: "UN-2742 Show profile name instead of LLM..." | Re-trigger Greptile
jaseemjaskp
left a comment
jaseemjaskp
left a comment
There was a problem hiding this comment.
Automated PR review (PR Review Toolkit: code-reviewer, silent-failure-hunter, type-design-analyzer, pr-test-analyzer, comment-analyzer, code-simplifier).
Verdict: Clean, focused change — correctly implements UN-2742 (profile name as the tab label, LLM model in a hover tooltip). Change is consistent across both files. No critical/high issues. Findings below are LOW/nit.
Inline comments cover the two behavioral edge cases (blank tab + redundant/empty tooltip when fields are missing).
Additional non-blocking notes (lines outside the diff, so not inlined):
- Weak prop contract (LOW) —
JsonView.jsx:111declaresadapterData: PropTypes.array, which permits any array. The producergetLLMModelNamesForProfiles(GetStaticData.js:476-491) always emits{ profile_name, llm_model, profile_id }, so tightening toPropTypes.arrayOf(PropTypes.shape({ profile_id: PropTypes.string.isRequired, profile_name: PropTypes.string, llm_model: PropTypes.string }))would document the contract and surface dev-mode warnings.profile_idis load-bearing (tab key + selection). - Duplication (nit, no action) — the tab-label JSX is now identical in both files. A shared
ProfileTabLabelis not worth it for two call sites with no existing shared-component precedent in these sibling directories; extract only if a third call site appears. - Tests (optional) — frontend has Vitest infra but minimal coverage; this presentational change doesn't warrant new tests under current conventions. If anything, a single guard around the empty-string
profile_namefallback.
| <TabPane | ||
| tab={<span>{adapter?.llm_model || adapter?.profile_name}</span>} | ||
| tab={ | ||
| <Tooltip title={adapter?.llm_model}> |
There was a problem hiding this comment.
LOW / nit — tooltip is redundant or empty in the fallback path.
When profile_name is missing, the span already falls back to llm_model, and the tooltip title is also llm_model — so the hover just repeats the visible text. And when llm_model itself is undefined (the adapter for profile.llm doesn't resolve in getLLMModelNamesForProfiles), the tooltip is empty and hover reveals nothing. If both fields are falsy the tab renders with no label at all (a blank, anonymous-but-selectable tab) — pre-existing, but the new tooltip was a chance to address it.
Suggestion (only show the tooltip when it adds info, and guarantee a visible label):
tab={
<Tooltip title={adapter?.profile_name ? adapter?.llm_model : undefined}>
<span>{adapter?.profile_name || adapter?.llm_model || "Unnamed profile"}</span>
</Tooltip>
}(antd suppresses the popup when title is undefined.) Non-blocking.
| <TabPane | ||
| tab={<span>{adapter?.llm_model || adapter?.profile_name}</span>} | ||
| tab={ | ||
| <Tooltip title={adapter?.llm_model}> |
There was a problem hiding this comment.
LOW / nit — same as JsonView.jsx. When profile_name is absent the span and the tooltip both show llm_model (redundant); when llm_model is also undefined the tooltip is empty and a both-empty case yields a blank, anonymous tab. Mirror the same guard here to keep the two files consistent:
tab={
<Tooltip title={adapter?.profile_name ? adapter?.llm_model : undefined}>
<span>{adapter?.profile_name || adapter?.llm_model || "Unnamed profile"}</span>
</Tooltip>
}Non-blocking.
jaseemjaskp
left a comment
jaseemjaskp
left a comment
There was a problem hiding this comment.
Automated PR review (PR Review Toolkit: Code Reviewer, Silent Failure Hunter, Type Design Analyzer, Test Analyzer, Comment Analyzer, Code Simplifier).
Verdict: ship-able. The change is correct, symmetric across both files, null-safe (optional chaining), and matches the stated intent (profile name as label, model in tooltip). No blocking issues.
Two MEDIUM robustness findings are inline below. The recurring root cause is that llm_model — the field the new tooltip relies on — is exactly the field most likely to be undefined: getLLMModelNamesForProfiles (frontend/src/helpers/GetStaticData.js:484-490) derives it from adapterMap[profile?.llm], which misses whenever the profile's adapter isn't in the returned adapter list (deleted/renamed adapter, or a partial list).
Non-blocking, no inline comment posted:
- Type/data contract (weak):
adapterDatais declared only asPropTypes.array(JsonView.jsx:111); the{ profile_name, llm_model, profile_id }shape lives implicitly in the producer. The new behavior now depends onprofile_name, but that contract is expressed nowhere. Cheap fix:PropTypes.arrayOf(PropTypes.shape({ profile_name: PropTypes.string, llm_model: PropTypes.string, profile_id: PropTypes.string })), plus a JSDoc@returnstypedef on the helper (the shape also reachesOutputForDocModalvia local state, which PropTypes can't cover). - Comment (optional): A one-line comment explaining the
profile_name || llm_modellabel +llm_modeltooltip intent would guard the fallback from being misread as redundant/dead code. - Simplification: None warranted — extracting a shared
<ProfileTabLabel>for a 4-line snippet duplicated twice is disproportionate. - Tests: No test required. Frontend uses Vitest + RTL but only for non-trivial branching logic; this presentational change is consistent with leaving such code untested.
| <TabPane | ||
| tab={<span>{adapter?.llm_model || adapter?.profile_name}</span>} | ||
| tab={ | ||
| <Tooltip title={adapter?.llm_model}> |
There was a problem hiding this comment.
MEDIUM — Tooltip can render empty, and the tab label can be blank.
llm_model is the field most prone to being undefined here (lookup miss in getLLMModelNamesForProfiles, GetStaticData.js:484-490, when the profile's adapter isn't in the list). When it's undefined, <Tooltip title={undefined}> shows nothing on hover — defeating the stated goal of keeping the model name discoverable.
Separately, {adapter?.profile_name || adapter?.llm_model} (next line) renders an empty <span> when both are missing, producing a blank, unidentifiable tab (still keyed by profile_id, so output loads but the user can't tell which profile it is).
Suggest terminal fallbacks so neither is ever empty:
<Tooltip title={adapter?.llm_model || adapter?.profile_name}>
<span>
{adapter?.profile_name || adapter?.llm_model || `Profile ${adapter?.profile_id ?? "?"}`}
</span>
</Tooltip>| <TabPane | ||
| tab={<span>{adapter?.llm_model || adapter?.profile_name}</span>} | ||
| tab={ | ||
| <Tooltip title={adapter?.llm_model}> |
There was a problem hiding this comment.
MEDIUM — same robustness gap as JsonView.jsx (mirror this file's fix).
The tooltip binds to llm_model, the field most likely to be undefined (lookup miss in getLLMModelNamesForProfiles, GetStaticData.js:484-490); when absent the tooltip shows nothing on hover. And {adapter?.profile_name || adapter?.llm_model} (next line) renders an empty tab label when both fields are missing.
Apply the same terminal-fallback fix as in JsonView.jsx so neither the tooltip nor the label can be empty:
<Tooltip title={adapter?.llm_model || adapter?.profile_name}>
<span>{adapter?.profile_name || adapter?.llm_model || `Profile ${index + 1}`}</span>
</Tooltip>3 participants
What
Why
UN-2742 — users create profiles with specific intent (the same model can be used with different LLMWhisperer modes, chunking, retrieval settings). When comparing outputs across profiles, the model name is ambiguous — two tabs can read identically while being different profiles. The profile name is the meaningful identity; the model is secondary context, now preserved in a tooltip.
How
combined-output/JsonView.jsx— flipped label preference toprofile_name || llm_model, wrapped in<Tooltip title={llm_model}>. This is the shared tab strip used by both the Output Analyzer and the main combined-output view, so both stay consistent.output-for-doc-modal/OutputForDocModal.jsx— same pattern applied to the identical tab strip there.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
profile_id(JsonView) / index (OutputForDocModal), never on the label text, so behavior is unchanged — only the visible label flips. No tests, e2e selectors, or CSS reference the label text.profile_nameis a required, whitespace-validated field at profile creation, so labels can't be blank. If a profile's adapter was deleted (no resolvable model), the tooltip simply doesn't render (antd disables tooltips with undefined title) — previously that case showed the profile name with no tooltip, so nothing is lost.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependent Features
Notes on Testing
vite buildsucceeds.Screenshots
N/A
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code