Handle phantom-continuation folios in upload/batch flow - #174
Merged
Conversation
A folio with no chant-start rows in Cantus DB's CSV (e.g. CH-Fco Ms. 2's 003r, where all its text continues the previous folio's chant) never appears in Cantus DB's own folio list. Since every folio picker in landing-page was sourced only from that list, such a folio could not be uploaded at all: batch uploads hit a hard "counts must match" block with no override, and single-image uploads had no way to select it. - landing-page/src/utils/folio.ts: areFoliosContiguous (ports mothra-text/run_chain.py's _are_contiguous), isValidFolioShape, and buildEffectiveFolioSequence - reconciles the canonical folio list against uploaded filenames, recognizing a genuine single-step gap (an uploaded image the user demonstrably has, contiguous with its neighbors) as a phantom-continuation folio rather than an error. - ImageTab.tsx: effectiveFolioSequence tracks the reconciled sequence through runBatchUpload/finishUpload; computeFolioReviewRows surfaces a new "phantom-continuation" status ahead of "not-in-source"; visible badges on grid/QuickLook/BatchTab folio labels. - New shared FolioSelect.tsx: adds a "custom folio..." free-text fallback to the three closed folio <select>s (CantusSourcePanel x3, EditFolioModel), since single-image uploads have no batch to auto-detect adjacency from and need direct manual entry. - ProjectDetail.tsx: batchFolioSequence trusts a manually-typed off-canonical start/end boundary instead of collapsing to empty. - BatchFolioReviewModal.tsx/BatchTab.tsx: distinct, non-error styling for the new status. Also fixes a related backend bug surfaced while testing this: text-service's /batch-run threaded prev_folio_state between every folio in a batch unconditionally, with no check that consecutive folios were actually manuscript-adjacent - so skipping a folio (e.g. running 002v then 003v without 003r) silently carried 002v's leftover continuation words into 003v. text-service/main.py now imports mothra-text's _are_contiguous and resets prev_folio_state (and suppresses infer_continuation) whenever two folios in a batch aren't truly contiguous, mirroring run_chain.py's existing CLI-side guard. mothra-text submodule bumped to include both the infer_continuation suppression fix (mothra-text#43) and the empty-remaining_words authoritative-state fix (mothra-text, cassie/fix-empty-remaining-words-fallthrough) discovered while verifying this end-to-end. Verified: tsc/vite build clean, 253/253 mothra-text tests pass, and manual reproduction against the live app for both the 002v/003r/003v batch and the 002v/003v (003r skipped) regression case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous bump referenced the empty-remaining_words fix's own feature branch tip before it was merged (mothra-text#44). Now that it's merged, point at main directly instead of an unmerged branch.
📝 WalkthroughWalkthroughThe change adds custom folio selection and phantom-continuation handling. Batch uploads reconcile detected folios with canonical sequences. The interface labels phantom continuations. The text service resets continuation inference after non-contiguous folios. ChangesFolio continuation handling
Repository maintenance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ImageTab
participant FolioUtilities
participant TextService
User->>ImageTab: Start batch upload
ImageTab->>FolioUtilities: Reconcile detected folio guesses
FolioUtilities-->>ImageTab: Return effective sequence and phantom flags
ImageTab->>TextService: Process folios with continuation setting
TextService-->>ImageTab: Return processed batch results
ImageTab-->>User: Display folios and continuation status
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: 5
🤖 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/src/components/project/ImageTab.tsx`:
- Around line 484-495: Defer the setEffectiveFolioSequence update in the
oversized-upload path until confirmation: store the computed candidate sequence
in pendingSizeWarning alongside the upload data, apply it only when the user
continues, and restore or retain the prior sequence when cancelling. Update the
relevant confirmation and cancellation handlers in ImageTab so cancelled uploads
leave the batch sequence unchanged.
- Around line 480-488: Update the reconciliation flow around
buildEffectiveFolioSequence to pass the preceding assigned folio from the
already accepted sequence, rather than starting from remaining alone, so later
uploads can identify genuine gaps. Exclude generated PDF-page names by passing
undefined as their folio guess, and apply the same preceding-folio context and
PDF handling in computeFolioReviewRows.
In `@landing-page/src/components/project/ProjectDetail.tsx`:
- Around line 121-123: The folio range construction around the startIdx/endIdx
logic must not add unrelated canonical folios for custom boundaries. Update the
custom-start branch to begin from the custom folio’s intended canonical
neighbor, and apply the same placement correction to the custom-end branch; if
either boundary cannot be placed relative to its canonical neighbor, reject it
rather than constructing an invalid range.
In `@landing-page/src/components/shared/FolioSelect.tsx`:
- Around line 45-49: Expose custom-input validity through FolioSelect’s
component contract, forwarding raw custom input or an explicit invalid state so
consumers cannot retain stale values; selecting custom mode in FolioSelect must
not replace the persisted folio with an empty value. In CantusSourcePanel, block
grid uploads when the custom folio is invalid or empty and block batch-range
operations when either custom boundary is invalid. In EditFolioModel, disable
saving until the custom folio is valid. Apply these changes at
landing-page/src/components/shared/FolioSelect.tsx lines 45-49 and 79-83,
landing-page/src/components/project/CantusSourcePanel.tsx lines 194-202 and
223-237, and landing-page/src/components/project/EditFolioModel.tsx lines 23-30.
In `@landing-page/src/utils/folio.ts`:
- Around line 246-250: Update the fallback sequence-building logic around
nextCanonical, canonicalConsumed, and c so c increments only when nextCanonical
is defined. Keep pushing the fallback value and updating isPhantom as before,
while ensuring canonicalConsumed never exceeds the available canonical sequence
length.
🪄 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: 4783a84b-8239-49c3-bb93-81dfd3b46c3e
📒 Files selected for processing (11)
.gitignorelanding-page/src/components/project/BatchFolioReviewModal.tsxlanding-page/src/components/project/BatchTab.tsxlanding-page/src/components/project/CantusSourcePanel.tsxlanding-page/src/components/project/EditFolioModel.tsxlanding-page/src/components/project/ImageTab.tsxlanding-page/src/components/project/ProjectDetail.tsxlanding-page/src/components/shared/FolioSelect.tsxlanding-page/src/utils/folio.tsmothra-texttext-service/main.py
Comment on lines
+480
to
+488
| const remaining = effectiveFolioSequence.slice(batchImages.length); | ||
| const guesses = combined.map((f) => extractFolioFromFilename(f.name)); | ||
| const { sequence, canonicalConsumed } = buildEffectiveFolioSequence(remaining, guesses); | ||
| batchPositionalFolios = sequence; | ||
| setEffectiveFolioSequence((prev) => [ | ||
| ...prev.slice(0, batchImages.length), | ||
| ...sequence.filter((f): f is string => f !== undefined), | ||
| ...remaining.slice(canonicalConsumed), | ||
| ]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Provide correct reconciliation context.
remaining excludes the prior accepted folio. A later upload of 003r after an uploaded 002v starts reconciliation with 003v, so buildEffectiveFolioSequence cannot identify 003r as a genuine gap.
guesses also includes generated PDF-page names. A PDF named with a folio token can create a false phantom continuation for a later page.
Pass the preceding assigned folio into reconciliation. Pass undefined for PDF pages. Use the same context in computeFolioReviewRows.
🤖 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/src/components/project/ImageTab.tsx` around lines 480 - 488,
Update the reconciliation flow around buildEffectiveFolioSequence to pass the
preceding assigned folio from the already accepted sequence, rather than
starting from remaining alone, so later uploads can identify genuine gaps.
Exclude generated PDF-page names by passing undefined as their folio guess, and
apply the same preceding-folio context and PDF handling in
computeFolioReviewRows.
Comment on lines
+484
to
495
| setEffectiveFolioSequence((prev) => [ | ||
| ...prev.slice(0, batchImages.length), | ||
| ...sequence.filter((f): f is string => f !== undefined), | ||
| ...remaining.slice(canonicalConsumed), | ||
| ]); | ||
| } | ||
|
|
||
| if (oversized.length > 0) { | ||
| setConverting(false); | ||
| section.setUploadModal(false); | ||
| setPendingSizeWarning({ imageFiles, pdfPageFiles, oversized, folioOverride }); | ||
| setPendingSizeWarning({ imageFiles, pdfPageFiles, oversized, folioOverride, batchPositionalFolios }); | ||
| return; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not commit the effective sequence before upload confirmation.
An oversized upload updates effectiveFolioSequence before the warning opens. If the user cancels, no image is uploaded, but the phantom folio remains in the batch sequence. The next upload can receive the wrong folio.
Keep the candidate sequence in pendingSizeWarning. Commit it only when the user continues. Restore the prior sequence on cancel.
🤖 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/src/components/project/ImageTab.tsx` around lines 484 - 495,
Defer the setEffectiveFolioSequence update in the oversized-upload path until
confirmation: store the computed candidate sequence in pendingSizeWarning
alongside the upload data, apply it only when the user continues, and restore or
retain the prior sequence when cancelling. Update the relevant confirmation and
cancellation handlers in ImageTab so cancelled uploads leave the batch sequence
unchanged.
Comment on lines
+121
to
+123
| if (startIdx === -1 && endIdx === -1) return [batchStartFolio, batchEndFolio]; | ||
| if (startIdx === -1) return endIdx === -1 ? [] : [batchStartFolio, ...folios.slice(0, endIdx + 1)]; | ||
| if (endIdx === -1) return [...folios.slice(startIdx), batchEndFolio]; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Build the range relative to a custom boundary.
Line 122 prepends the complete canonical prefix after any custom start. If the user selects custom 003r and canonical 003v, the generated sequence is 003r, 001r, ... 003v. Subsequent uploads receive incorrect folios.
Apply the same correction to the custom end case on Line 123. Locate a custom boundary through its intended canonical neighbour, or reject an unplaceable custom boundary. Do not add unrelated canonical folios.
🤖 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/src/components/project/ProjectDetail.tsx` around lines 121 -
123, The folio range construction around the startIdx/endIdx logic must not add
unrelated canonical folios for custom boundaries. Update the custom-start branch
to begin from the custom folio’s intended canonical neighbor, and apply the same
placement correction to the custom-end branch; if either boundary cannot be
placed relative to its canonical neighbor, reject it rather than constructing an
invalid range.
Comment on lines
+45
to
+49
| onChange={(e) => { | ||
| const v = e.target.value; | ||
| setCustomInput(v); | ||
| if (v === "" || isValidFolioShape(v)) onChange(v); | ||
| }} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Expose custom-folio validity to consumers.
FolioSelect keeps invalid text in local state but does not update or invalidate the parent value. Consumers can then submit a stale folio, or the empty value set when custom mode starts, while the user sees invalid text.
landing-page/src/components/shared/FolioSelect.tsx#L45-L49: expose raw input validity through the component contract instead of silently withholding invalid edits.landing-page/src/components/shared/FolioSelect.tsx#L79-L83: do not make selecting custom mode sufficient to replace the persisted value with an empty value.landing-page/src/components/project/CantusSourcePanel.tsx#L194-L202: prevent a grid upload from using a stale or empty folio while custom input is invalid.landing-page/src/components/project/CantusSourcePanel.tsx#L223-L237: prevent batch-range use while either custom boundary is invalid.landing-page/src/components/project/EditFolioModel.tsx#L23-L30: disable save until the custom folio is valid.
📍 Affects 3 files
landing-page/src/components/shared/FolioSelect.tsx#L45-L49(this comment)landing-page/src/components/shared/FolioSelect.tsx#L79-L83landing-page/src/components/project/CantusSourcePanel.tsx#L194-L202landing-page/src/components/project/CantusSourcePanel.tsx#L223-L237landing-page/src/components/project/EditFolioModel.tsx#L23-L30
🤖 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/src/components/shared/FolioSelect.tsx` around lines 45 - 49,
Expose custom-input validity through FolioSelect’s component contract,
forwarding raw custom input or an explicit invalid state so consumers cannot
retain stale values; selecting custom mode in FolioSelect must not replace the
persisted folio with an empty value. In CantusSourcePanel, block grid uploads
when the custom folio is invalid or empty and block batch-range operations when
either custom boundary is invalid. In EditFolioModel, disable saving until the
custom folio is valid. Apply these changes at
landing-page/src/components/shared/FolioSelect.tsx lines 45-49 and 79-83,
landing-page/src/components/project/CantusSourcePanel.tsx lines 194-202 and
223-237, and landing-page/src/components/project/EditFolioModel.tsx lines 23-30.
Comment on lines
+246
to
+250
| // fall back to today's positional behavior | ||
| sequence.push(nextCanonical); | ||
| isPhantom.push(false); | ||
| if (nextCanonical !== undefined) lastAccepted = nextCanonical; | ||
| c++; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep canonicalConsumed within the canonical sequence.
Line 250 increments c when nextCanonical is undefined. A batch with more guesses than canonical entries then reports canonical entries that do not exist as consumed. Increment c only when nextCanonical exists.
Proposed fix
sequence.push(nextCanonical);
isPhantom.push(false);
- if (nextCanonical !== undefined) lastAccepted = nextCanonical;
- c++;
+ if (nextCanonical !== undefined) {
+ lastAccepted = nextCanonical;
+ c++;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // fall back to today's positional behavior | |
| sequence.push(nextCanonical); | |
| isPhantom.push(false); | |
| if (nextCanonical !== undefined) lastAccepted = nextCanonical; | |
| c++; | |
| // fall back to today's positional behavior | |
| sequence.push(nextCanonical); | |
| isPhantom.push(false); | |
| if (nextCanonical !== undefined) { | |
| lastAccepted = nextCanonical; | |
| c++; | |
| } |
🤖 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/src/utils/folio.ts` around lines 246 - 250, Update the fallback
sequence-building logic around nextCanonical, canonicalConsumed, and c so c
increments only when nextCanonical is defined. Keep pushing the fallback value
and updating isPhantom as before, while ensuring canonicalConsumed never exceeds
the available canonical sequence length.
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.
Closes #165.
A folio with no chant-start rows in Cantus DB's CSV (e.g. CH-Fco Ms. 2's 003r, where all its text continues the previous folio's chant) never appears in Cantus DB's own folio list. Since every folio picker in landing-page was sourced only from that list, such a folio couldn't be uploaded at all: batch uploads hit a hard "counts must match" block with no override, and single-image uploads had no way to select it at all.
What changed
landing-page/src/utils/folio.ts:areFoliosContiguous(portsmothra-text/run_chain.py's_are_contiguous),isValidFolioShape, andbuildEffectiveFolioSequence— reconciles the canonical folio list against uploaded filenames, recognizing a genuine single-step gap (an uploaded image the user demonstrably has, contiguous with its neighbors) as a phantom-continuation folio rather than an error.ImageTab.tsx:effectiveFolioSequencetracks the reconciled sequence throughrunBatchUpload/finishUpload;computeFolioReviewRowssurfaces a new"phantom-continuation"status ahead of"not-in-source"; visible badges on grid/QuickLook/BatchTab folio labels.FolioSelect.tsx: adds a "custom folio…" free-text fallback to the three closed folio<select>s (CantusSourcePanel×3,EditFolioModel), since single-image uploads have no batch to auto-detect adjacency from and need direct manual entry.ProjectDetail.tsx:batchFolioSequencenow trusts a manually-typed off-canonical start/end boundary instead of collapsing to empty.BatchFolioReviewModal.tsx/BatchTab.tsx: distinct, non-error styling for the new status.Related backend fix
Testing this end-to-end surfaced a real, independent bug:
text-service's/batch-runthreadedprev_folio_statebetween every folio in a batch unconditionally, with no check that consecutive folios were actually manuscript-adjacent — so skipping a folio (e.g. running 002v then 003v without 003r) silently carried 002v's leftover continuation words into 003v.text-service/main.pynow imports mothra-text's_are_contiguousand resetsprev_folio_state(and suppressesinfer_continuation) whenever two folios in a batch aren't truly contiguous, mirroringrun_chain.py's existing CLI-side guard.The
mothra-textsubmodule is bumped to pick up that fix plus a second, related one found during the same testing pass (an explicit-but-emptyprev_folio_statewas falling through to a stale CSV-guess instead of being trusted) — both already merged upstream (mothra-text#43, mothra-text#44).Verification
tsc -b/vite buildclean.Summary by CodeRabbit
New Features
Bug Fixes