Surface staffline rhythm diagnostics + close text-batch staffline gap - #154
Merged
Merged
Conversation
Surfaces the gap-distribution diagnostic group_staves.py computes server-side (previously only visible via run_page.py's own matplotlib output, never through the landing-page pipeline). Computed client-side from the already-stored JSOMR JSON via a new rhythmGaps.ts helper, rendered with a new inline-SVG RhythmChart component -- no new backend storage. Added the missing scale_unit field to JsomrLineRecord in types.ts (already sent by the backend, just not typed).
tasks_text_batch.py (the Celery task backing /text-batch/run, the Cantus-aligned batch text-finding path) runs its own independent YOLO layer-separation pass and previously never called staffline_stage.py, so images processed only through it got no staffline_detections row -- a known gap flagged in documentation_allons-y/STAFFLINE_INTEGRATION_FOLLOWUPS.md. Mirrors tasks_predict.py's existing has_class -> run_staffline_detection block, right after write_annotation().
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesStaffline processing and rhythm diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TextBatchPipeline
participant YOLO
participant StafflineDetector
participant EventPublisher
TextBatchPipeline->>YOLO: Run image inference
TextBatchPipeline->>StafflineDetector: Detect stafflines when the class is present
StafflineDetector-->>TextBatchPipeline: Return events or an error
TextBatchPipeline->>EventPublisher: Publish events or log errors
sequenceDiagram
participant StafflineViewerModal
participant computeRhythmGaps
participant RhythmChart
StafflineViewerModal->>computeRhythmGaps: Provide ready line records
computeRhythmGaps-->>StafflineViewerModal: Return rhythm summary
StafflineViewerModal->>RhythmChart: Render the Rhythm tab
RhythmChart-->>StafflineViewerModal: Display the SVG chart
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@landing-page/scripts/tasks_text_batch.py`:
- Around line 90-93: Update the staffline-detection handling around
run_staffline_detection so error events are published unchanged to
publish_event, preserving their type: "error" and the resulting failed job
status. Only retain the log conversion if the text-batch event contract
explicitly treats staffline failures as non-fatal.
In `@landing-page/src/components/project/RhythmChart.tsx`:
- Around line 106-118: Update the legend spans in RhythmChart’s chart legend to
replace each fixed-color inline style with Tailwind arbitrary color utility
classes, using the existing NOISE_FLOOR_COLOR, CUT_THRESHOLD_COLOR,
MAX_INTERP_COLOR, and ANOMALY_COLOR values. Remove the corresponding style props
while preserving the legend text and layout.
- Around line 39-104: Update the SVG rendered by RhythmChart to include an
accessible title and description or equivalent screen-reader data summary
describing the chart and its bar values. In the bars mapping, preserve the
anomaly color while adding a non-color visual indicator such as a distinct
stroke, pattern, or label whenever b.isAnomalous is true.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 16af9a08-d752-4ba0-a94e-d89438b20966
📒 Files selected for processing (5)
landing-page/scripts/tasks_text_batch.pylanding-page/src/components/project/RhythmChart.tsxlanding-page/src/components/project/StafflineViewerModal.tsxlanding-page/src/lib/rhythmGaps.tslanding-page/src/types.ts
- RhythmChart.tsx: add an SVG <title>/<desc> (page-level and per-bar) so the chart has an accessible name/data summary, and give anomalous bars a dark stroke outline so they are distinguishable without relying on color alone.
- RhythmChart.tsx: replace the legend's inline style={{color:...}} props with Tailwind arbitrary-value classes, per this repo's Tailwind-only styling rule (CLAUDE.md).
Left the third finding (tasks_text_batch.py:93, staffline errors downgraded from type:error to type:log before publish_event) as-is with a reply explaining it -- this is the exact existing pattern tasks_predict.py already uses (job_store.publish_event flips the whole job to failed on any type:error event, and a per-image staffline hiccup already gets its own status=failed row in staffline_detections, so it deliberately shouldn't fail the entire predict/text-batch job).
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
1 participant
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
StafflineViewerModal.tsx, reconstructing the diagnosticgroup_staves.pycomputes server-side (previously only visible viarun_page.py's own matplotlib output, never through the landing-page pipeline). No new backend storage -- computed from the already-stored JSOMR JSON.documentation_allons-y/STAFFLINE_INTEGRATION_FOLLOWUPS.md):tasks_text_batch.py's independent YOLO pass (backing/text-batch/run, the Cantus-aligned batch path) never calledstaffline_stage.py, so images processed only through it got nostaffline_detectionsrow. Now mirrorstasks_predict.py's existinghas_class->run_staffline_detectionblock.Context
Local
mainhad fallen behindorigin/mainby the whole staffline integration (staffline_stage.py, the StafflinesTab UI, etc. all already existed upstream), which is what looked like nothing was wired up. This PR is the follow-up work once synced: filling in the two real gaps that remained.Changes
landing-page/src/lib/rhythmGaps.ts(new): per-stave consecutive-line gap computation + noise-floor/cut-threshold/max-interp-gap reference values, mirroringgroup_staves.py's multipliers.landing-page/src/components/project/RhythmChart.tsx(new): inline-SVG bar chart rendering the above.landing-page/src/components/project/StafflineViewerModal.tsx: overlay/rhythm tab toggle.landing-page/src/types.ts: added missingscale_unitfield toJsomrLineRecord(already sent by the backend, just untyped).landing-page/scripts/tasks_text_batch.py: runs staffline detection after YOLO layer-separation, same as the predict-job path.Verification
tsc --noEmitclean.eslinton changed files: only the 2 pre-existing warnings/errors that were already there before this change (confirmed via stash diff), nothing new.landing-page/scripts/tests/test_staffline_adapter.py: 7/7 pass.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes