Interpolation preview/accept for flagged staves + stave-source verification + bbox tracing - #162
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 23 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 (2)
📝 WalkthroughWalkthroughThe change adds staffline interpolation preview and confirmation, records stave-source metadata, supports optional synthetic staff lines, adds MEI zone parity tracing, and extends frontend persistence and display. CI now installs PyYAML and runs the complete script test suite. ChangesStaffline encoding and provenance
Interpolation workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Reviewer
participant StafflineViewerModal
participant InferenceAPI
participant StafflineStage
Reviewer->>StafflineViewerModal: request interpolation preview
StafflineViewerModal->>InferenceAPI: POST interpolate-preview
InferenceAPI->>StafflineStage: compute preview records
StafflineStage-->>InferenceAPI: unpersisted records
InferenceAPI-->>StafflineViewerModal: preview response
Reviewer->>StafflineViewerModal: accept preview
StafflineViewerModal->>InferenceAPI: POST interpolate-confirm
InferenceAPI->>StafflineStage: rerun and persist detection
StafflineStage-->>InferenceAPI: new detection metadata
InferenceAPI-->>StafflineViewerModal: accepted staffline set
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: 4
🧹 Nitpick comments (2)
landing-page/scripts/mei_api.py (1)
24-24: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider constraining
staveSourceto the known tier tags.
types.tsmodelsstaveSourceas a closedStaveSourceunion, but this endpoint accepts any string and stores it verbatim. A client can persist a value the frontend does not model. ALiteralkeeps the database column aligned with the union.♻️ Proposed refactor
+from typing import Literal + +StaveSource = Literal[ + "staffline_detection", "yolo_annotation", "glyph_estimate", + "glyph_estimate_unresolved_lines", "glyph_estimate_synthetic_lines", + "placeholder_no_glyphs", +] + class AddMeiBody(BaseModel): name: str xmlContent: str imageName: Optional[str] = None logs: Optional[list[str]] = None - staveSource: Optional[str] = None + staveSource: Optional[StaveSource] = NoneAlso applies to: 31-33
🤖 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 `@landing-page/scripts/mei_api.py` at line 24, Constrain the staveSource parameter in the endpoint definition to a typing.Literal containing the known StaveSource tier tags, replacing the unrestricted Optional[str] annotation while preserving its default of None and existing optional behavior.landing-page/scripts/encode_to_mei.py (1)
295-297: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPropagate
allow_synthetic_linesto the missed-stave recovery path.
assign_glyphs_to_stavesalso calls_cluster_glyphs_into_staves(Line 192), but it cannot passallow_synthetic_lines. Staves that recover a row the detector missed therefore always getline_ys=[], even when the caller enabled synthetic lines. The result is a mixed page: tier-3 estimated staves carry synthetic geometry while recovered staves do not, andstave_sourcereports only the tier-3 label.If you want the option to apply consistently, thread the flag through
assign_glyphs_to_stavesas well.♻️ Proposed change outside the selected range
def assign_glyphs_to_staves( glyphs: list[Glyph], staves: list[StaveBbox], page_w: int, page_h: int, allow_synthetic_lines: bool = False, ) -> tuple[dict[int, list[Glyph]], list[StaveBbox]]: ... row_groups = _cluster_glyphs_into_staves( glyphs, page_w, page_h, id_prefix="row", allow_synthetic_lines=allow_synthetic_lines, )Then pass the flag from
tasks_encode.py's_encode_onecall site.Also applies to: 344-345
🤖 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 `@landing-page/scripts/encode_to_mei.py` around lines 295 - 297, Thread allow_synthetic_lines through assign_glyphs_to_staves and its _cluster_glyphs_into_staves recovery call, preserving the default-disabled behavior. Update tasks_encode.py’s _encode_one call site to pass the caller’s flag so recovered staves receive the same synthetic geometry option as tier-3 staves.
🤖 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/encode_to_mei.py`:
- Around line 968-980: Update the coordinate parsing in the stave-zone
verification logic to use float-compatible numeric conversion for all zone
attributes before comparing with the expected StaveBbox coordinates. Preserve
numeric comparison and ensure valid fractional coordinates cannot raise
ValueError during the tracing-only check invoked by _encode_one.
In `@landing-page/scripts/inference_api.py`:
- Around line 209-227: Update run_staffline_detection to yield the newly
inserted staffline detection id, and capture that id in the endpoint’s
event-processing loop. Replace the ORDER BY created_at/id latest-row query with
a lookup by the yielded primary key, preserving the existing missing-row error
handling and response field assignments.
- Around line 155-160: Convert preview_staffline_interpolation and the related
confirm endpoint from direct async execution into Celery-backed kickoff
handlers: validate inputs synchronously, create a jobs row containing the exact
task kwargs in params, enqueue the interpolation task, and return {"job_id":
...}. Move image loading, interpolation, and database work into the task so
handlers do not block the event loop or hold connections during processing.
In `@landing-page/src/components/project/StafflineViewerModal.tsx`:
- Around line 340-344: Add role="alert" to the error banner rendered by the
interpolateError conditional so assistive technology announces interpolation
failures while preserving the existing message and styling.
---
Nitpick comments:
In `@landing-page/scripts/encode_to_mei.py`:
- Around line 295-297: Thread allow_synthetic_lines through
assign_glyphs_to_staves and its _cluster_glyphs_into_staves recovery call,
preserving the default-disabled behavior. Update tasks_encode.py’s _encode_one
call site to pass the caller’s flag so recovered staves receive the same
synthetic geometry option as tier-3 staves.
In `@landing-page/scripts/mei_api.py`:
- Line 24: Constrain the staveSource parameter in the endpoint definition to a
typing.Literal containing the known StaveSource tier tags, replacing the
unrestricted Optional[str] annotation while preserving its default of None and
existing optional behavior.
🪄 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: ba8a591f-1554-44a4-a3de-ffa43b8f33d1
📒 Files selected for processing (21)
.github/workflows/tests.ymllanding-page/scripts/auth_api.pylanding-page/scripts/encode_api.pylanding-page/scripts/encode_to_mei.pylanding-page/scripts/inference_api.pylanding-page/scripts/mei_api.pylanding-page/scripts/projects_api.pylanding-page/scripts/staffline_stage.pylanding-page/scripts/tasks_encode.pylanding-page/scripts/tasks_predict.pylanding-page/scripts/tasks_text_batch.pylanding-page/scripts/tests/test_bbox_pipeline_integrity.pylanding-page/src/components/AppRouter.tsxlanding-page/src/components/project/MeiViewerModal.tsxlanding-page/src/components/project/ProjectDetail.tsxlanding-page/src/components/project/RhythmChart.tsxlanding-page/src/components/project/StafflineViewerModal.tsxlanding-page/src/components/project/StafflinesTab.tsxlanding-page/src/components/workflow/ProcessingPage.tsxlanding-page/src/hooks/useEncodingFlow.tslanding-page/src/types.ts
kyrieb-ekat
added a commit
that referenced
this pull request
Aug 6, 2026
- mei_api.py: constrain AddMeiBody.staveSource to a Literal matching types.ts's StaveSource union, instead of accepting any string. - encode_to_mei.py: thread allow_synthetic_lines through assign_glyphs_to_staves's missed-stave-recovery call to _cluster_glyphs_into_staves -- previously a page could end up with tier-3 staves carrying synthetic pitch geometry while recovered staves silently didn't, even with the flag on. - encode_to_mei.py: trace_stave_zone_parity() now parses zone coordinates as float instead of int -- a fractional StaveBbox coordinate (not enforced by the dataclass's type hints) would otherwise raise ValueError and abort the whole encode job over what's meant to be a tracing-only check. - inference_api.py: interpolate-preview/interpolate-confirm are now plain def, not async def, so FastAPI runs their blocking CPU-bound work (image decode, component filtering, centerline fitting) in its threadpool instead of stalling the event loop; confirm especially held a pooled DB connection open across the whole detection run. - staffline_stage.py / inference_api.py: run_staffline_detection now yields the newly inserted row's id directly; interpolate-confirm looks it up by that id instead of ORDER BY created_at DESC, id DESC LIMIT 1, which wasn't a reliable tiebreak (created_at uses NOW(), so same-transaction inserts can share a value; id is a random uuid4, not insertion order). - StafflineViewerModal.tsx: add role="alert" to the interpolation error banner so screen readers announce preview/confirm failures. All 6 findings were about code this PR itself introduced for the staffline feature -- none were pre-existing landing-page issues, so all addressed here rather than deferred.
Collaborator
Author
|
Pushed 305f470 addressing all 6 CodeRabbit findings (4 actionable + 2 nitpicks, replied individually on the 4 threaded ones):
All 6 were about code this PR itself introduced for the staffline feature -- nothing here touched unrelated landing-page code, per scope. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
landing-page/scripts/inference_api.py (1)
175-179: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBind confirmation to the previewed annotation revision.
_load_image_and_yolo_for_detectionloads the latest annotation on each request. If the annotation changes after preview, confirmation reruns interpolation with different input and persists a detection that does not match the accepted preview.Load the selected detection's stored
annotation_id, or return an annotation revision from preview and require confirmation to validate it before processing.Also applies to: 215-224
🤖 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 `@landing-page/scripts/inference_api.py` around lines 175 - 179, Update the preview and confirmation flow around _load_image_and_yolo_for_detection and the corresponding confirmation handler so confirmation is bound to the annotation revision used during preview. Return or persist the selected detection’s annotation_id with the preview, then validate that revision before rerunning interpolation and saving; reject confirmation when the annotation has changed.
🧹 Nitpick comments (1)
landing-page/scripts/staffline_stage.py (1)
331-352: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument and relay the
detection_idevent.
run_staffline_detection()emits{"type": "detection_id", "id": new_id}, but the generator docstring still lists only"log"and"error". Update the docstring and make sure every published event downstream handlesdetection_idwithout readingmessage; the current path re-publishedsf_ev, but extend the same pattern wherever staffline detection events are consumed.🤖 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 `@landing-page/scripts/staffline_stage.py` around lines 331 - 352, Update run_staffline_detection()’s docstring to document the detection_id event and its id payload alongside log and error events. Trace every downstream consumer and publisher of staffline detection events, including the sf_ev path, and relay detection_id events without accessing a message field; preserve existing handling for log and error events.
🤖 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 `@README.md`:
- Line 35: Add the text language identifier to both fenced code blocks in
README.md, including the blocks near the sections corresponding to lines 35 and
82, by changing each opening fence to use text.
---
Outside diff comments:
In `@landing-page/scripts/inference_api.py`:
- Around line 175-179: Update the preview and confirmation flow around
_load_image_and_yolo_for_detection and the corresponding confirmation handler so
confirmation is bound to the annotation revision used during preview. Return or
persist the selected detection’s annotation_id with the preview, then validate
that revision before rerunning interpolation and saving; reject confirmation
when the annotation has changed.
---
Nitpick comments:
In `@landing-page/scripts/staffline_stage.py`:
- Around line 331-352: Update run_staffline_detection()’s docstring to document
the detection_id event and its id payload alongside log and error events. Trace
every downstream consumer and publisher of staffline detection events, including
the sf_ev path, and relay detection_id events without accessing a message field;
preserve existing handling for log and error events.
🪄 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: 1060718d-6bfb-4e87-aac4-5d3f86d92757
📒 Files selected for processing (7)
README.mdlanding-page/scripts/encode_to_mei.pylanding-page/scripts/inference_api.pylanding-page/scripts/mei_api.pylanding-page/scripts/staffline_stage.pylanding-page/scripts/tasks_encode.pylanding-page/src/components/project/StafflineViewerModal.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- landing-page/scripts/mei_api.py
- landing-page/scripts/tasks_encode.py
- landing-page/src/components/project/StafflineViewerModal.tsx
|
|
||
| ## Repository Structure | ||
| Assume the branch is `main` unless otherwise specified. | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to the fenced code blocks.
markdownlint reports MD040 because these fences omit a language. Use text for both blocks.
Proposed fix
-```
+```textApply this change at Lines 35 and 82.
Also applies to: 82-82
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@README.md` at line 35, Add the text language identifier to both fenced code
blocks in README.md, including the blocks near the sections corresponding to
lines 35 and 82, by changing each opening fence to use text.
Source: Linters/SAST tools
Red overlay lines/badges were blending into the red rubrics/staff-lines common in these manuscripts. Reapplied against the current file content -- c8bd9ec (the raw-tab/copy/download feature) landed on main after this fix was first drafted, so re-read both files fresh before editing rather than replaying a stale diff.
Adds mei_files.stave_source, tagged at every tier decision point in tasks_encode.py (staffline_detection / yolo_annotation / glyph_estimate / glyph_estimate_unresolved_lines / glyph_estimate_synthetic_lines / placeholder_no_glyphs), surfaced as a badge in MeiViewerModal.tsx. Previously this only existed as a transient job-event log line with no durable per-file record. Also flips estimate_staves_from_glyphs's tier-3 fallback (_cluster_glyphs_into_staves) to default OFF on fabricating evenly-spaced line_ys when too few real staff-line glyphs exist -- it now leaves line_ys empty (pitch explicitly unresolved, via _nc_pitch's own <2-entry guard) unless allow_synthetic_lines is passed, threaded through from encode-upload/encode-batch's kickoff body. Previously this silently produced plausible-looking but invented pitch geometry by default. Also fixes useEncodingFlow.ts's add_mei calls, which hardcoded logs: [] even though mei_api.py's project_logs plumbing to store them already existed -- ProcessingPage.tsx now attaches the actually-collected log lines (filtered to the current item for batch jobs) to the result event.
Adds [trace]-prefixed job-log lines at each handoff in the stave-detection/encoding pipeline, so the bbox chain's integrity is directly inspectable per run instead of only assertable by code review: - tasks_predict.py / tasks_text_batch.py: log the resolved model's label/hash right when stave-class boxes are found, confirming which checkpoint actually produced them. - staffline_stage.py: log raw stave-class box count and how many survived crop+fit (non-degenerate). - tasks_encode.py's _resolve_hints(): log staves_from_jsomr()'s JSOMR-record-in vs StaveBbox-out counts. - encode_to_mei.py: new trace_stave_zone_parity() (DB-independent, so it's unit-testable like the rest of this module) parses the just-built MEI's actual <zone type="staff"> elements back out and confirms they match the StaveBbox list build_mei() was handed -- same count, same coordinates -- logging a loud [warn] instead of a quiet line if they ever diverge.
test_bbox_pipeline_integrity.py: hand-built JSOMR fixture -> staves_from_jsomr() -> build_mei() -> trace_stave_zone_parity(), asserting zone coordinates survive unchanged (and that the trace actually flags a genuine divergence, not just the happy path). Starts from JSOMR rather than a raw YOLO .txt since the real detection stage (staffline_stage.run_staffline_detection) imports job_store -> auth_api, which connects to Postgres at import time -- same DB-independence constraint test_staffline_adapter.py's own docstring already works around. Also asserts medieval_models.py's bundled stave-detector filename/class-map haven't silently drifted. CI: run the whole landing-page/scripts/tests/ directory instead of naming test_staffline_adapter.py specifically, and add PyYAML to the install step (medieval_models.py -> config.py needs it, previously untested).
Gives flagged (under-populated) staves something actionable in StafflineViewerModal, instead of just a status display. interpolate_missing already existed as a plumbed-through parameter to run_staffline_detection but was never triggered from the app (staff-finding/dox/STATUS.md flags it as not yet validated across the corpus), hence a review-before-persist flow rather than a one-click apply.
- staffline_stage.py: extracted _fit_and_group() (per-box fit + stave grouping, job_id-optional so it works outside a Celery task) out of run_staffline_detection(), and added preview_interpolation() -- same computation with interpolate_missing=True, no DB write.
- inference_api.py: two new routes mirroring the existing GET stafflines/{id} detail route's auth/lookup style -- POST .../interpolate-preview (computes only) and .../interpolate-confirm (re-runs for real via run_staffline_detection, persists as a new staffline_detections row per that table's existing accumulate-forever design). Both resolve the image's CURRENT annotation by image_id rather than the detection's possibly-stale annotation_id, since re-annotating replaces that row entirely.
- StafflineViewerModal.tsx: 'interpolate missing lines' button appears when any stave is flagged; preview renders as the same overlay view (dashed lines, existing visual language) with accept/discard controls swapped in for the tab switcher. Accepting swaps the modal to the newly confirmed detection.
- StafflinesTab.tsx / ProjectDetail.tsx: onAddStaffline merges the newly confirmed detection into project.stafflines client-side, no full project refetch.
- mei_api.py: constrain AddMeiBody.staveSource to a Literal matching types.ts's StaveSource union, instead of accepting any string. - encode_to_mei.py: thread allow_synthetic_lines through assign_glyphs_to_staves's missed-stave-recovery call to _cluster_glyphs_into_staves -- previously a page could end up with tier-3 staves carrying synthetic pitch geometry while recovered staves silently didn't, even with the flag on. - encode_to_mei.py: trace_stave_zone_parity() now parses zone coordinates as float instead of int -- a fractional StaveBbox coordinate (not enforced by the dataclass's type hints) would otherwise raise ValueError and abort the whole encode job over what's meant to be a tracing-only check. - inference_api.py: interpolate-preview/interpolate-confirm are now plain def, not async def, so FastAPI runs their blocking CPU-bound work (image decode, component filtering, centerline fitting) in its threadpool instead of stalling the event loop; confirm especially held a pooled DB connection open across the whole detection run. - staffline_stage.py / inference_api.py: run_staffline_detection now yields the newly inserted row's id directly; interpolate-confirm looks it up by that id instead of ORDER BY created_at DESC, id DESC LIMIT 1, which wasn't a reliable tiebreak (created_at uses NOW(), so same-transaction inserts can share a value; id is a random uuid4, not insertion order). - StafflineViewerModal.tsx: add role="alert" to the interpolation error banner so screen readers announce preview/confirm failures. All 6 findings were about code this PR itself introduced for the staffline feature -- none were pre-existing landing-page issues, so all addressed here rather than deferred.
Replaces the stale DocLayout-YOLO/annotator-era README with an overview of the live landing-page app, an ecosystem map of related pieces (IC, pitch finding, staffline detection, mothra print, text-finding), and pointers to CLAUDE.md for technical detail.
kyrieb-ekat
force-pushed
the
kyrie/staffline-interpolate-and-tracing
branch
from
August 7, 2026 17:04
8e93157 to
b783369
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/tests.yml (1)
101-102: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable persisted checkout credentials.
actions/checkout@v4keeps its GitHub token in localgit configby default. Thefrontendjob runsnpm ci, and dependency lifecycle scripts can read that token. Setpersist-credentials: falseon both checkout steps, or remove checkout persistence from this workflow unless authenticatedgitcommands are needed.🤖 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 @.github/workflows/tests.yml around lines 101 - 102, Update both checkout steps in the workflow to disable persisted credentials by setting persist-credentials to false on each actions/checkout@v4 invocation; leave checkout behavior otherwise unchanged.Source: Linters/SAST tools
🤖 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/inference_api.py`:
- Around line 229-259: Refactor the route around db_cursor and
run_staffline_detection so the database connection is released before CPU-bound
staffline computation: load the required inputs within db_cursor, close that
context, then run detection without holding con/cur, and reacquire a connection
afterward for inserting or querying the generated result. Preserve the existing
error handling and use of new_detection_id while ensuring all database
operations still receive an active cursor and connection.
---
Outside diff comments:
In @.github/workflows/tests.yml:
- Around line 101-102: Update both checkout steps in the workflow to disable
persisted credentials by setting persist-credentials to false on each
actions/checkout@v4 invocation; leave checkout behavior otherwise unchanged.
🪄 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: bbcd95f0-337d-4d39-8ab4-4e471498d0e4
📒 Files selected for processing (6)
.github/workflows/tests.ymllanding-page/scripts/inference_api.pylanding-page/scripts/tasks_predict.pylanding-page/scripts/tasks_text_batch.pylanding-page/src/components/AppRouter.tsxlanding-page/src/components/project/ProjectDetail.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- landing-page/src/components/project/ProjectDetail.tsx
- landing-page/scripts/tasks_predict.py
- landing-page/src/components/AppRouter.tsx
Follow-up CodeRabbit finding on PR #162: interpolate-confirm was holding a pooled db_cursor() connection open for the entire staffline detection run (component filtering/centerline fitting), even though the earlier async->def fix only addressed the event-loop blocking, not the connection-pool exhaustion risk from concurrent confirmations each pinning a connection for that whole duration. staffline_stage.py: split run_staffline_detection's persist step into a new persist_staffline_detection() (shared INSERT-and-commit, used by both run_staffline_detection itself and the route below), and added compute_staffline_interpolation() -- like preview_interpolation() but returns everything persist_staffline_detection() needs (stave_ids/grouping_result/scale_unit/settings, not just records) -- so a caller can compute without ever holding a connection, and only acquire one for the actual write. inference_api.py: interpolate-confirm now loads inputs (connection released), computes via compute_staffline_interpolation() with no connection held, then reacquires one just for persist_staffline_detection() and the id lookup. Left out of this PR (unrelated to staffline): CodeRabbit's other finding on the same review, disabling persist-credentials on .github/workflows/tests.yml's checkout steps -- spun off as its own branch/PR since it's a general CI security concern, not staffline-specific.
This was referenced Aug 7, 2026
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/RhythmChart-- red blended into these manuscripts' red rubrics/staff-lines.interpolate_missingalready existed as a plumbed-through parameter but was never triggered from the app.tasks_encode.py's 3-tier stave-source fallback produced each MEI file's zones (mei_files.stave_source), surfaced as a badge inMeiViewerModal.useEncodingFlow.ts'sadd_meicalls, which hardcodedlogs: []even though the backend'sproject_logsplumbing to store them already existed.[trace]log lines through the whole stave-bbox pipeline (model used -> box survival counts -> JSOMR-to-StaveBbox counts -> a hard zone-coordinate parity check against the MEI actually written), so the bbox chain's integrity is directly inspectable per run.test_bbox_pipeline_integrity.py): JSOMR ->build_mei-> zone-parity, a genuine-divergence case, and a check that the bundled stave-detector checkpoint/class-map hasn't silently drifted.Context
Follow-up to #154. Verified that stave bounding-box data reaching Neon is not using placeholder/default values in the normal case -- but there was no durable way to check that after the fact, and one fallback path did fabricate plausible-looking-but-invented pitch geometry by default. Both addressed here.
Changes
landing-page/scripts/staffline_stage.py: extracted_fit_and_group()(job-id-optional) out ofrun_staffline_detection(); addedpreview_interpolation().landing-page/scripts/inference_api.py: newPOST .../stafflines/{id}/interpolate-previewand.../interpolate-confirmroutes.landing-page/scripts/encode_to_mei.py:estimate_staves_from_glyphs()now returns(staves, stave_source_tag); newallow_synthetic_linesparam (defaultFalse); newtrace_stave_zone_parity()helper.landing-page/scripts/tasks_encode.py/tasks_predict.py/tasks_text_batch.py: tier tagging,allow_synthetic_linesthreading,[trace]log lines.landing-page/scripts/auth_api.py:mei_files.stave_sourcemigration.landing-page/src/components/project/StafflineViewerModal.tsx: interpolate-missing-lines button, preview/accept/discard flow, anomaly color fix.landing-page/src/components/project/MeiViewerModal.tsx:stave_sourcebadge.landing-page/src/hooks/useEncodingFlow.ts/ProcessingPage.tsx: real log lines threaded through toadd_meiinstead oflogs: [].Verification
tsc --noEmitclean across the whole branch.eslintacross every touched file: zero new warnings/errors vs. each file's pre-change baseline (checked viagit stashdiff per file).landing-page/scripts/tests/: 10/10 pass, including the newtest_bbox_pipeline_integrity.py.[trace]lines showing up in the job-log dropdown.🤖 Generated with Claude Code
Summary by CodeRabbit