Flag studies the project already has when adding, and fix the matching edge cases - #841
Merged
Merged
Conversation
…g edge cases Closes #820. Staged studies are now matched against the studies the project already holds. A match is flagged, never acted on: the row names the study it duplicates and why, sits out of the upload by default, and carries an Add anyway toggle. This is deliberate - production has a project that builds 14 studies from 4 byte-identical PDFs, one per site, so auto-skipping would break a live workflow. Matching was measured against 90 production PDFs and rewritten: - matchEntries reports how two entries match (doi, file, title, title-substring) instead of returning a boolean - two DOIs settle it both ways, so different DOIs are different papers whatever the titles say - PDF byte size identifies a file, which matched R2 bytes 90/90 with no collisions, so no hashing and no schema change - isWeakTitle rejects the boilerplate extraction actually produces, so "untitled" and "RE S E AR C H RE P O R T" no longer merge unrelated papers - substring matching catches titles cut off where they wrapped, and studies named "Author Year- <title>" Merging: a file-only match no longer merges, so the multi-site workflow keeps one study per site; a second distinct PDF is attached through extraPdfs rather than dropped; merged rows report the fields their sources disagree on. Also fixed along the way: - retrying a failed extraction re-read the file through handlePdfSelect, which filtered it out as already staged and left the row spinning forever - DOIs broken across a line break resolved to nothing; they are rejoined before the regex runs (1 of 90 PDFs) - Crossref registers some titles as escaped JATS, so a study could be named "<p>MRI Changes After..."; entities are decoded before tags are stripped (1 of 43 resolvable DOIs) - the canonical title from the DOI lookup is used when the page gave nothing usable or only a truncated prefix, taking studies named from published metadata from 24 to 40 of 41 Verified against production: 90/90 re-uploads detected, 0 false flags, 0 unrelated same-title merges. Matching 50 staged against 300 existing takes 10ms, so no indexing. Claude-Session: https://claude.ai/code/session_01JySapHzgzwPW1yBWHccZYf
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (18)
✨ 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 |
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
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 #820.
What changed
Staged studies are now matched against the studies the project already holds. A match is flagged, never acted on: the row names the study it duplicates and why, sits out of the upload by default, and carries an
Add anywaytoggle.That restraint is deliberate. One production project deliberately builds 14 studies from 4 byte-identical PDFs, one study per site the report covers, so auto-skipping or auto-merging would have broken a live workflow.
Matching, rewritten against real data
I pulled 90 PDFs out of production projects and ran the real extractors over them offline before changing anything. The issue's premise turned out to be slightly off: the filename fallback it worried about essentially never fires, and the real hazard is junk titles, which 27% of PDFs produce.
matchEntriesreports how two entries match (doi,file,title,title-substring) instead of returning a booleanpdfs.sizematched R2 bytes 90/90 with zero collisions, so no hashing and no schema changeisWeakTitlerejects the boilerplate extraction actually produces, sountitledandRE S E AR C H RE P O R Tno longer merge unrelated papersAuthor Year- <title>Merging: a
file-only match no longer merges, so the multi-site workflow keeps one study per site. A second distinct PDF is attached viaextraPdfsrather than dropped. Merged rows report the fields their sources disagree on.Bugs fixed along the way
handlePdfSelect, which filtered it out as already staged and left the row spinning forever<p>MRI Changes After.... Entities are decoded before tags are stripped (1 of 43 resolvable DOIs)Verified against production
Re-importing all 23 PDFs of a real project flags 23 of 23 (19 by DOI, 4 by file) and adds nothing silently. Matching 50 staged rows against a 300-study project takes 10ms, so no indexing was added.
Testing
duplicates.test.tsis new and its fixtures are strings production actually produced: the junk titles extraction returns, and the published titles those papers carry. 681 web, 322 web-server and 127 workers tests pass; typecheck and oxlint clean.Notes for review
collectModeinuseAddStudiesandAddStudiesFormis now dead code, sinceCreateProjectModalno longer adds studies. That makes the issue's third open question moot. Flagged rather than deleted.StudyMetadata.authorsis typedstring[]but the lookup returns a formatted string andaddBatchcasts it back. Pre-existing; I left one localised cast with a comment rather than widening the change.pdfsrow in one production project pointing at a missing R2 key. Unrelated to this work.https://claude.ai/code/session_01JySapHzgzwPW1yBWHccZYf
Summary by CodeRabbit
New Features
Bug Fixes