feat(ui): inline result charts for agent answers#330
Conversation
Render agent-authored ```chart / ```chart-carousel JSON specs as themed inline SVG so numeric answers (rankings, trends, distributions, gains/losses, single metrics) read as charts, not walls of prose or tables. - New self-contained ResultChart module (frontends/ui/.../ResultChart): a Zod spec contract, pure parse/normalize/scale/geometry/render layers, and a thin SVG shell. Chart types: bar, hbar, line, area, grouped-bar, delta, KPI-only, and a pageable chart-carousel. Per-chart "Show data" toggle and CSV export (CSV neutralizes spreadsheet formula injection). - MarkdownRenderer routes ```chart fences to the renderer and falls back to a code block when a spec is malformed; multiple charts per answer are supported. - Safe-by-default rendering: zero-baseline magnitude bars, compacted axis ticks, disclosed top-N truncation for rankings, gaps (never zero) for missing points. - Generic chart contract added to the shallow researcher and deep-research writer prompts; a pytest guards the embedded examples against schema drift. - 100% statement/branch/function/line coverage for the module, enforced by a scoped vitest threshold. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
WalkthroughChangesResult chart pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AgentPrompt
participant MarkdownRenderer
participant ChartBlock
participant ResultChart
participant ChartToolbar
participant ChartDataTable
AgentPrompt->>MarkdownRenderer: emit chart or chart-carousel JSON
MarkdownRenderer->>ChartBlock: normalize and pass fence contents
ChartBlock->>ResultChart: parse and render chart specification
ResultChart->>ChartToolbar: render data and CSV controls
ChartToolbar->>ChartDataTable: toggle accessible data table
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)frontends/ui/src/app/globals.cssFile contains syntax errors that prevent linting: Line 155: Tailwind-specific syntax is disabled.; Line 168: Tailwind-specific syntax is disabled.; Line 172: Tailwind-specific syntax is disabled.; Line 176: Tailwind-specific syntax is disabled.; Line 180: Tailwind-specific syntax is disabled.; Line 184: Tailwind-specific syntax is disabled.; Line 188: Tailwind-specific syntax is disabled.; Line 192: Tailwind-specific syntax is disabled.; Line 196: Tailwind-specific syntax is disabled.; Line 200: Tailwind-specific syntax is disabled.; Line 208: Tailwind-specific syntax is disabled.; Line 212: Tailwind-specific syntax is disabled.; Line 216: Tailwind-specific syntax is disabled.; Line 220: Tailwind-specific syntax is disabled.; Line 225: Tailwind-specific syntax is disabled.; Line 231: Tailwind-specific syntax is disabled.; Line 235: Tailwind-specific syntax is disabled.; Line 239: Tailwind-specific syntax is disabled.; Line 247: Tailwind-specific syntax is disabled.; Line 251: Tailwind-specific syntax is disabled.; Line 255: Tailwind-specific syntax is disabled.; Line 259: Tailwind-specific syntax is disabled.; Line 263: Tailwind-specific syntax is disabled.; Line 267: Tailwind-specific syntax is disabled.; Line 271: Tailwind-specific syntax is disabled.; Line 275: Tailwind-specific syntax is disabled.; Line 279: Tailwind-specific syntax is disabled.; Line 283: Tailwind-specific syntax is disabled. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/ui/src/app/globals.css`:
- Around line 391-396: Update the .result-chart-kpi.tone-warn and
.result-chart-kpi.tone-alarm styles to use the existing semantic feedback theme
tokens for warning and danger text colors instead of hardcoded hex values,
preserving the current tone-specific behavior.
In `@frontends/ui/src/shared/components/ResultChart/ChartBlock.tsx`:
- Around line 19-35: Memoize the parsing and normalization pipeline inside
ChartBlock using raw as the dependency, including parseCarouselSpec,
parseChartSpec, degenerateKpis, normalizeChart, and parseKpiSpec. Keep the
existing rendering precedence and fallback behavior unchanged while returning
the memoized result for unchanged raw content.
In `@frontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsx`:
- Around line 17-19: Move the URL global cleanup from the test body into the
existing ChartToolbar afterEach hook, ensuring vi.unstubAllGlobals() runs even
when assertions fail. Keep the cleanup alongside vi.restoreAllMocks() and remove
the now-redundant inline cleanup from the affected test.
In `@frontends/ui/src/shared/components/ResultChart/ChartToolbar.tsx`:
- Around line 18-26: Connect the disclosure control to its data region: in
frontends/ui/src/shared/components/ResultChart/ChartToolbar.tsx lines 18-26, add
a matching aria-controls value to the toggle button; in
frontends/ui/src/shared/components/ResultChart/ChartDataTable.tsx line 13, add
the same value as an id on the role="region" div.
In `@frontends/ui/src/shared/components/ResultChart/renderers/bar.ts`:
- Around line 41-43: Update the single-series value-label placement in the bar
renderer around the marks.push call to branch on value >= 0: keep positive
labels above the bar, but place negative labels below its tip with the
appropriate offset and anchor, matching the behavior of renderHbar. Ensure
labels remain connected to the bar for both value directions.
In `@frontends/ui/src/shared/components/ResultChart/renderers/hbar.ts`:
- Around line 19-46: Align delta chart legend behavior with the sign-based bar
colors in renderHbar: update the ResultChart legend logic to suppress the
per-series legend when spec.type is "delta" (or otherwise provide a sign-based
legend). Preserve the existing series legend for non-delta charts and keep the
current GAIN/LOSS bar coloring unchanged.
In `@frontends/ui/src/shared/components/ResultChart/ResultChart.tsx`:
- Around line 136-180: Update the interactive rect and dot marks in the
ResultChart rendering switch to be keyboard focusable and expose the same
tooltip behavior on focus and blur as on mouse movement and leave. Reuse the
existing tipHandlers integration and preserve the ChartDataTable fallback and
current visual rendering.
In `@src/aiq_agent/agents/shallow_researcher/prompts/researcher.j2`:
- Around line 36-60: Extract the duplicated “Presenting Data (Charts)” guidance
into a shared Jinja partial, preserving the chart contract, schema, and examples
byte-for-byte; update
src/aiq_agent/agents/shallow_researcher/prompts/researcher.j2 lines 36-60 and
src/aiq_agent/agents/deep_researcher/prompts/writer.j2 lines 105-129 to include
it, using a template variable only for required per-agent wording differences.
In `@tests/aiq_agent/agents/test_result_chart_prompt.py`:
- Around line 36-42: Extend the prompt contract tests to extract and validate
chart-carousel examples, not just detect the substring. Add a carousel fence
matcher and a helper alongside _chart_examples, then update
test_prompt_chart_examples_match_the_schema to require a non-empty charts list
and apply the existing full-chart schema assertions to every carousel entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: adfe0d1e-8feb-4ff6-84b9-aa0410ecbf62
📒 Files selected for processing (44)
frontends/ui/src/app/globals.cssfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.chart.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/ResultChart/ChartBlock.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/ChartKpi.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartKpi.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxfrontends/ui/src/shared/components/ResultChart/ResultChart.tsxfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.spec.tsxfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.tsxfrontends/ui/src/shared/components/ResultChart/export-csv.spec.tsfrontends/ui/src/shared/components/ResultChart/export-csv.tsfrontends/ui/src/shared/components/ResultChart/geometry.spec.tsfrontends/ui/src/shared/components/ResultChart/geometry.tsfrontends/ui/src/shared/components/ResultChart/index.tsfrontends/ui/src/shared/components/ResultChart/normalize.spec.tsfrontends/ui/src/shared/components/ResultChart/normalize.tsfrontends/ui/src/shared/components/ResultChart/palette.spec.tsfrontends/ui/src/shared/components/ResultChart/palette.tsfrontends/ui/src/shared/components/ResultChart/parse.spec.tsfrontends/ui/src/shared/components/ResultChart/parse.tsfrontends/ui/src/shared/components/ResultChart/renderers/axis.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/axis.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.tsfrontends/ui/src/shared/components/ResultChart/renderers/hbar.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/hbar.tsfrontends/ui/src/shared/components/ResultChart/renderers/index.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/index.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.tsfrontends/ui/src/shared/components/ResultChart/renderers/types.tsfrontends/ui/src/shared/components/ResultChart/scale.spec.tsfrontends/ui/src/shared/components/ResultChart/scale.tsfrontends/ui/src/shared/components/ResultChart/types.tsfrontends/ui/vitest.config.tssrc/aiq_agent/agents/deep_researcher/prompts/writer.j2src/aiq_agent/agents/shallow_researcher/prompts/researcher.j2tests/aiq_agent/agents/test_result_chart_prompt.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/shared/components/ResultChart/palette.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.chart.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.spec.tsxfrontends/ui/vitest.config.tsfrontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/renderers/types.tsfrontends/ui/src/shared/components/ResultChart/renderers/index.tsfrontends/ui/src/shared/components/ResultChart/ChartKpi.tsxfrontends/ui/src/shared/components/ResultChart/renderers/index.spec.tsfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/geometry.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/shared/components/ResultChart/ChartKpi.spec.tsxfrontends/ui/src/shared/components/ResultChart/renderers/hbar.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.spec.tsfrontends/ui/src/shared/components/ResultChart/types.tsfrontends/ui/src/shared/components/ResultChart/export-csv.tsfrontends/ui/src/shared/components/ResultChart/normalize.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.tsxfrontends/ui/src/shared/components/ResultChart/renderers/bar.tsfrontends/ui/src/shared/components/ResultChart/geometry.tsfrontends/ui/src/shared/components/ResultChart/palette.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxfrontends/ui/src/shared/components/ResultChart/export-csv.spec.tsfrontends/ui/src/shared/components/ResultChart/parse.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.tsxfrontends/ui/src/shared/components/ResultChart/renderers/hbar.tsfrontends/ui/src/shared/components/ResultChart/index.tsfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartBlock.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/ResultChart/renderers/axis.tsfrontends/ui/src/shared/components/ResultChart/scale.spec.tsfrontends/ui/src/shared/components/ResultChart/scale.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.tsfrontends/ui/src/shared/components/ResultChart/parse.tsfrontends/ui/src/shared/components/ResultChart/normalize.tsfrontends/ui/src/shared/components/ResultChart/renderers/axis.spec.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/shared/components/ResultChart/palette.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.chart.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.spec.tsxfrontends/ui/vitest.config.tsfrontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/renderers/types.tsfrontends/ui/src/shared/components/ResultChart/renderers/index.tsfrontends/ui/src/shared/components/ResultChart/ChartKpi.tsxfrontends/ui/src/shared/components/ResultChart/renderers/index.spec.tsfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/geometry.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/shared/components/ResultChart/ChartKpi.spec.tsxfrontends/ui/src/shared/components/ResultChart/renderers/hbar.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.spec.tsfrontends/ui/src/shared/components/ResultChart/types.tsfrontends/ui/src/shared/components/ResultChart/export-csv.tsfrontends/ui/src/shared/components/ResultChart/normalize.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.tsxfrontends/ui/src/shared/components/ResultChart/renderers/bar.tsfrontends/ui/src/shared/components/ResultChart/geometry.tsfrontends/ui/src/shared/components/ResultChart/palette.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxfrontends/ui/src/shared/components/ResultChart/export-csv.spec.tsfrontends/ui/src/shared/components/ResultChart/parse.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.tsxfrontends/ui/src/shared/components/ResultChart/renderers/hbar.tsfrontends/ui/src/shared/components/ResultChart/index.tsfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartBlock.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/ResultChart/renderers/axis.tsfrontends/ui/src/shared/components/ResultChart/scale.spec.tsfrontends/ui/src/shared/components/ResultChart/scale.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.tsfrontends/ui/src/shared/components/ResultChart/parse.tsfrontends/ui/src/shared/components/ResultChart/normalize.tsfrontends/ui/src/shared/components/ResultChart/renderers/axis.spec.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/shared/components/ResultChart/palette.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.chart.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.spec.tsxfrontends/ui/vitest.config.tsfrontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/renderers/types.tsfrontends/ui/src/shared/components/ResultChart/renderers/index.tsfrontends/ui/src/shared/components/ResultChart/ChartKpi.tsxfrontends/ui/src/shared/components/ResultChart/renderers/index.spec.tsfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/geometry.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/shared/components/ResultChart/ChartKpi.spec.tsxfrontends/ui/src/shared/components/ResultChart/renderers/hbar.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.spec.tsfrontends/ui/src/shared/components/ResultChart/types.tsfrontends/ui/src/shared/components/ResultChart/export-csv.tsfrontends/ui/src/shared/components/ResultChart/normalize.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.tsxfrontends/ui/src/shared/components/ResultChart/renderers/bar.tsfrontends/ui/src/shared/components/ResultChart/geometry.tsfrontends/ui/src/shared/components/ResultChart/palette.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxfrontends/ui/src/shared/components/ResultChart/export-csv.spec.tsfrontends/ui/src/shared/components/ResultChart/parse.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.tsxfrontends/ui/src/shared/components/ResultChart/renderers/hbar.tsfrontends/ui/src/app/globals.cssfrontends/ui/src/shared/components/ResultChart/index.tsfrontends/ui/src/shared/components/ResultChart/ResultChartCarousel.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartBlock.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/ResultChart/renderers/axis.tsfrontends/ui/src/shared/components/ResultChart/scale.spec.tsfrontends/ui/src/shared/components/ResultChart/scale.tsfrontends/ui/src/shared/components/ResultChart/renderers/line.tsfrontends/ui/src/shared/components/ResultChart/parse.tsfrontends/ui/src/shared/components/ResultChart/normalize.tsfrontends/ui/src/shared/components/ResultChart/renderers/axis.spec.ts
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/shallow_researcher/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/prompts/writer.j2
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/agents/test_result_chart_prompt.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/test_result_chart_prompt.py
🪛 ast-grep (0.44.1)
tests/aiq_agent/agents/test_result_chart_prompt.py
[warning] 41-41: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: _CHART_BLOCK.findall(text)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
🔇 Additional comments (43)
frontends/ui/src/shared/components/ResultChart/types.ts (2)
6-9: Series cap (6) exceeds the color cycle (5) — see consolidated comment.Deferred to the consolidated comments section together with
palette.tsCYCLE.Also applies to: 39-39
1-72: LGTM!frontends/ui/src/shared/components/ResultChart/palette.ts (1)
16-16:CYCLElength (5) vs series max (6) — see consolidated comment.Deferred to the consolidated comments section together with
types.tsSeriesSchema/series.max(6).frontends/ui/src/shared/components/ResultChart/scale.ts (1)
1-111: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/axis.ts (1)
1-46: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/axis.spec.ts (1)
1-45: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/bar.ts (1)
1-40: LGTM!Also applies to: 44-55
frontends/ui/src/shared/components/ResultChart/renderers/hbar.ts (1)
1-18: LGTM!Also applies to: 47-61
frontends/ui/src/shared/components/ResultChart/renderers/index.spec.ts (1)
1-27: LGTM!frontends/ui/src/shared/components/ResultChart/parse.ts (2)
21-31: 🎯 Functional Correctness | ⚡ Quick win
toNumberdoesn't actually handle the"$1.2M"case its own docstring advertises.The regex at Line 25 strips
,,$, whitespace, and%, but never interpretsK/M/Bsuffixes.toNumber('$1.2M')cleans to"1.2M",Number("1.2M")isNaN, and the function returnsnull— silently dropping that data point instead of converting it to1200000as the docstring implies.🔧 Proposed fix to support magnitude suffixes
if (typeof value === 'string') { - const cleaned = value.replace(/[,$\s%]/g, '') - if (cleaned === '') return null - const parsed = Number(cleaned) - return Number.isFinite(parsed) ? parsed : null + const cleaned = value.replace(/[,$\s%]/g, '') + if (cleaned === '') return null + const match = cleaned.match(/^(-?\d*\.?\d+)([KMB])?$/i) + if (!match) return null + const [, num, suffix] = match + const multiplier = suffix ? ({ K: 1e3, M: 1e6, B: 1e9 } as const)[suffix.toUpperCase() as 'K' | 'M' | 'B'] : 1 + const parsed = Number(num) * multiplier + return Number.isFinite(parsed) ? parsed : null }Alternatively, if compact-suffix input is never expected in practice, just narrow the docstring instead.
38-98: LGTM!frontends/ui/src/shared/components/ResultChart/parse.spec.ts (1)
1-123: LGTM!frontends/ui/src/shared/components/ResultChart/scale.spec.ts (1)
1-137: LGTM!frontends/ui/src/shared/components/ResultChart/palette.spec.ts (1)
1-25: LGTM!frontends/ui/vitest.config.ts (1)
37-45: 📐 Maintainability & Code QualityConfirm the glob-scoped threshold actually fails CI on regression.
The
'src/shared/components/ResultChart/**'glob threshold syntax matches Vitest's documented per-file/glob threshold shape, so it should be picked up correctly. However, there's a history of glob-scoped threshold violations not surfacing a build-failing error message in some Vitest versions (unlike global thresholds), so it's worth confirming a coverage drop under this path actually fails CI on vitest 4.0.18 rather than passing silently.frontends/ui/src/shared/components/ResultChart/normalize.ts (1)
1-92: LGTM!frontends/ui/src/shared/components/ResultChart/normalize.spec.ts (1)
1-101: LGTM!frontends/ui/src/shared/components/ResultChart/index.ts (1)
1-19: LGTM!frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsx (2)
53-54: LGTM!Also applies to: 70-85, 256-256
13-16: 📐 Maintainability & Code QualityReact/Next pairing is fine
frontends/ui/package.jsonalready pinsnext@^16.2.6withreact/react-dom@^18.2.0, which is within the declared peer range, so there’s no compatibility issue here.> Likely an incorrect or invalid review comment.frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.chart.spec.tsx (1)
1-57: LGTM!frontends/ui/src/shared/components/ResultChart/geometry.ts (1)
1-49: LGTM!frontends/ui/src/shared/components/ResultChart/geometry.spec.ts (1)
1-61: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/types.ts (1)
1-32: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/bar.spec.ts (1)
1-55: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/hbar.spec.ts (1)
1-63: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/line.ts (1)
15-69: Segment-splitting and area-fill logic verified correct.Walked through the null-gap segmentation, area-baseline closing, and single-point centering against
line.spec.ts— all match. No concerns.frontends/ui/src/shared/components/ResultChart/renderers/line.spec.ts (1)
1-103: LGTM!frontends/ui/src/shared/components/ResultChart/renderers/index.ts (1)
1-21: LGTM!frontends/ui/src/shared/components/ResultChart/ChartBlock.spec.tsx (1)
1-59: LGTM!frontends/ui/src/shared/components/ResultChart/ChartKpi.spec.tsx (1)
1-42: LGTM!frontends/ui/src/shared/components/ResultChart/ResultChartCarousel.spec.tsx (1)
1-53: LGTM!frontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsx (1)
1-96: LGTM!frontends/ui/src/shared/components/ResultChart/ResultChartCarousel.tsx (1)
12-22: 🎯 Functional CorrectnessNo empty-carousel guard needed.
ChartCarouselSpecSchema.chartsisz.array(...).min(2), socount === 0cannot occur here.> Likely an incorrect or invalid review comment.frontends/ui/src/shared/components/ResultChart/ChartDataTable.spec.tsx (1)
20-34: LGTM!frontends/ui/src/shared/components/ResultChart/ChartKpi.tsx (1)
8-19: LGTM!Also applies to: 26-40
frontends/ui/src/shared/components/ResultChart/ChartToolbar.tsx (1)
27-35: LGTM!frontends/ui/src/shared/components/ResultChart/ResultChart.tsx (3)
71-105: LGTM!
44-55: 🩺 Stability & Availability
RENDERERSis exhaustive forChartType, andseriesis constrained to at least one item, so this crash risk isn’t present here.> Likely an incorrect or invalid review comment.
63-69: 🩺 Stability & AvailabilityNo change needed:
.result-chartalready setsposition: relative, so the absolute tooltip is anchored correctly.> Likely an incorrect or invalid review comment.frontends/ui/src/shared/components/ResultChart/export-csv.spec.ts (1)
22-78: LGTM!frontends/ui/src/shared/components/ResultChart/export-csv.ts (2)
15-49: LGTM!
55-64: LGTM!
- KPI warn/alarm tones use the semantic feedback theme tokens instead of hardcoded hex. - Memoize the ChartBlock parse/normalize pipeline on the raw block. - Wire aria-controls from the Show data toggle to the data-region id (useId). - Place negative single-series bar value labels below the zero baseline. - Suppress the per-series legend for delta charts (bars are colored by sign). - Restore the URL global stub in an afterEach so it survives a failed assertion. - Validate chart-carousel prompt examples against the schema. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/ui/src/shared/components/ResultChart/ResultChart.tsx`:
- Line 106: Update the ResultChart rendering around ChartDataTable so the
element referenced by ChartToolbar’s aria-controls={dataId} remains mounted when
dataOpen is false; toggle its hidden/visibility state instead of conditionally
removing it, and add or update tests to verify the referenced element exists
while collapsed.
In `@tests/aiq_agent/agents/test_result_chart_prompt.py`:
- Around line 55-67: Ensure test_prompt_carousel_examples_match_the_schema
asserts that _CAROUSEL_BLOCK.findall(text) returns at least one match before
iterating. Preserve the existing carousel schema checks and use the same
non-empty assertion pattern as the chart-example test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a9e3ac27-0469-4ee0-9b94-617f269479b1
📒 Files selected for processing (9)
frontends/ui/src/app/globals.cssfrontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/ResultChart.tsxfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.tstests/aiq_agent/agents/test_result_chart_prompt.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/renderers/bar.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/renderers/bar.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/shared/components/ResultChart/ResultChart.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/shared/components/ResultChart/ChartBlock.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartDataTable.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/renderers/bar.tsfrontends/ui/src/shared/components/ResultChart/renderers/bar.spec.tsfrontends/ui/src/app/globals.cssfrontends/ui/src/shared/components/ResultChart/ResultChart.tsx
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/agents/test_result_chart_prompt.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/test_result_chart_prompt.py
🪛 ast-grep (0.44.1)
tests/aiq_agent/agents/test_result_chart_prompt.py
[warning] 57-57: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: _CAROUSEL_BLOCK.findall(text)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
🔇 Additional comments (11)
frontends/ui/src/shared/components/ResultChart/renderers/bar.ts (2)
41-44: 🎯 Functional CorrectnessNegative-label placement fix verified correct.
The
value >= 0branch now correctly anchors negative labels below the bar's bottom tip (Math.max(y, baselineY) + 12) and positive labels above the top (Math.min(y, baselineY) - 5), resolving the previously flagged bug where negative labels rendered just above the baseline. Confirmed againstbar.spec.tslines 34-51.
1-40: LGTM!Also applies to: 45-56
frontends/ui/src/app/globals.css (1)
391-396: 📐 Maintainability & Code QualitySemantic token fix applied as requested.
tone-warn/tone-alarmnow reference--text-color-feedback-warning/--text-color-feedback-dangerinstead of hardcoded hex, matching the earlier review request.frontends/ui/src/shared/components/ResultChart/renderers/bar.spec.ts (2)
34-51: LGTM!
1-33: LGTM!Also applies to: 53-73
tests/aiq_agent/agents/test_result_chart_prompt.py (1)
1-53: LGTM!Also applies to: 69-90
frontends/ui/src/shared/components/ResultChart/ChartBlock.tsx (1)
6-6: LGTM!Also applies to: 19-36
frontends/ui/src/shared/components/ResultChart/ChartDataTable.tsx (1)
8-36: LGTM!frontends/ui/src/shared/components/ResultChart/ResultChart.tsx (1)
14-57: LGTM!Also applies to: 62-105, 107-195
frontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsx (1)
18-50: LGTM!frontends/ui/src/shared/components/ResultChart/ChartToolbar.tsx (1)
12-39: LGTM!
Replace the flat 15% area fill with a per-series vertical linearGradient (28% opacity at the line to 0% at the baseline) so the area reads as a subtle wash instead of a solid block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
Drop the raised surface fill so the chart card takes on the page background (reads as black in the dark theme) instead of a tinted panel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
KyleZheng1284
added
enhancement
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/aiq_agent/agents/test_result_chart_prompt.py`:
- Around line 56-63: Update test_prompt_carousel_examples_match_the_schema to
iterate over each carousel["charts"] entry and assert its required
chart["title"] is present, while preserving the existing outer-title and
minimum-chart validations. Add focused coverage and update the relevant
prompt-contract documentation as required by the repository guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 5de979d7-027b-42c8-8069-3d8cb93f108e
📒 Files selected for processing (7)
frontends/ui/src/app/globals.cssfrontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxsrc/aiq_agent/agents/deep_researcher/prompts/writer.j2src/aiq_agent/agents/shallow_researcher/prompts/researcher.j2tests/aiq_agent/agents/test_result_chart_prompt.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsxfrontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsxfrontends/ui/src/shared/components/ResultChart/ChartToolbar.tsxfrontends/ui/src/app/globals.css
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/deep_researcher/prompts/writer.j2src/aiq_agent/agents/shallow_researcher/prompts/researcher.j2
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/agents/test_result_chart_prompt.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/test_result_chart_prompt.py
🔇 Additional comments (7)
frontends/ui/src/shared/components/ResultChart/ChartToolbar.tsx (1)
12-17: LGTM!Also applies to: 18-39
frontends/ui/src/shared/components/ResultChart/ResultChart.spec.tsx (1)
96-104: LGTM!frontends/ui/src/shared/components/ResultChart/ChartToolbar.spec.tsx (1)
17-53: LGTM!frontends/ui/src/app/globals.css (1)
315-357: LGTM!Also applies to: 358-401, 402-478, 479-509, 510-558
src/aiq_agent/agents/deep_researcher/prompts/writer.j2 (1)
105-134: LGTM!src/aiq_agent/agents/shallow_researcher/prompts/researcher.j2 (1)
36-65: LGTM!tests/aiq_agent/agents/test_result_chart_prompt.py (1)
23-44: LGTM!Also applies to: 71-92
2 participants
What this does
Lets the agent present numeric answers (rankings, trends, distributions, gains/losses, single metrics) as inline charts instead of walls of text or tables. The agent writes a small JSON chart spec in its reply; the frontend renders it as a styled inline SVG. No code execution and no new runtime dependencies.
How it works
chart(orchart-carousel) block of JSON in its answer.MarkdownRendererdetects that fence and hands it to the newResultChartmodule.Chart types
bar,hbar,line,area,grouped-bar,delta(diverging gains/losses), KPI-only cards, and a pageablechart-carousel. One answer can contain several charts.Correctness and safety
Where the numbers come from
The renderer is tool agnostic: it charts whatever numbers a tool returns, so it works today with web search and will work with any future data source. A generic chart contract was added to the shallow researcher and deep-research writer prompts (no dataset-specific coupling) and is guarded by a test.
Testing
ResultChartmodule, enforced by a scoped Vitest threshold (global coverage is unaffected).🤖 Generated with Claude Code
Summary by CodeRabbit