fix(prompts): address Copilot follow-up review on canvas-halve PRs#7394
Merged
Merged
Conversation
Eight inline Copilot comments across the canvas-halve PRs landed after the PRs were auto-merged. All substantive, all applied: - prompts/plot-generator.md (from #7391 review): Output Files snippet still showed `dpi=300` for matplotlib + `width=1600 scale=3` for plotly — the exact stale values the PR was supposed to remove. Updated to the new 3200×1800 convention. - prompts/library/plotnine.md (from #7389 review): "Sizing" snippet uses `element_text(...)` but Import block didn't include it → generated code would `NameError`. Added `element_text`, `element_rect`, `element_line` to the import list. - prompts/library/highcharts.md (from #7389 review): "X-axis labels cut off" pitfall still recommended `'14px'`, contradicting the new 12px default. Reframed as 12px default with 14px as the specific-case bump. - prompts/default-style-guide.md (from #7389 review): Native-pixel column in the Visual Sizing Defaults table mixed `'pt'` (bokeh), px (highcharts), and unitless (pygal) under one cell. Split per-library so readers don't copy the wrong unit; added explanatory note. - prompts/library/matplotlib.md + seaborn.md (from #7389 review): `ax.legend(...)` was unconditional; on single-series plots that prints "No artists with labels found to put in legend". Wrapped in `if len(ax.get_legend_handles_labels()[0]) > 1:`. - prompts/library/bokeh.md (from #7389 review): Sizing snippet had no legend fontsize example. Added commented-out `p.legend.label_text_font_size = '12pt'` line (commented because legends only apply when add_layout/legend_label= is used). - prompts/quality-evaluator.md (from #7393 review): Example JSON arithmetic drifted when I bumped `vq01_text_legibility` from 5 → 6 in #7393 but forgot to update `visual_quality.total` (was 23, sub-scores now sum to 24) and the top-level `score` (was 76 → now 77). Fixed both. These were all reasonable suggestions that arrived after auto-merge fired. Future work: poll Copilot review explicitly before --auto-merging cosmetic / prompt PRs so the suggestions can be addressed in the same PR. Added as a note to project_impl_generate_process_gaps.md follow-up list.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the prompt set to incorporate late follow-up review feedback from the recent canvas-halving prompt PRs, aiming to keep generation guidance self-consistent (especially around output sizing, legend handling, and scoring examples).
Changes:
- Corrected output sizing examples (matplotlib/plotly) and clarified sizing units across library families.
- Fixed/updated library-specific prompt snippets (plotnine imports, legend guidance, highcharts clipping advice, bokeh legend note).
- Adjusted the quality-evaluator example JSON to reflect updated scoring, but it still needs a consistency fix (see comments).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| prompts/quality-evaluator.md | Updates example JSON score/visual_quality totals (currently inconsistent with summed totals). |
| prompts/plot-generator.md | Updates output sizing snippets for matplotlib + plotly in “Output Files”. |
| prompts/library/seaborn.md | Adds conditional legend call to avoid unnecessary legend/warnings. |
| prompts/library/plotnine.md | Adds missing imports for theme element helpers used in snippets. |
| prompts/library/matplotlib.md | Adds conditional legend call to avoid unnecessary legend/warnings. |
| prompts/library/highcharts.md | Aligns “labels cut off” guidance with the 12px default and conditional bump to 14px. |
| prompts/library/bokeh.md | Adds a commented example for legend font sizing. |
| prompts/default-style-guide.md | Splits/clarifies native-pixel sizing units per library (bokeh/highcharts/pygal). |
Comment on lines
92
to
+99
| ```json | ||
| { | ||
| "score": 76, | ||
| "score": 77, | ||
| "tier": "Good", | ||
| "pass": false, | ||
|
|
||
| "visual_quality": { | ||
| "total": 23, | ||
| "total": 24, |
Comment on lines
+450
to
452
| # matplotlib/seaborn/plotnine (static, PNG only) — figsize=(8, 4.5) dpi=400 → 3200×1800 | ||
| plt.savefig(f'plot-{THEME}.png', dpi=400, bbox_inches='tight', facecolor=PAGE_BG) | ||
|
|
| ax.set_xlabel(x_label, fontsize=12) | ||
| ax.set_ylabel(y_label, fontsize=12) | ||
| ax.tick_params(axis='both', labelsize=10) | ||
| # Legend at 10pt — skip for single-series plots (no labeled artists → warning) |
Comment on lines
+57
to
+58
| # Legend at 10pt — skip ax.legend() for single-series plots (avoids the | ||
| # "No artists with labels found to put in legend" warning) |
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.
Summary
Late-arriving Copilot review comments on #7389 / #7391 / #7393 — all substantive, all applied here.
Fixes
High-impact (would have caused generation failures or wrong sizing):
Consistency:
Process note
All these comments arrived AFTER the parent PRs auto-merged via `--auto`. Per CLAUDE.md PR Follow-Through, I should poll Copilot review explicitly before `--auto`-merging prompt/cosmetic PRs in the future. Adding to the process-gaps memo.
🤖 Generated with Claude Code