Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions .github/workflows/impl-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,105 @@ jobs:
fi
echo "::notice::Found both theme renders — proceeding to review"

# Canvas dimension gate — runs before the AI review.
# The saved PNG must be exactly 3200×1800 (landscape) or 2400×2400 (square)
# within ±16 px (≤0.5 %). On drift, write /tmp/anyplot-canvas-gate.txt with
# a synthetic weakness (named actual+target+signed delta + likely cause).
# ai-quality-review.md reads that file and treats canvas_gate=fail as a
# MUST-flag weakness that forces VQ-05 to 0, which drags the quality_score
# below the repair threshold — re-routing the PR into impl-repair through
# the existing 5-review/4-repair cascade. Never raw `exit 1` here:
# impl-repair.yml is dispatched by ai-rejected label only, so a hard step
# failure would leave a dead PR with no retry path.
- name: Canvas dimension gate
env:
LIBRARY: ${{ steps.pr.outputs.library }}
ATTEMPT: ${{ steps.attempts.outputs.display }}
run: |
# Clean any stale file from a previous run on this runner.
rm -f /tmp/anyplot-canvas-gate.txt

pip install --quiet pillow

python3 <<'PY'
import os
import sys
from pathlib import Path

from PIL import Image

LIBRARY = os.environ["LIBRARY"]
ATTEMPT = os.environ["ATTEMPT"]
TOLERANCE = 16 # ≤0.5 % on the 3200-axis
TARGETS = [(3200, 1800), (2400, 2400)]

path = Path("plot_images/plot-light.png")
with Image.open(path) as img:
w, h = img.size

# Pick the closer target so the repair feedback names the right one.
tw, th = min(TARGETS, key=lambda t: abs(w - t[0]) + abs(h - t[1]))
dw, dh = w - tw, h - th
ok = abs(dw) <= TOLERANCE and abs(dh) <= TOLERANCE

status = "pass" if ok else "fail"
print(
f"::notice::canvas_gate library={LIBRARY} status={status} "
f"actual={w}x{h} target={tw}x{th} delta={dw:+d}x{dh:+d} attempt={ATTEMPT}"
)

if ok:
sys.exit(0)

# Build the synthetic weakness — include actual, target, signed delta,
# direction, possible aspect mismatch, and library-specific likely cause.
# This is what impl-repair sees in metadata.review.weaknesses.
direction_parts = []
if dw < 0:
direction_parts.append(f"width is {-dw} px short")
elif dw > 0:
direction_parts.append(f"width is {dw} px over")
if dh < 0:
direction_parts.append(f"height is {-dh} px short")
elif dh > 0:
direction_parts.append(f"height is {dh} px over")
direction = "; ".join(direction_parts) or "axes on target individually but combined off"

aspect_note = ""
# Aspect-mismatch hint: actual is landscape-ish but closest target is square (or vice versa)
actual_aspect = w / h if h else 1
if (tw, th) == (2400, 2400) and actual_aspect > 1.2:
aspect_note = " — the implementation rendered a landscape canvas but the closest target is square; reconsider orientation."
elif (tw, th) == (3200, 1800) and actual_aspect < 0.85:
aspect_note = " — the implementation rendered a portrait/square canvas but the closest target is landscape; reconsider orientation."

# Library-specific likely cause (concrete repair hints, not generic).
causes = {
"matplotlib": "Most likely cause: `bbox_inches='tight'` on `savefig` — remove it (default `bbox_inches=None`).",
"seaborn": "Most likely cause: `bbox_inches='tight'` on `savefig` — remove it (default `bbox_inches=None`).",
"altair": "Most likely cause: vl-convert padding title/legend outside `width`/`height`. Add `configure_view(continuousWidth=…, continuousHeight=…)` + zero `padding` and normalize the saved PNG with the PIL crop/pad snippet in `prompts/library/altair.md`.",
"plotly": "Most likely cause: `autosize=True` or implicit margin growth. Set `autosize=False` and pin `margin=dict(...)` explicitly.",
"bokeh": "Most likely cause: bokeh toolbar adding ~30-50 px above the figure. Set `toolbar_location=None`.",
"highcharts": "Most likely cause: Chrome chrome eating viewport space. Use `Emulation.setDeviceMetricsOverride` via CDP (see `prompts/library/highcharts.md` Save section) — `--window-size` alone is not authoritative in headless mode.",
"pygal": "Check that `pygal.<Chart>(width=…, height=…)` and the SVG→PNG conversion don't override dims.",
"plotnine": "Check `ggsave(width=…, height=…, units='in', dpi=400)` and `theme(figure_size=…)`; do not pass `bbox_inches='tight'`.",
"letsplot": "Check `ggsize(W, H)` and `ggsave(..., scale=4)` pair — only the two canonical pairs land on target.",
"ggplot2": "Check `ggsave(width=…, height=…, units='in', dpi=400)` with `ragg::agg_png`.",
}
cause = causes.get(LIBRARY, f"Review `prompts/library/{LIBRARY}.md` 'Canvas — hard rule' section.")

weakness = (
f"Canvas dimensions drifted from required target. "
f"Actual: {w}×{h}. Closest valid target: {tw}×{th} (±{TOLERANCE} px tolerance). "
f"Signed delta: {dw:+d} × {dh:+d} — {direction}.{aspect_note} "
f"{cause} "
f"Re-render at exactly {tw}×{th}; the post-render gate enforces this."
)

Path("/tmp/anyplot-canvas-gate.txt").write_text(weakness, encoding="utf-8")
print(f"::warning::Canvas gate FAILED — wrote synthetic weakness to /tmp/anyplot-canvas-gate.txt: {weakness}")
PY

- name: React with eyes emoji
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
1 change: 1 addition & 0 deletions app/eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export default [
requestAnimationFrame: 'readonly',
cancelAnimationFrame: 'readonly',
performance: 'readonly',
crypto: 'readonly',
// DOM types
HTMLElement: 'readonly',
HTMLDivElement: 'readonly',
Expand Down
2 changes: 1 addition & 1 deletion app/src/components/SpecTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export function SpecTabs({
const handleTagClick = useCallback(
(paramName: string, value: string) => {
onTrackEvent?.('tag_click', { param: paramName, value, source: 'spec_detail' });
navigate(`/?${paramName}=${encodeURIComponent(value)}`);
navigate(`/plots?${paramName}=${encodeURIComponent(value)}`);
},
[navigate, onTrackEvent]
);
Expand Down
47 changes: 44 additions & 3 deletions app/src/pages/StatsPage.test.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, screen, waitFor } from '../test-utils';
import { render, screen, userEvent, waitFor } from '../test-utils';
import { StatsPage } from './StatsPage';

vi.mock('react-helmet-async', () => ({
Helmet: ({ children }: { children: React.ReactNode }) => <>{children}</>,
}));

const mockTrackEvent = vi.fn();

vi.mock('../hooks', () => ({
useAnalytics: () => ({
trackPageview: vi.fn(),
trackEvent: vi.fn(),
trackEvent: mockTrackEvent,
}),
}));

Expand Down Expand Up @@ -55,7 +57,9 @@ const mockDashboard = {
],
tag_distribution: {
plot_type: { scatter: 42, line: 30 },
data_type: { numeric: 80 },
// "time series" has a space so the tag-link test below can assert
// encodeURIComponent is actually exercised (href -> data=time%20series).
data_type: { numeric: 80, 'time series': 12 },
},
score_distribution: { '50-60': 5, '60-70': 10, '70-80': 20, '80-90': 30, '90-100': 15 },
timeline: [
Expand Down Expand Up @@ -118,6 +122,7 @@ function mockFetchError() {
describe('StatsPage', () => {
beforeEach(() => {
vi.restoreAllMocks();
mockTrackEvent.mockClear();
});

// vi.restoreAllMocks() doesn't undo vi.stubGlobal — without this hook the
Expand Down Expand Up @@ -247,4 +252,40 @@ describe('StatsPage', () => {
expect(screen.getByText(/visitor data unavailable/)).toBeInTheDocument();
});
});

it('routes tag links to /plots and fires tag_click on click', async () => {
// Regression: pre-fix the href pointed at /?plot=scatter, which landed
// on LandingPage and silently dropped the filter intent. Filter routing
// lives on /plots after the page split, so the link must target that.
mockFetchSuccess();
const user = userEvent.setup();

render(<StatsPage />);

await waitFor(() => {
expect(screen.getByText('scatter')).toBeInTheDocument();
});

const scatterLink = screen.getByText('scatter').closest('a');
expect(scatterLink).toHaveAttribute('href', '/plots?plot=scatter');

// Tag with a space exercises encodeURIComponent — proves the encoding
// step isn't a no-op.
const timeSeriesLink = screen.getByText('time series').closest('a');
expect(timeSeriesLink).toHaveAttribute('href', '/plots?data=time%20series');

await user.click(scatterLink!);
expect(mockTrackEvent).toHaveBeenCalledWith('tag_click', {
param: 'plot',
value: 'scatter',
source: 'stats',
});

await user.click(timeSeriesLink!);
expect(mockTrackEvent).toHaveBeenCalledWith('tag_click', {
param: 'data',
value: 'time series',
source: 'stats',
});
});
});
3 changes: 2 additions & 1 deletion app/src/pages/StatsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,8 @@ export function StatsPage() {
<Link
key={tag}
component={RouterLink}
to={param ? `/?${param}=${tag}` : '/'}
to={param ? `/plots?${param}=${encodeURIComponent(tag)}` : '/plots'}
onClick={() => { if (param) trackEvent('tag_click', { param, value: tag, source: 'stats' }); }}
sx={{
fontFamily: typography.fontFamily, fontSize: size, fontWeight: weight, textDecoration: 'none',
px: 0.75, py: 0.25, borderRadius: 0.5,
Expand Down
Loading
Loading