Skip to content

Consolidate duplicated path/PDF helpers into shared leaf modules (#654) - #670

Merged
marcvergees merged 1 commit into
fireform-core:development-approach-bfrom
abhishek-8081:issue-654-consolidate-path-helpers
Aug 15, 2026
Merged

Consolidate duplicated path/PDF helpers into shared leaf modules (#654)#670
marcvergees merged 1 commit into
fireform-core:development-approach-bfrom
abhishek-8081:issue-654-consolidate-path-helpers

Conversation

@abhishek-8081

Copy link
Copy Markdown
Collaborator

Closes #654.

Consolidates the helpers that got duplicated across files during the #640/#642 refactors
into two shared leaf modules:

  • app/core/paths.py — PROJECT_ROOT, _resolve_project_file, _resolve_target_directory.
  • app/core/pdf_utils.py — _pdf_text, _humanize, _count_pdf_widgets, _extract_pdf_fields,
    _FIELD_TYPE_BY_FT.

Both are dependency-free leaves (import only pathlib/pdfrw/fastapi, nothing from
app.services/app.api), which is what dissolves the circular import (template → controller →
file_manipulator → filler → template) that forced the duplication in the first place.
forms.py, form.py, template.py, filler.py, and purge.py now import from these instead of
holding their own copies.

Three things worth flagging (beyond a pure move):

  • The three _resolve_project_file copies weren't identical — forms.py/form.py raised
    AppError, template.py raised HTTPException. The exception is swallowed at every observable
    call site, so I standardized on HTTPException (the only choice provably behavior-identical
    everywhere, including the one path where the type could theoretically surface).
  • _count_pdf_widgets depends on _extract_pdf_fields, which needs PROJECT_ROOT — so those
    came along too (pdf_utils reaches PROJECT_ROOT via app.core.paths, an app.core→app.core
    ref, not reopening the cycle).
  • purge.py had its own PROJECT_ROOT copy (a fourth site) — consolidated too;
    _safe_delete_file (different logic) left untouched.

Behavior-preserving — 155 tests pass with only monkeypatch-location changes (the 6
PROJECT_ROOT patches now target app.core.paths.PROJECT_ROOT via module-attribute access, so
one patch covers all callers). ruff check app/ clean. Each helper is now defined in exactly
one place.

@marcvergees
marcvergees merged commit 491aa49 into fireform-core:development-approach-b Aug 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants