Repository navigation
Stop requiring an id key in chart files and name the y-axis label form - #110
Merged
Merged
Conversation
The chart format reference listed `id` as required, but the chart previewer never reads it, so quick charts were often saved without it. The reference now marks `id` as optional and says the y-axis label is a `yAxisLabel` string, not a `yAxis` object. The quick-chart step in the skill names the required keys directly. The behaviour-suite grader no longer requires `id`. Plugin version is now 0.35.0. Fixes #109 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chart files no longer need an
idkey. The chart format reference now marksidas optional and says that the y-axis label is ayAxisLabelstring. The quick-chart step in the skill now lists the required keys itself. A quick chart that leaves outidnow meets the documented format. The plugin version is now 0.35.0.Fixes #109.
The reference required a key that nothing reads
references/output/chart-spec.mdlistedidas required.scripts/term_chart.py, the script that prints the chart preview, never readsid. It also never reads a y-axis key. No other part of the plugin uses a saved chart file'sid. Nothing failed when the key was missing, so quick-chart runs often left it out. In the behaviour suite, two of three runs of thequick-chart-of-a-trendcase saved a file withoutidand failed the format check. The runs also wrote the y-axis label in two ways: asyAxisLabelin one run and as ayAxisobject in two.The fix makes
idoptional and names one y-axis formtitle,type,xField,seriesanddata. It marksidas optional and says that no tool reads it. It says to write the y-axis label as ayAxisLabelstring, not as ayAxisobject. The field list has matching entries.skills/factiq/SKILL.mdnow names the required keys andyAxisLabel. The model sees them without having to open the reference.spec-is-a-valid-chartspecno longer requiresid..claude-plugin/plugin.jsonand.codex-plugin/plugin.json.Before and after
The "Minimal valid spec" section of
references/output/chart-spec.md:Before:
After:
Quick-chart step in
skills/factiq/SKILL.md:Before:
After:
Grader
evals/quick-chart-of-a-trend/graders/spec-is-a-valid-chartspec.md:Before:
It is one JSON object with the keysid,title,type,xField,seriesanddata.After:
It is one JSON object with the keystitle,type,xField,seriesanddata.Tests
tests/test_sql_docs.pyhas three new tests inChartSpecRequiredKeysTests:test_id_is_not_a_required_key: checks that neither the reference's required list nor the skill's list of required keys includesid.test_y_axis_label_form_is_stated: checks that the reference namesyAxisLabeland rejects ayAxisobject.test_renderer_accepts_a_spec_without_id: renders a spec that has noidwithterm_chart.pyand checks that the render succeeds.python -m pytest tests/ -q: 72 passed. The behaviour suite was not run again for this change.🤖 Generated with Claude Code