Skip to content

Sweep grid: review follow-ups from #17 - #18

Merged
cirsteve merged 1 commit into
mainfrom
fix/sweep-grid-review-followups
Sep 10, 2026
Merged

cirsteve merged 1 commit into
mainfrom
fix/sweep-grid-review-followups

Conversation

@cirsteve

@cirsteve cirsteve commented Sep 10, 2026 •

Copy link
Copy Markdown
Member

The four review comments on #17 that arrived after it merged.

  • Axis keys are slugs. projects, prompts, backends keys become sweep file name components; the schema now constrains them with propertyNames, and validate_grid mirrors the check at runtime. Test for a key with a path separator.
  • Chart page doctype. charts.template.html gets <!DOCTYPE html> and <html lang="en">; charts.html regenerated. Tables unchanged.
  • Baseline validation. format_report drops partial runs before selecting rows and takes the baseline only when every selected row agrees; an empty pool raises instead of StopIteration.
  • Full sweep names. pool_relevance no longer truncates the sweep column.

Local: ruff and mypy repo-wide clean, 2435 tests passed, evidence check up to date.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RzSnFgZ2rHydpunbz2b1V9

Summary by CodeRabbit

  • Bug Fixes

    • Improved sweep reporting by excluding partial runs and validating baseline data before generating tables and charts.
    • Sweep names now display in full instead of being truncated.
    • Chart pages now use complete, standards-compliant HTML structure.
  • Validation

    • Project, prompt, and backend keys now require a consistent lowercase slug format.
    • Invalid keys and path separators are rejected before processing.

Follow-ups from the review of #17 that landed after merge:
- projects/prompts/backends keys must be slugs (schema propertyNames plus
  validate_grid at runtime): they become sweep file name components, so a
  key with a path separator would write outside the output directory.
- charts.template.html gets a doctype and an html[lang] root so the page no
  longer renders in quirks mode; charts.html regenerated.
- format_report: partial runs (fewer cases than the project's largest run)
  are excluded before selection, and the baseline is taken only when every
  selected row agrees on it; an empty pool is an error instead of a
  StopIteration.
- pool_relevance no longer truncates sweep names to 48 characters.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RzSnFgZ2rHydpunbz2b1V9
@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Essentials

Run ID: e8215876-b7a7-48d9-b7bd-c2d9367553ff

📥 Commits

Reviewing files that changed from the base of the PR and between 5e3ba88 and 50b6a2a.

📒 Files selected for processing (7)
  • contracts/replay-sweep-grid.v1.schema.json
  • evidence/relevance-sweeps-2026-09/charts.html
  • evidence/relevance-sweeps-2026-09/scripts/charts.template.html
  • evidence/relevance-sweeps-2026-09/scripts/format_report.py
  • evidence/relevance-sweeps-2026-09/scripts/pool_relevance.py
  • src/scout/replay/grid.py
  • tests/test_replay_grid.py

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

The changes add replay grid slug validation and expose validate_grid. Relevance reports now exclude partial runs, validate baseline metrics, preserve full sweep names, and generate complete HTML documents.

Changes

Replay grid validation

Layer / File(s) Summary
Grid key contract
contracts/replay-sweep-grid.v1.schema.json
Project, prompt, and backend keys now require lowercase slug syntax. The schema documents their use in sweep filenames.
Runtime grid validation
src/scout/replay/grid.py, tests/test_replay_grid.py
load_grid applies validate_grid, which rejects invalid axis keys. Tests cover path separators and invalid prompt keys. The validator is publicly exported.

Relevance report output

Layer / File(s) Summary
Run and baseline selection
evidence/relevance-sweeps-2026-09/scripts/format_report.py
Partial runs are excluded. Baselines must be non-empty and internally consistent before table and chart generation.
Sweep name formatting
evidence/relevance-sweeps-2026-09/scripts/pool_relevance.py
Formatted output now preserves the complete sweep name.
Chart document structure
evidence/relevance-sweeps-2026-09/charts.html, evidence/relevance-sweeps-2026-09/scripts/charts.template.html
The chart page and template now include standard HTML metadata and complete document tags.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 50b6a

The change validates replay-grid keys and improves report filtering, baseline handling, naming, and HTML structure. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the sweep grid changes as follow-up work from PR #17. It is concise and related to the main changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sweep-grid-review-followups

Comment @coderabbitai help to get the list of available commands.

@cirsteve
cirsteve merged commit ac58b97 into main Sep 10, 2026
3 checks passed
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.

1 participant