UN-2742 [FIX] Show profile name instead of LLM name in Output Analyzer tabs - #2030
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughTwo output components add Ant Design tooltips to adapter tabs, show the LLM model on hover, and prefer profile names for visible labels with model and generated profile fallbacks. ChangesAdapter tab labels
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
|
| Filename | Overview |
|---|---|
| frontend/src/components/custom-tools/combined-output/JsonView.jsx | Adds Tooltip wrapper around tab labels; flips label preference to profile_name with llm_model as tooltip. Logic is correct; minor observation on tooltip fallback. |
| frontend/src/components/custom-tools/output-for-doc-modal/OutputForDocModal.jsx | Mirrors the same tab label / tooltip flip from JsonView; uses index-based fallback label instead of profile_id-based, consistent with its pre-existing key strategy. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[adapter object] --> B{profile_name present?}
B -- Yes --> C[Label = profile_name]
B -- No --> D{llm_model present?}
D -- Yes --> E["Label = llm_model"]
D -- No --> F["Label = Profile {id/index}"]
A --> G{llm_model present?}
G -- Yes --> H["Tooltip = llm_model"]
G -- No --> I["Tooltip = profile_name (redundant) or undefined"]
C --> J[Render TabPane]
E --> J
F --> J
H --> J
I --> J
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[adapter object] --> B{profile_name present?}
B -- Yes --> C[Label = profile_name]
B -- No --> D{llm_model present?}
D -- Yes --> E["Label = llm_model"]
D -- No --> F["Label = Profile {id/index}"]
A --> G{llm_model present?}
G -- Yes --> H["Tooltip = llm_model"]
G -- No --> I["Tooltip = profile_name (redundant) or undefined"]
C --> J[Render TabPane]
E --> J
F --> J
H --> J
I --> J
Reviews (3): Last reviewed commit: "Merge branch 'main' into UN-2742-profile..." | 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.
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.
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>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
677940f to
83ba681
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Unstract test resultsPer-group results
Critical paths❌ Regressions (must be zero)
|
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