refactor: streamline, deduplicate, and optimize CiteForge end-to-end#7
Merged
Conversation
Behavior-preserving pass executed per territory: - merge core: guard-table dispatch for per-field override rules, single-source pages validation, extract duplicate-scan match ladder, hoist 9 hot regexes, fix hash-order leak in debug log, drop dead dedup subterm - pipeline: collapse nine Phase 2/2.5 search blocks into _phase2_search, sorted fsscan iteration for year-window cleanup and post-run fixup, data-driven optional-key loading, remove dead logger monkey-patching, correct main.py docstring (positional CSV path never implemented) - text/bibtex: precompile ~40 regexes, hoist serializer closures, memoize name_signature, merge duplicate preprint predicate and similarity cores - clients: single-source title|author cache-key format, dedupe fetch and negative-cache scaffolding, shared PubMed esearch/esummary core, delete dead API config objects, merge duplicated Gemini except blocks - io: hoist lazy imports (import graph verified acyclic), unify JSON writes Verified: ruff clean, mypy clean, 791 tests pass; merge core differential- fuzzed vs HEAD (28k cases); text layer golden-hashed over 3,671 live .bib files; live cache-hit pipeline run byte-identity check in flight.
- iter_parsed_author_bibs in fsscan: single sorted scan+parse core consumed by the Phase 4 candidate-DOI dedup loop and save_entry_to_file; each site keeps its own skip convention, error envelope, and log lines - venue.first_non_generic_container: provably identical selection core shared by bibtex_from_csl and _extract_venue (full consolidation rejected on differential evidence: divergent debug logs and a None-event crash case) - preprint predicate unification declined with divergence witness (DATA_DOI_PREFIXES membership); canonical-predicate comments added instead 25 new differential/pin tests (tests/test_fsscan.py, tests/test_venue.py) prove equivalence against verbatim replicas of the legacy loops. Gates: ruff clean, mypy clean, 816 tests pass.
There was a problem hiding this comment.
Pull request overview
This PR performs a repo-wide refactor to deduplicate repeated logic and make determinism structural across the end-to-end enrichment pipeline, while also tightening documentation and expanding differential tests that pin behavior against legacy inline implementations.
Changes:
- Consolidates repeated selection / scan / search scaffolding into shared helpers (e.g., venue container selection, directory scan+parse, Phase 2 search blocks, DOI-parse validation tail).
- Improves determinism and reduces per-call overhead by centralizing sorted filesystem iteration and hoisting frequently used regexes to module scope.
- Rewrites README/CLAUDE.md and compresses docstrings to a consistent “sober/factual” style, plus adds new differential tests to protect behavior-preservation claims.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_venue.py | Adds differential + behavior-pin tests for the new shared venue container selection helper. |
| tests/test_textnorm.py | Minor comment formatting update to match documentation style conventions. |
| tests/test_fsscan.py | Adds unit + differential tests for deterministic scan helpers and shared scan+parse behavior. |
| README.md | Updates positioning and usage docs; clarifies key requirements and removes/adjusts stale claims. |
| main.py | Deduplicates optional-key loading/logging via a shared helper. |
| CLAUDE.md | Updates contributor guidance, quality gates wording, and documented conventions. |
| citeforge/venue.py | Introduces first_non_generic_container helper for shared container-title selection logic. |
| citeforge/textnorm.py | Docstring/comment wording adjustments (no functional changes visible in diff). |
| citeforge/text_utils.py | Hoists regexes, memoizes name_signature for string inputs, and deduplicates shared similarity/author parsing logic. |
| citeforge/pipeline/scheduler.py | Deduplicates author output-dir naming and clarifies scheduling behavior via helper. |
| citeforge/pipeline/postrun.py | Uses deterministic scan helpers and unifies JSON writes via safe_write_json. |
| citeforge/pipeline/article.py | Collapses repeated Phase 2 search blocks into _phase2_search; centralizes skip-summary and “title is venue” checks; uses shared scan+parse for Phase 4 DOI dedup. |
| citeforge/models.py | Docstring compression only. |
| citeforge/merge_utils.py | Extracts field-override rules into guard table, single-sources pages validation, and extracts duplicate-entry check into helper; uses shared scan+parse. |
| citeforge/log_utils.py | Removes dead custom-method injection; keeps logging API on the wrapper. |
| citeforge/io_utils.py | Hoists previously-lazy imports after confirming import graph; adopts deterministic scan helpers in multiple places. |
| citeforge/id_utils.py | Docstring compression for public DOI normalize alias. |
| citeforge/http_utils.py | Docstring/comment wording adjustments for clarity. |
| citeforge/fsscan.py | Adds shared deterministic scan+parse iterator used by multiple duplicate-scan sites. |
| citeforge/doi_utils.py | Deduplicates CSL/BibTeX validation tail into shared parse+match helper. |
| citeforge/config.py | Docstring/comment wording updates; removes unused constant in shown diff; clarifies constraints. |
| citeforge/clients/utility_apis.py | Consolidates DOI cache lookup via shared helper; tweaks Gemini retry/error handling structure. |
| citeforge/clients/serply_scholar.py | Docstring wording adjustments. |
| citeforge/clients/serpapi_scholar.py | Docstring wording adjustments. |
| citeforge/clients/search_apis.py | Centralizes cache key format + negative-cache list retrieval; factors PubMed/EuropePMC query scaffolds; shares venue-scored search scaffold; uses shared venue selection helper. |
| citeforge/clients/helpers.py | Adds single-source title_author_cache_key and DOI cache lookup helper; hoists imports. |
| citeforge/canonicalize.py | Deduplicates preprint-journal demotion logic into shared helper; narrows/normalizes server-substring checks. |
| citeforge/cache.py | Comment/doc clarifications around negative caching semantics. |
| citeforge/bibtex_utils.py | Hoists regexes/serializer cleanup tables; removes duplicate preprint predicate by reusing text_utils._is_preprint_fields. |
| citeforge/bibtex_build.py | Hoists imports and regex; docstring compression and minor dedupe. |
| citeforge/api_generics.py | Centralizes cache-key construction and venue-array selection; factors out raw-result fetch helper. |
| citeforge/api_configs.py | Removes dead PubMed/EuropePMC/DataCite mapping/config objects now documented as staying hand-rolled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
Repo-wide streamline, dedupe, and optimization pass, plus a sober rewrite of all documentation. Behavior-preserving by construction; byte-identity verified four independent ways (below). Executed as five parallel territory refactors (merge core, pipeline orchestration, text/bibtex utils, API clients, docs) followed by cross-territory consolidation.
Code changes
Merge/decision core (
merge_utils.py,canonicalize.py)_FIELD_GUARDS); the merge loop body went from ~180 nested lines to an 11-line dispatch. Pick order, log messages, and rank fall-through unchanged._invalid_pages_reason); the 215-line duplicate-scan body ofsave_entry_to_fileextracted to_is_duplicate_bib_entry.sorted().Pipeline (
pipeline/,main.py,io_utils.py,log_utils.py)_phase2_search(); source order and every log string preserved.fsscanhelpers (one real determinism violation fixed: raw unsortedos.listdir).main.pymade data-driven;main.pydocstring corrected (positional CSV path was documented but never implemented).baseline.json/badges.jsonwrites unified onsafe_write_json;io_utilslazy imports hoisted after verifying the import graph is acyclic.Text/bibtex utils (
text_utils.py,bibtex_utils.py,bibtex_build.py)_is_preprint_entry(line-for-line copy of_is_preprint_fields), the shared core oftitle_similarity/venue_similarity, the given/family join inextract_authors_from_any.name_signaturememoized withlru_cache(the O(n^2) inner loop of author-overlap scoring); returns a fresh copy per call so aliasing is unchanged.text_utilsvstextnormjudged a correct layer split (lossy comparison-normalization vs case-preserving display repair), not duplication.API clients (
clients/,api_generics.py,api_configs.py,http_utils.py,doi_utils.py)norm-title|authorcache-key format single-sourced intitle_author_cache_key()(was hand-built in 7 places); DOI-keyed cache preamble shared via_doi_cache_lookup().PUBMED_SEARCH_CONFIG,PUBMED_FIELD_MAPPING,EUROPEPMC_FIELD_MAPPING,DATACITE_FIELD_MAPPING); they could not reproduce the hand-rolled builders' output, so wiring them in was ruled out after per-API analysis.Documentation
README and CLAUDE.md rewritten sober: marketing tone removed, stale claims corrected (positional CSV arg never existed; Serply is recommended, not required, per actual key handling), invariants preserved verbatim (Three-Way Fix Pattern, determinism, config-driven convention). House style enforced (no em-dash prose, no colon-led prose, no bold pseudo-headings). Docstrings across the package compressed to factual form; every constraint-bearing comment kept.
Verification
ruffclean,mypyclean (33 files), full suite 816/816 (25 new differential tests).merge_with_policycases, 600save_entry_to_filecases with on-disk snapshots, 24,000 canonicalize stage-cases; zero mismatches..bibfiles (parse, serialize, citekeys, filenames, similarity, dedup scoring); identical digest before/after.mainwere each run against the same warmed API cache from the same starting output tree (main's run in an isolated worktree). The two output trees are byte-identical,diff -rexit 0 over every.bib,baseline.json,summary.csv, and the a2i2 folder. Deltas observed in intermediate runs were traced to cache convergence (entries a run writes after its own decisions) and reproduce identically under both code versions.External review (Codex, top reasoning tier) found no blockers; its one flag (PubMed
[:retmax]slice) was verified a no-op because NCBI honoursretmaxserver-side.Left alone deliberately
PUBLISHER_CORRECTIONSfirst-match vs last-match divergence between merge and canonicalize sites); each now carries a comment.