Skip to content

Audit and remove unused dependencies across base/local/production - #1636

Merged
JSv4 merged 3 commits into
mainfrom
claude/audit-backend-dependencies-8tJm3
May 13, 2026
Merged

Audit and remove unused dependencies across base/local/production#1636
JSv4 merged 3 commits into
mainfrom
claude/audit-backend-dependencies-8tJm3

Conversation

@JSv4

@JSv4 JSv4 commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Comprehensive dependency audit removing unused and vestigial packages from requirements/base.txt, requirements/local.txt, and requirements/production.txt. Grep-audited all direct dependencies against codebase references in opencontractserver/, config/, scripts, compose, Dockerfiles, settings, and CI workflows.

Key Changes

Removed from base.txt (~10 direct deps):

  • python-slugify — zero verifiable code references
  • pandas — zero verifiable code references
  • django-model-utils — zero verifiable code references
  • tiktoken — zero verifiable code references
  • jsonschema — zero verifiable code references
  • django-extensions — moved to local.txt (only used in config/settings/local.py)
  • pytesseract — moved to local.txt (only used by test-only test_pdf_redaction.py; production OCR runs in docling-parser container)

Removed from local.txt (~5 direct deps):

  • Werkzeug[watchdog] — zero verifiable code references
  • ipdb — zero verifiable code references
  • pytest-sugar — zero verifiable code references
  • pylint-django — zero verifiable code references
  • pylint-celery — zero verifiable code references

Removed from production.txt:

  • django-anymail[mailgun] — settings block was fully commented out

Deleted orphan requirement files:

  • requirements/filetypes/docx.txt (contained only mammoth==1.11.0 for inactive docx pipeline)
  • requirements/ingestors/nlm_ingest.txt (body was literally # pass)
  • requirements/processors/gliner.txt (body was only commented placeholders)
  • .pylintrc (pylint is not invoked by pre-commit, CI, or any script)

Updated compose/production/django/Dockerfile:

  • Removed tesseract-ocr and tesseract-ocr-eng apt installs (pytesseract moved to test-only, production OCR runs in docling-parser container)

Updated .pre-commit-config.yaml:

  • Removed dropped packages from mypy hook's additional_dependencies list to keep in lockstep with requirements/

Implementation Details

  • pytesseract moved to local.txt with explanatory comment: "Test-only — OCR pipeline used by tests/test_pdf_redaction.py to verify the docling-parser microservice's OCR behavior. Production OCR runs in the docling-parser container, so this is not needed in base.txt."
  • django-extensions deduplicated (was redundantly listed in both base.txt and local.txt; kept only in local.txt since it is only added to INSTALLED_APPS in config/settings/local.py)
  • Snyk pins duplicated across all three requirements files intentionally left in place per existing comment in production.txt

Benefits

  • Smaller production image (no Rust-built tiktoken wheel, no pandas, no tesseract apt packages)
  • Cleaner dependency tree with only actively-used packages
  • Reduced attack surface and maintenance burden
  • Improved clarity of actual project dependencies

https://claude.ai/code/session_01KTbftXBiTyrn8Xhi9FrfF4

Grep-audited every direct backend dependency across opencontractserver/,
config/, scripts, compose, Dockerfiles, settings, and CI workflows.

Dropped from requirements/base.txt (0 verifiable refs in code):
  python-slugify, django-model-utils, pandas, tiktoken, jsonschema
  (pandas, tiktoken, jsonschema remain installed transitively via
  plasmapdf, pydantic-ai-slim[openai], and mcp respectively.)

Dropped from requirements/local.txt (0 refs, none in pre-commit/CI):
  Werkzeug[watchdog], ipdb, pytest-sugar, pylint-django, pylint-celery
  (pylint is never invoked by pre-commit, scripts, or CI — .pylintrc
  was the only consumer.)

Dropped from requirements/production.txt:
  django-anymail[mailgun] — settings block in production.py is fully
  commented out.

Deduped:
  django-extensions==4.1 was redundantly listed in both base.txt and
  local.txt; kept only in local.txt (the only place it is added to
  INSTALLED_APPS).

Moved to local.txt as test-only:
  pytesseract — only used by tests/test_pdf_redaction.py to mirror
  the docling-parser microservice OCR; production OCR runs in the
  docling-parser container. Also dropped tesseract-ocr/tesseract-ocr-eng
  apt installs from compose/production/django/Dockerfile.

Deleted orphan requirement files:
  requirements/filetypes/docx.txt   (mammoth, docx pipeline not active)
  requirements/ingestors/nlm_ingest.txt  (file body was '# pass')
  requirements/processors/gliner.txt  (only commented placeholders)
  .pylintrc  (pylint not run anywhere)

Updated .pre-commit-config.yaml mypy hook additional_dependencies to
match the trimmed requirements (removed django-model-utils, pandas,
tiktoken, jsonschema, python-slugify). Snyk pins duplicated across all
three requirements files are intentionally left in place per the
existing comment in production.txt (Snyk does not understand -r
inheritance).

Validated via pip install --dry-run that the resolved install set is
unchanged for any directly imported package; pandas/tiktoken/jsonschema
remain present via transitive deps.

https://claude.ai/code/session_01KTbftXBiTyrn8Xhi9FrfF4
@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Code Review

Overview

This PR performs a targeted dependency audit, removing ~16 direct packages from base.txt/local.txt/production.txt that have zero verifiable usage in the codebase. All deletions are backed by grep evidence. The overall approach is sound and well-executed.


What's done correctly

  • Dependency audit methodology is solid: grep-checking opencontractserver/, config/, settings, Dockerfiles, compose files, and CI workflows is the right scope.
  • pytesseract split is correct: Moved from base.txtlocal.txt, and the test environment (test.yml) builds from compose/local/django/Dockerfile, which retains the tesseract-ocr + tesseract-ocr-eng system packages (lines 85–86). Tests will continue to pass unmodified.
  • tesseract-ocr system packages removed only from production Dockerfile — not the local one. This is exactly right.
  • django-extensions deduplication is correct: only referenced in config/settings/local.py; moving exclusively to local.txt is the correct call.
  • python-slugify removal is safe: codebase uses Django's built-in from django.utils.text import slugify throughout (verified: opencontractserver/agents/models.py, agents/migrations/0005_generate_missing_slugs.py).
  • jsonschema removal is safe: the only reference in timeline_schema.py is inside a docstring comment — no actual import jsonschema exists anywhere in the codebase.
  • django-anymail removal is safe: all references in config/settings/production.py are fully commented out.
  • Pre-commit mypy hook kept in sync: removing pandas, tiktoken, jsonschema, python-slugify, and django-model-utils from additional_dependencies is necessary and correctly done.
  • Orphan files deleted cleanly: nlm_ingest.txt (body was # pass), gliner.txt (all commented), filetypes/docx.txt (no active imports of mammoth), .pylintrc (pylint not wired to pre-commit or CI) — all justified.

Issues and Suggestions

1. Minor: pytesseract test guard (non-blocking, pre-existing)

test_pdf_redaction.py lazily imports pytesseract inside test methods without a @skipUnless guard:

# line 370
import pytesseract

If someone runs tests using base.txt only (not the Docker test environment), these will ImportError rather than skip gracefully. This is a pre-existing pattern, not a regression introduced here, but now that pytesseract is explicitly marked test-only it's a good opportunity to add a guard:

import shutil
import unittest

@unittest.skipUnless(shutil.which("tesseract"), "tesseract binary not available")
class TestPDFRedaction(TestCase):
    ...

Not blocking since the standard test invocation (docker compose -f test.yml run django pytest) uses the local Dockerfile which has tesseract, but worth the extra robustness.

2. Minor: django-anymail removal could surprise future operators

The anymail block in production.py is fully commented out, so the removal is technically safe. However, if an operator wants to enable transactional email (Mailgun), they'll need to re-add the package without any breadcrumb that it was intentionally removed. Consider adding a one-line comment to production.txt in the email section or the commented-out production.py block:

# To enable Mailgun email: add django-anymail[mailgun] to requirements/production.txt
# INSTALLED_APPS += ["anymail"]

This is entirely optional — just improves discoverability.

3. Trivial: CHANGELOG entry formatting

The single mega-paragraph changelog entry would be easier to scan as bullet points, consistent with how other entries in the same file are formatted. The content is excellent; just the presentation is denser than the surrounding entries.


No Issues Found With

  • pandas, tiktoken, django-model-utils, Werkzeug, ipdb, pytest-sugar, pylint-django, pylint-celery — all confirmed zero references in non-requirement files.
  • mammoth — no imports anywhere; docx pipeline is genuinely inactive.
  • Snyk pins preserved across all three files per existing policy — correct.

Summary

This is a clean, well-researched maintenance PR. All removals are verified against the actual codebase. The pytesseract/tesseract-ocr split between local and production environments is the most nuanced part and it's handled correctly. The two suggestions above are both minor quality-of-life improvements, not blockers. Approve with confidence.

JSv4 added 2 commits May 12, 2026 22:46
…, bullet CHANGELOG

- Add @skipUnless guards to test_pdf_redactor_with_ocr_verification and
  test_pdf_redactor_with_specific_tokens so they degrade cleanly when
  pytesseract or the tesseract binary is absent (now that pytesseract is
  test-only). The Docker test image still installs both, so CI behaviour
  is unchanged.
- Drop a breadcrumb comment in config/settings/production.py pointing
  future operators at re-adding django-anymail[mailgun] to
  requirements/production.txt before uncommenting the Mailgun block.
- Reformat the dependency-audit CHANGELOG entry from one mega-paragraph
  into nested bullets to match the surrounding entry style.
import os
import random
import shutil
import unittest
@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Code Review: Dependency Audit and Cleanup

Overview

This is a well-scoped dependency audit removing unused packages across requirements/base.txt, requirements/local.txt, and requirements/production.txt, plus cleaning up orphan files. The motivation is sound, the execution is methodical, and the added comments/guards are solid. A few items worth verifying before merge:


Items to Verify

1. django-model-utils -- confirm no model inheritance

django-model-utils provides TimeStampedModel, StatusField, Choices, and FieldTracker. These show up quietly in model base classes. Worth running before merge:

grep -r 'TimeStampedModel\|StatusModel\|FieldTracker\|model_utils' opencontractserver/ config/ --include='*.py' -l

If any results appear, the removal will cause an ImportError at startup -- not caught by a grep on the package name alone.

2. python-slugify -- confirm Django's built-in slugify is sufficient

Django ships django.utils.text.slugify, which handles most cases. If anything in the codebase used python-slugify for extended Unicode handling (e.g., transliterating Arabic/Chinese characters), that would silently break since Django's version drops non-ASCII by default rather than transliterating. The import form to grep for is from slugify import slugify or import slugify.

3. Werkzeug removal may affect runserver_plus

django-extensions (kept in local.txt) includes runserver_plus, which requires Werkzeug as a runtime dep. Removing Werkzeug from local.txt means manage.py runserver_plus will fail with ModuleNotFoundError for anyone using it locally. If nobody on the team uses runserver_plus this is fine -- but worth a quick confirmation, or adding a comment noting Werkzeug must be reinstalled manually.

4. Confirm tesseract is still present in compose/local/django/Dockerfile

The PR description says the system binary is "installed in compose/local/django/Dockerfile and used by test.yml". The diff only shows the removal from the production Dockerfile. Before merge, verify the local Dockerfile still installs tesseract-ocr -- otherwise _OCR_AVAILABLE will correctly skip the OCR tests but also silently reduce CI test coverage.

5. django-anymail backwards compatibility

Operators who have already deployed and uncommented the Anymail block in their config/settings/production.py will get an ImportError on next deploy. The added comment in production.py is the right approach; a brief upgrade note warning active Anymail users would also help.


Positive Notes

  • The @skipUnless pattern in test_pdf_redaction.py is exactly right: importlib.util.find_spec() for the package check (no import side-effects) and shutil.which() for the binary check, combined at module level so test discovery handles it cleanly. This degrades gracefully without a try/except import hack.

  • Orphan file deletions (requirements/ingestors/nlm_ingest.txt whose body was literally # pass, requirements/processors/gliner.txt with only commented-out placeholders) are unambiguously correct.

  • The re-enable comment added in production.py for anymail is excellent operator UX -- it tells you exactly what package line to add and where, rather than leaving a confusing commented-out block with no guidance.

  • Keeping mypy additional_dependencies in lockstep with requirements/ is important and often missed in audits like this -- good catch.

  • The CHANGELOG entry is detailed and accurate; the per-file breakdown will be useful for anyone doing a rollback or audit.


Minor Nits

  • The pytesseract entry in local.txt is unpinned (unlike most other entries in that file). Consider pinning it (e.g., pytesseract==0.3.13) for reproducibility -- consistent with the pinning discipline applied to everything else.

  • The comment block added in local.txt for pytesseract places the horizontal rule separator after the comment, inconsistent with the rest of the file (other sections put the rule before the entries). Minor formatting nit.


Summary

The audit approach is sound and the execution is clean. The main risk is false negatives from a grep-only audit on packages that surface through model inheritance or indirect imports -- django-model-utils is the one I would double-check most carefully. Everything else looks correct and well-justified. Addressing the verification items above will make this merge-ready.

@codecov

codecov Bot commented May 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@JSv4
JSv4 merged commit 070c2f2 into main May 13, 2026
18 checks passed
@JSv4
JSv4 deleted the claude/audit-backend-dependencies-8tJm3 branch May 13, 2026 05:11
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