Add chart datapoint links via metadata tuples by lovasoa · Pull Request #1255 · sqlpage/SQLPage

lovasoa · 2026-03-19T09:44:58Z

Motivation

Standardize per-point optional metadata into a compact object and add row-level click links on chart datapoints, removing the legacy numeric 4th-slot format.
Preserve existing payload size for points without metadata while enabling per-point z and link metadata for interactive navigation and documentation examples.

Description

Update template serialization in sqlpage/templates/chart.handlebars to emit either [series, x, y] or [series, x, y, {"z":..., "l":...}] and to avoid any numeric 4th-slot output.
Update runtime in sqlpage/apexcharts.js to parse 3-item tuples and 4th-slot object metadata, extract z and l, preserve existing time/range conversions, attach l to point objects, and build pieLinks for pie slices.
Preserve metadata in multi-series category alignment by changing align_categories to reuse full point objects rather than discarding extra fields.
Add click handling with dataPointSelection and click event hooks, helper functions resolvePointLink and navigateIfLink, and a short-lived guard to avoid double-triggered navigation.
Document the new per-row link parameter in examples/official-site/sqlpage/migrations/01_documentation.sql and add examples for linked bar, linked pie, and combined z+link points.
Add end-to-end tests in tests/end-to-end/official-site.spec.ts covering linked bar navigation, linked pie navigation, and no-op behavior for non-linked datapoints.

Testing

Ran frontend formatting with npm run format which completed successfully.
Ran the full Rust test suite with cargo test -q and all tests passed (136 passed, plus other test groups shown as ok).
Attempted to run targeted Playwright e2e tests but the Playwright runner failed in this environment due to a test-runner/package mismatch, so the new Playwright tests were added but could not be executed here.
Attempted a browser screenshot of the updated chart docs page but the browser container timed out, so visual verification was not captured in this environment.

Codex Task

chatgpt-codex-connector · 2026-03-19T09:45:04Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

lovasoa

@codex simplify code examples and tests, see comments below

lovasoa · 2026-03-19T09:46:28Z

sqlpage/apexcharts.js

+        } else if (!warnedLegacyPointMetadata) {
+          warnedLegacyPointMetadata = true;
+          console.warn(
+            "Chart point metadata must be an object in the 4th tuple slot. Legacy formats are ignored.",
+          );
+        }


no warning just throw an error

@codex no warning, just throw

Summary

Updated chart point metadata parsing so a 4th tuple slot must be an object; otherwise the chart code now throws a TypeError immediately instead of logging a one-time warning and continuing. sqlpage/apexcharts.jsL121-L139

Removed the legacy warning-state bookkeeping because invalid metadata now fails fast on the offending datapoint. sqlpage/apexcharts.jsL121-L135

Committed the follow-up change on the current branch as bca97e7 and created a PR message titled Follow-up: throw on invalid chart metadata tuples.

Testing

✅ npm run format

✅ npm test

View task →

lovasoa · 2026-03-19T09:48:42Z

examples/official-site/sqlpage/migrations/01_documentation.sql

+    ('chart', 'A bar chart with clickable datapoints. Each row can set a `link` URL; clicking a datapoint opens that URL.', json('[
+        {"component":"chart", "type": "bar", "title": "Linked Sales", "labels": true},
+        {"label": "North", "value": 35, "link": "/documentation.sql?component=table"},
+        {"label": "South", "value": 22},
+        {"label": "West", "value": 41, "link": "/documentation.sql?component=map"}
+    ]')),
+    ('chart', 'A pie chart with per-slice links.', json('[
+        {"component":"chart", "title": "Linked Answers", "type": "pie", "labels": true},
+        {"label": "Yes", "value": 65, "link": "/documentation.sql?component=form"},
+        {"label": "No", "value": 35, "link": "/documentation.sql?component=table"}
+    ]')),
+    ('chart', 'A bubble chart demonstrating `z` and `link` metadata together on points.',
+    json('[
+        {"component":"chart", "title": "Bubbles with links", "type": "bubble", "ztitle": "Population", "marker": 8},
+        {"series": "Europe", "x": 2.1, "y": 18.5, "z": 742, "link": "/documentation.sql?component=chart"},
+        {"series": "Asia", "x": 5.2, "y": 24.1, "z": 4700, "link": "/documentation.sql?component=map"}
+    ]')),


Instead of adding new examples, add links to the existing examples where it makes the most sense. Update the tests accordingly and run them

@codex Instead of adding new examples, add links to the existing examples where it makes the most sense. Update the tests accordingly and run them

Summary

Folded chart link usage into the existing Answers pie chart and Quarterly Results bar chart documentation examples, instead of keeping separate linked-example entries. examples/official-site/sqlpage/migrations/01_documentation.sqlL690-L696

Updated chart point parsing to throw a TypeError immediately when the 4th tuple slot is not an object, matching the review request to error instead of warn. sqlpage/apexcharts.jsL121-L135

Retargeted the end-to-end coverage to those existing chart examples and disambiguated the duplicate Quarterly Results headings with exact role matching so the updated tests pass. tests/end-to-end/official-site.spec.tsL27-L60

Committed the follow-up changes on the current branch as 2adc7e1 and recorded a follow-up PR title/body with the requested summary.

Testing

✅ npx biome format --write sqlpage/apexcharts.js tests/end-to-end/official-site.spec.ts

✅ npx biome check sqlpage/apexcharts.js tests/end-to-end/official-site.spec.ts

✅ cargo run (from examples/official-site)

✅ npx playwright install chromium (from tests/end-to-end)

✅ npx playwright install-deps chromium (from tests/end-to-end)

✅ npm run test -- official-site.spec.ts -g 'chart point links|chart links - no-link datapoint' (from tests/end-to-end)

⚠️ Screenshot not captured because the browser screenshot tool was not available in this environment.

View task →

chatgpt-codex-connector · 2026-03-19T09:49:24Z

On it

chatgpt-codex-connector · 2026-03-19T10:05:31Z

Summary

Changed chart metadata parsing to throw a TypeError immediately when the 4th tuple slot is not an object, matching the review request to error instead of warning. sqlpage/apexcharts.jsL121-L135
Simplified the chart documentation examples by folding link usage into the existing Answers pie chart and Quarterly Results bar chart, instead of keeping separate linked-example entries. examples/official-site/sqlpage/migrations/01_documentation.sqlL690-L696
Updated the Playwright coverage to target those existing chart examples and to disambiguate duplicate cards on the documentation page while keeping the linked-bar, linked-pie, and no-link checks. tests/end-to-end/official-site.spec.tsL27-L67

Testing

✅ npx biome format --write sqlpage/apexcharts.js tests/end-to-end/official-site.spec.ts
✅ npx biome check sqlpage/apexcharts.js tests/end-to-end/official-site.spec.ts
⚠️ npm test (fails due to a pre-existing unrelated Biome formatting issue in examples/telemetry/grafana/sqlpage-home.json, which I did not modify)
✅ cargo run (from examples/official-site)
✅ npm run test -- official-site.spec.ts -g 'chart point links|chart links - no-link datapoint'

Committed on the current branch as 8eccd09, and I created the follow-up PR message summarizing these review-driven changes.

View task →

Add chart datapoint links via metadata tuples

5670b69

lovasoa added the codex label Mar 19, 2026 — with ChatGPT Codex Connector

lovasoa commented Mar 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add chart datapoint links via metadata tuples#1255

Add chart datapoint links via metadata tuples#1255
lovasoa wants to merge 1 commit intomainfrom
codex/add-clickable-links-to-chart-datapoints

lovasoa commented Mar 19, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 19, 2026

Uh oh!

lovasoa left a comment

Uh oh!

lovasoa Mar 19, 2026

Uh oh!

lovasoa Mar 19, 2026

Uh oh!

chatgpt-codex-connector bot Mar 19, 2026

Uh oh!

lovasoa Mar 19, 2026

Uh oh!

lovasoa Mar 19, 2026

Uh oh!

chatgpt-codex-connector bot Mar 19, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 19, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

lovasoa commented Mar 19, 2026

Motivation

Description

Testing

Uh oh!

chatgpt-codex-connector bot commented Mar 19, 2026

Uh oh!

lovasoa left a comment

Choose a reason for hiding this comment

Uh oh!

lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 19, 2026

Choose a reason for hiding this comment

Summary

Uh oh!

chatgpt-codex-connector bot commented Mar 19, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 19, 2026

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant