feat: v1.0.2 visualization notebooks, shared queries, and effect-size summary - #82
Conversation
Reporting a null needs an effect size beside the non-significant p, and a results table cannot carry one value per pairwise contrast. effect_sizes() now emits a summary block with the range of absolute Cohen's d and the widest contrast, and report.md prints it for both scoring conventions. Absolute values throughout: the sign of a pairwise d only records which strategy sorted first. Pairs with no finite d are counted rather than dropped, so a partial range is never presented as a complete one. Purely additive, so schema_version stays 1.1.
Add core-visualization.ipynb (Chapter 5 figures) and analysis_tables.ipynb (formatted statistics tables reading the canonical analysis outputs). Promote the queries both notebooks and the dashboard share into viz/queries.py with tests, rename the crew_ai display label to CrewAI, and add outcome-neutral colors to the theme. Wire an nbstripout pre-commit hook so notebook outputs, execution counts, and volatile metadata are stripped before commit, and ignore the generated out/ and output/ build artifacts.
Bundle the visualization notebooks, promoted queries, and analysis refinements (per-cell scoring, effect-size summary, mixed-effects) since v1.0.1 into the v1.0.2 CHANGELOG entry. Bump pyproject and CITATION.cff to 1.0.2. The v1.0.1 result database is unchanged.
Colinho22
added
documentation
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVersion 1.0.2 adds visualization notebooks, reusable convention-aware database queries, effect-size summaries in analysis outputs and reports, updated visualization styling, notebook-output stripping, and release metadata. ChangesAnalysis and visualization release
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant core_visualization_ipynb
participant maestro_viz_queries
participant maestro_db
participant figures
core_visualization_ipynb->>maestro_db: validate hash and load aggregates
core_visualization_ipynb->>maestro_viz_queries: request outcomes and taxonomy rates
maestro_viz_queries-->>core_visualization_ipynb: return visualization data
core_visualization_ipynb->>figures: save SVG, PNG, and figure_values.json
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Consolidate shared pandas/scipy imports into the setup cell (removing a cross-cell F811 redefinition), move mid-cell matplotlib imports to cell top, and wrap two long lines. Both notebooks pass ruff check + format and execute end-to-end. No figure output changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
pyproject.toml (1)
80-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the dependency with the pinned pre-commit hook.
pyproject.tomlallows anynbstripoutversion at or above0.8.1, while.pre-commit-config.yamlpins the hook to exactly0.8.1. If reproducible notebook stripping is intended, pin this dependency to the same version or document why drift is acceptable.Proposed alignment
- "nbstripout>=0.8.1", + "nbstripout==0.8.1",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 80 - 82, Align the nbstripout dependency declaration in pyproject.toml with the exact 0.8.1 version used by the pre-commit hook, ensuring both notebook-stripping entry points remain reproducible.src/maestro/viz/core-visualization.ipynb (1)
375-387: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOptional: reuse the promoted query instead of inline SQL. This
# PROMOTEblock now duplicatesq.mean_entity_id_f1_by_strategy_by_convention, which the tables notebook already consumes. Swapping to the tested function removes the divergence risk between the two notebooks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/maestro/viz/core-visualization.ipynb` around lines 375 - 387, The # PROMOTE block duplicates the logic provided by q.mean_entity_id_f1_by_strategy_by_convention. Replace the inline vo and itt SQL queries with calls to that promoted query function, passing the same strategy set and connection inputs, and preserve the existing valid-only and intent-to-treat results consumed by the visualization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/maestro/viz/core-visualization.ipynb`:
- Line 556: Remove the duplicate scipy.stats alias import from the notebook,
retaining a single import in the cell that uses st and eliminating the other
occurrence so Ruff F811 passes while preserving standalone execution where
appropriate.
- Line 595: Move the module-level imports to the beginning of their respective
notebook cells to satisfy Ruff E402: in src/maestro/viz/core-visualization.ipynb
lines 595-595 and 663-663, move from matplotlib.patches import Patch to the top
of the “Correctness by input complexity” and “Error-mode profile” cells; at
lines 978-978, move from matplotlib.colors import LinearSegmentedColormap to the
top of the “Strategy x model robustness” cell.
- Line 1017: Wrap the overlong return expression in the contrast-selection logic
so each branch remains within Ruff’s 88-character line limit, preserving the
existing color-selection behavior.
- Line 454: Wrap the overlong ax.plot call in the visualization notebook cell to
satisfy Ruff’s 88-character line limit, preserving the existing arguments and
plotting behavior.
In `@src/maestro/viz/queries.py`:
- Around line 143-159: The result conversion in the strategy aggregation helper
must handle a NULL valid_only value before calling float(). Update the list
comprehension using r["valid_only"] to reuse the sibling taxonomy rate helper’s
None-guard pattern or apply the established explicit default, while leaving
intent_to_treat conversion unchanged.
---
Nitpick comments:
In `@pyproject.toml`:
- Around line 80-82: Align the nbstripout dependency declaration in
pyproject.toml with the exact 0.8.1 version used by the pre-commit hook,
ensuring both notebook-stripping entry points remain reproducible.
In `@src/maestro/viz/core-visualization.ipynb`:
- Around line 375-387: The # PROMOTE block duplicates the logic provided by
q.mean_entity_id_f1_by_strategy_by_convention. Replace the inline vo and itt SQL
queries with calls to that promoted query function, passing the same strategy
set and connection inputs, and preserve the existing valid-only and
intent-to-treat results consumed by the visualization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d7d7134b-2ffc-48c2-9643-f94c7192fd95
📒 Files selected for processing (15)
.gitignore.pre-commit-config.yamlCHANGELOG.mdCITATION.cffdocs/visualization_design_guide.mdpyproject.tomlsrc/maestro/analysis/__main__.pysrc/maestro/analysis/statistics.pysrc/maestro/viz/analysis_tables.ipynbsrc/maestro/viz/core-visualization.ipynbsrc/maestro/viz/queries.pysrc/maestro/viz/theme.pytests/analysis/test_statistics.pytests/viz/test_theme.pytests/viz/test_views.py
Coalesce a NULL valid_only mean (a strategy with zero parsed runs) to 0.0 in mean_entity_id_f1_by_strategy_by_convention, matching the sibling taxonomy helper; add a test. Replace the notebook's inline mean-F1 SQL with the promoted query so the values-only cell and its table share one source.
Notebook cell helpers are catalogue-local, not importable library API, so they should not be gated by the src docstring-coverage metric (which exists for src/maestro/**/*.py). Adds add coderabbit.yaml*/*.ipynb to path_filters, consistent with the existing .db / .MMD / .JSON exclusions.
|
for the following findings RUFF also flagged them before you finished your review. Commit is underway
Your findings for these two things are valid and will be fixed now:
Your finding aout the Will trigger a review once commits are landed. |
|
Tip For best results, initiate chat on the files or code changes.
I’ll wait for the landed commits and your review trigger for the remaining fixes. 🐇 ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coderabbit.yaml`:
- Line 28: Remove the global "!**/*.ipynb" entry from reviews.path_filters in
coderabbit.yaml, or replace it with a coverage-specific setting that excludes
only notebook-local helpers while keeping notebooks in CodeRabbit’s general
review scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7761203-068c-4d30-a227-d54c5503dfc8
📒 Files selected for processing (5)
coderabbit.yamlsrc/maestro/viz/analysis_tables.ipynbsrc/maestro/viz/core-visualization.ipynbsrc/maestro/viz/queries.pytests/viz/test_views.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/maestro/viz/core-visualization.ipynb
- src/maestro/viz/queries.py
- src/maestro/viz/analysis_tables.ipynb
- tests/viz/test_views.py
ruff 0.16.0 formats Python inside markdown fences by default, which reformats doc code blocks and misreads illustrative fragments (a trailing-comma snippet becomes a tuple). CI floated to 0.16.0 via the unpinned ruff>=0.15.21 and failed on three docs. Cap at <0.16; adopt fence formatting deliberately later with explicit config. Aligns with the dependabot 0.15.22 bump.
Remove the add coderabbit.yaml*/*.ipynb path filter: it excluded notebooks from all CodeRabbit review, not just the docstring-coverage metric it was meant to quiet. The docstring check is a non-blocking warning, and notebook logic review is worth more than silencing it. Reverts the prior over-broad exclusion.
1 participant
Summary
Prepares the v1.0.2 "Polished visualization" release: two runnable
notebooks that reproduce every Chapter 5 figure and statistical table from the
frozen
v1.0.1database, the reusable queries they share with the dashboard,and the effect-size summary that lets a null result be reported with an effect
size beside its non-significant p.
The result database is unchanged. This branch changes how those results are
analysed and presented, not the data. It builds on the per-cell scoring change
already on
main(#80).What's in it
Visualization notebooks (
src/maestro/viz/)core-visualization.ipynbrenders the figures (reliability funnel,correctness with confidence intervals, pairwise strategy contrasts, error
profiles, correctness-vs-cost, strategy x model heatmap), writing an SVG, a
PNG, and a
figure_values.jsonof exact plotted values per figure.analysis_tables.ipynbformats the statistical tables (per-strategy F1, theone- and two-way ANOVAs, per-tier outcomes, error rates, efficiency,
mixed-effects estimates, per-model reliability) by reading the canonical
analysis JSON. It never recomputes a statistic, so a table can never disagree
with
report.md.references, so they read as a general visualization catalogue rather than a
thesis appendix. Every figure and table was verified against the thesis
exhibits digit-for-digit.
Shared queries (
viz/queries.py, +6 functions, all tested)run_outcomes_by_strategy,mean_entity_id_f1_by_strategy_by_convention,run_rates_by_tier,valid_rate_by_model,taxonomy_rates_per_valid_diagram,efficiency_by_strategy.one tested source. Each test pins its population and grain: per-cell means
for correctness, pooled counts for rates (the two are not
interchangeable).
Effect-size summary (
analysis/statistics.py,__main__.py)effect_sizesgains asummaryblock: the range of absolute Cohen's dacross the pairwise contrasts plus the widest contrast. Pairs with no finite
d are counted, not dropped, so a partial range is never shown as complete.
report.mdprints the range for both conventions. Additive only:schema_versionstays1.1.Supporting
crew_aidisplays asCrewAI(one word) across figures, tables, theme, anddashboard.
modes never read as extra strategies.
nbstripoutpre-commit hook +out//output/gitignored, so notebooks arecommitted as runnable sources and generated artifacts stay out.
Release
Bumps
pyproject(1.0.0 -> 1.0.2) andCITATION.cff(1.0.1 -> 1.0.2, dated2026-07-24), and moves the
[Unreleased]CHANGELOG section to[1.0.2]. Thev1.0.2tag and GitHub Release should be cut frommainafter merge.Verification
pytest); ruff check + format clean on all changed.py.output-stripped.
Notes for the reviewer
the thesis reviewer separately; in each the notebook computes the correct
value: a strategy x model interaction p (thesis
.93, correct.99), acorrelation CI bound (thesis
.899, correct.898), and a by-tiervalid-only F1 column that was a convention difference, not an error.
still in notebook cells pending a
viz/figures.pyextraction, after whichthe Streamlit dashboard views become thin consumers of
figures.py+queries.py. Deferred until after the defense.Summary by CodeRabbit