Skip to content

refactor: separate document processing from import workflow - #1063

Open
mikec-ai wants to merge 2 commits into
mainfrom
codex/document-processing-1058
Open

mikec-ai wants to merge 2 commits into
mainfrom
codex/document-processing-1058

Conversation

@mikec-ai

@mikec-ai mikec-ai commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Closes #1058. A caller can now reuse the ordered content-processing sequence with HTML text, without constructing a request, upload or document record. This is a small internal separation, not a package, service or framework migration.

Changes

  • Move character/link replacement, HTML parsing, existing NOFO normalization and section parsing into process_document_content.
  • Reuse it through the existing base import view for Builder, Composer and Compare.
  • Keep conversion, warning accounting, metadata inference, consumer hooks, error presentation, permissions, persistence and reimport workflow in their existing owners. Preserve order, result identity and exception boundaries.
  • Add direct synthetic contract tests without requests, uploads, users or database records. Retain characterization assertions, changing only moved patch points.
  • Document the boundary and remaining coupling: Django initialization, existing validation exceptions/settings, mutable parser objects and DEBUG snapshot output. Word conversion remains a separate source adapter; this does not establish a public interchange schema or promise compatibility with every editor.

Verification

  • Current merged main baseline: 895 targeted regression tests passed.
  • Refactored branch: 899 targeted tests passed, covering import/reimport/transforms/Markdown, all four fidelity fixes, Composer and Compare.
  • Independent review: no actionable findings; independently ran 584 focused tests and the 21 direct/characterization checks.
  • Local Chrome smoke check through the existing import form, using synthetic HTML and isolated SQLite with DEBUG off. Verified both step headings, Summary, applicant prose, captioned table with zero value, ordered list and working internal link.
  • Pre-commit checks and full CI passed, including Python/JavaScript tests, container build and all three security scans.

No deployment, feature-flag, endpoint, authorization or content-rule changes. The browser check exercises shared content processing, not a new Word conversion or PDF export path.

Browser evidence

Synthetic import with preserved headings, captioned table and list

Smoke check scope and boundary documentation.

@mikec-ai
mikec-ai marked this pull request as ready for review October 8, 2026 03:46
@mikec-ai
mikec-ai requested a review from benbrasso October 8, 2026 03:46

@benbrasso benbrasso left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the shared processing extraction and documentation. No actionable findings; the existing processing order, consumer hooks, mutable results, and error boundaries are preserved. Independently ran all 21 direct-processing and characterization tests against commit 5567133; all passed. The only subsequent change is the documentation cross-link in b6538bd. Approved; merge once the new-head CI checks are green.

@benbrasso

Copy link
Copy Markdown
Collaborator

Mike (@mikec-ai), reviewed this for behavior preservation, UX impact, performance implications, and repository documentation. Everything checked out; no actionable findings.

  • Builder, Composer, and Compare retain the processing sequence, consumer hooks, metadata handoff, warning accounting, and error boundaries. No intended user-facing change.
  • The same parsing and normalization passes run, without new document copies or database queries. No material performance concern from the code review; we did not run a large-document benchmark.
  • Independently ran the 21 direct-processing and characterization tests on 5567133: all passed, including persistence through all three consumers. The original-head unit tests, container build, security scans, and title check were green.
  • Documentation already covers the shared boundary and remaining Django/parser coupling and DEBUG snapshot side effect. The contribution Mermaid diagram remains accurate.

Added one tiny docs-only commit, b6538bd: linked DOCUMENT_PROCESSING.md from the contribution workflow’s “Changing import behavior” reading list. Verified the link target and whitespace check; no runtime changes.

Approved and good to go. You can merge whenever you’re ready, once CI for the new docs commit is green.

@benbrasso
benbrasso force-pushed the codex/document-processing-1058 branch 2 times, most recently from b6538bd to b4b2d6b Compare October 8, 2026 16:42
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.

Separate document processing from application workflow for reuse

2 participants