Skip to content

Add phase 2 of the insight chart color registry (slug threading + resolver) - #773

Open
yellowcap wants to merge 4 commits into
mainfrom
feat/insight-chart-colors-phase2
Open

Add phase 2 of the insight chart color registry (slug threading + resolver)#773
yellowcap wants to merge 4 commits into
mainfrom
feat/insight-chart-colors-phase2

Conversation

@yellowcap

Copy link
Copy Markdown
Collaborator

Summary

Phase 2 of the insight chart color plan (see docs/insight-chart-colors-plan.md, follows phase 1 in #771). Today the code-executor LLM writes already-translated category labels straight into chart_data, with no stable key surviving translation. This threads a stable category slug through the pipeline and resolves colors deterministically (never LLM-generated) against the phase-1 registry, so chart colors stay consistent across languages and match the map legend.

Changes

  • EXECUTOR_WORKFLOW (src/agent/subagents/analyst/prompts.py): instructs the code executor to emit a {column}__slug sibling column next to any categorical color column, using canonical slugs from the dataset's registry when available.
  • Analyst._resolve_charts / build_analysis_prompt (src/agent/subagents/analyst/tool.py): passes category_slug_hints (from get_dataset_palette(dataset_id)) into the prompt and threads dataset_id through.
  • New resolve_chart_colors() (src/agent/subagents/analyst/charts/color_resolver.py): deterministic, hash-based fallback color for slugs with no registry entry; attaches color_map/series_color/divergent_colors onto InsightChart.
  • update_insight_display (src/agent/tools/update_insight_display.py): re-runs the resolver on revision, using the dataset_id persisted on the original chart (revisions can re-map color_field to a different column).
  • New DB columns on insight_charts via migration d4f7b1e9a3c2_add_chart_color_registry_fields: dataset_id, color_map, series_color, divergent_colors.
  • InsightChart model/schema/API surface carries the new fields through both acquisition paths (chat stream + analysis LRO).

Compatibility

Additive: new nullable/defaulted DB columns, new optional model fields, new prompt section. Existing charts without a dataset_id resolve to empty/None colors (falls back to current frontend behavior) until the companion project-zeno-next PR consumes colorMap/seriesColor/divergentColors.

Test plan

  • uv run pytest tests/agent/test_update_insight_display.py tests/tools/test_generate_insights_tiered.py tests/unit/agent/tools/test_color_resolver.py tests/unit/agent/tools/test_insight_chart_model.py tests/api/test_insights.py
  • ruff / ruff-format / mypy clean on touched files

…olver)

Threads dataset_id and a stable category slug column through the analyst
code-executor prompt, resolves colors deterministically against the phase 1
registry onto InsightChart (color_map/series_color/divergent_colors), and
persists/serves the result end to end so charts stay color-consistent across
languages instead of relying on translated-string matches.
Avoids colliding with main's current version so the bump-calver
pre-commit hook (and CI lint check) passes.
The merge commit dropped uv.lock entirely, breaking CI (`uv sync --frozen`
couldn't find a lockfile). Regenerated to match pyproject.toml 2026.7.24.3.
@srmsoumya

Copy link
Copy Markdown
Collaborator

Conceptually this looks good to me @yellowcap - colors are resolved deterministically and persisted with dataset_id for re-resolution on revision.

One thing to check: stack_field / group_field don't drive color resolution. _categorical_column only looks at color_field (and x_axis for pies), and the prompt's slug instruction also only mentions color_field. So a stacked-bar chart with x=year, stack=driver and no color_field might not get a color_map.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants