Skip to content

refactor(backend): remove dead modules and the 14 vulture hits (#2493) - #2500

Merged
vybe merged 2 commits into
devfrom
refactor/2493-backend-dead-code
Sep 3, 2026
Merged

refactor(backend): remove dead modules and the 14 vulture hits (#2493)#2500
vybe merged 2 commits into
devfrom
refactor/2493-backend-dead-code

Conversation

@dolho

@dolho dolho commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #2493 — the four pieces of backend dead weight removed, with vulture --min-confidence 80 now reporting zero findings and no suppressions added. Two of the fourteen vulture hits turned out not to be dead; per the issue's own rule ("a behaviour change anywhere means the item was not dead"), those were fixed by making the liveness visible to vulture rather than by deleting live code.

The four items

  1. utils/errors.py — deleted. grep -rn "utils.errors\|SafeHTTPException\|log_and_raise" src/ tests/ is empty (AC 1).
  2. The two OSS-move schema shims — deleted. They had no importer in src/, but seven test call sites across six files used them, which is the AC 2 carve-out: the tests now call one shared applier, tests/conftest.py::ensure_schema_tables(*names), over the canonical db/schema.py DDL (Invariant Feature/vector log retention #3 — index statements matched by table name, preserving the shims' own drift rationale). ent#443's test_the_bootstrap_helper_owns_no_ddl repoints at the conftest helper, so the no-second-copy-of-DDL guard survives the shim it was written about. Placement note: the helper lives in tests/conftest.py, not tests/unit/conftest.pyfrom conftest import … resolves to the parent (the stubbed_modules precedent), which a first attempt found the loud way.
  3. scripts/poc/ — deleted outright (the issue offered archiving the Nevermined demo; git history is the archive, and nothing references the directory).
  4. The 14 vulture hits — resolved as:
    • 5 genuinely unused imports dropped: pop_frames (twilio_media_stream — the live copy is in voip_audio), get_portal_identity (router.py referenced it only in docstrings), SlackOAuthInitResponse, PageBreak, and event_bus's RedisResponseError try/except pair (bound, never read).
    • exc_type/tb ×3 are protocol parameters, not the sys.exc_info() unpacking the issue's technical note guessed — they sit in __exit__/__aexit__ signatures and cannot be dropped. Renamed to _-prefixed (idiomatic unused-argument marking, not a linter directive).
    • Coroutine was NOT dead — read by the string annotation "Coroutine[Any, Any, None]" on _spawn_bg, which vulture cannot see. Unquoted the annotation (evaluates fine at runtime; collections.abc generics are subscriptable).
    • NON_ROW_RETENTION_OPS_KEYS's re-export was NOT dead — two tests imported it from settings_service. Repointed them at config (the true home; the sibling re-exports stay, they have production readers) and dropped the one line.
    • old_subscription_id — a dead parameter of _perform_auto_switch; removed from the signature, its one production call site, and six test call sites (all keyword, so the removal is mechanical and total — zero residual grep hits).

Verification

  • vulture src/backend --min-confidence 80 --exclude '*/venv/*,*/__pycache__/*,*/tests/*,*/enterprise/*,*/migrations/versions/*'zero findings, exit 0 (AC 4).
  • Residual greps for every deleted symbol/module: empty (AC 1/2).
  • 497 tests across every touched suite pass locally (portal exposure ×3-site rewrite, logout block, shared sessions, room budget defaults, delegated identity, rooms-oss-core, all three auto-switch suites, retention edges, headroom history, both VoIP suites, telemetry sharing); full tests/unit run in flight locally and authoritative in CI (AC 5).
  • Deletions + test rewiring only; no production behaviour change anywhere (the two "not dead" items are annotation/import-shape changes with identical runtime semantics).

🤖 Generated with Claude Code

https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

dolho and others added 2 commits September 2, 2026 17:06
…-move schema shims, scripts/poc, and 14 vulture hits (#2493)

Every item re-verified before removal; two of the fourteen vulture hits
turned out NOT to be dead and were fixed by making the liveness visible
instead of deleting it:

- utils/errors.py (126 lines, zero importers for eight months) deleted;
  grep for utils.errors/SafeHTTPException/log_and_raise is empty.
- client_portal/schema.py + shared_sessions/schema.py deleted. Their
  callers were seven TEST sites — replaced by ONE test-side applier,
  tests/conftest.py::ensure_schema_tables(*names), over the canonical
  db/schema.py DDL (Invariant #3; index statements matched by table
  name, the shims' own drift rationale). ent#443's helper-owns-no-DDL
  guard repoints at the conftest helper so the drift rule survives the
  shim it was written about. (Placed in tests/conftest.py, not
  tests/unit/conftest.py — 'from conftest import' resolves to the
  parent, the stubbed_modules precedent.)
- scripts/poc/ (3 files) deleted outright; git history is the archive.
- vulture ≥80% now reports ZERO with no suppressions:
  * 5 genuinely unused imports dropped (pop_frames, get_portal_identity,
    SlackOAuthInitResponse, PageBreak, and event_bus's RedisResponseError
    try/except pair — the name was never read after binding).
  * exc_type/tb in three __exit__/__aexit__ signatures are PROTOCOL
    parameters, not sys.exc_info() unpacking as the issue guessed —
    renamed to _-prefixed (idiomatic unused-arg, not a linter directive).
  * Coroutine in task_execution_service was NOT dead: it is read by a
    string annotation vulture cannot see. Unquoted the annotation.
  * settings_service's NON_ROW_RETENTION_OPS_KEYS re-export was NOT
    dead either — two tests imported it there. Repointed them at
    config (the true home) and dropped the re-export.
  * _perform_auto_switch's old_subscription_id parameter removed with
    its one production call site and six test call sites (all keyword).

497 tests across every touched suite pass; full unit run in CI.

Closes #2493

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux
The residual grep that cleared the shim removal was piped through
head -3 and truncated past the conftest lines — the exhaustive rerun
found tests/unit/test_ent220_chain_shield_and_residuals.py (14 tests,
all erroring at collection in CI's regression diff). Rewired to the
shared conftest applier like its six sibling files.

Related to #2493

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NLfHNPtB5UCMk4LonZiJux

@obasilakis obasilakis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/validate-pr: APPROVE

Deletions map 1:1 to the issue's five ACs, and the two "not dead" items were correctly reclassified rather than deleted.

Dead-code verification — the failure mode that actually matters here — is clean. Greps run against the PR head ref:

  • utils.errors / SafeHTTPException / log_and_raise / safe_error_message / init_client_portal_schema / init_shared_sessions_schema / client_portal.schema / shared_sessions.schema / PageBreak / RedisResponseError — 0 hits repo-wide.
  • Enterprise submodule checked too (public CI cannot see it): its references are to enterprise.backend.{client_portal,shared_sessions}.schema, which are distinct files under src/backend/enterprise/backend/…/schema.py and untouched. No cross-repo break.
  • pop_frames — the live copy stays in voip_audio.py:39; twilio_media_stream.py has 0 call sites, so that import is genuinely unused.
  • old_subscription_id — removed from the signature plus 1 prod call site and 6 test sites, 0 residual hits.
  • Coroutine unquoting is runtime-safe: imported at module scope from collections.abc (task_execution_service.py:23), subscriptable on 3.13.

I also chased the one subtle risk: the new conftest.ensure_schema_tables narrows the old shim's index matcher from "enterprise_portal" in stmt to table(. Safe — every live INDEX statement in db/schema.py:1893-1910 uses the no-space table( form, so no index is silently dropped, and the ent#443 no-DDL guard plus a new idempotency test are repointed at the helper.

Warnings

  • pytest (base, seed 99999) is red from a 45-minute job timeout on the BASE (dev) side, not a test failure and not head. All three head seeds are green (8–10 min) and regression diff passes. Needs a re-run, not a code change — but it keeps the merge gate blocked.
  • docs/security/OWASP_COMPLIANCE_REPORT.md:243,264,294 still names the deleted utils/errors.py as the A10 remediation, marked "DONE". The control was never actually wired (error_handlers.py owns the 422/500 shape), so the report was already inaccurate — this deletion just exposes it. One line noting the live owner would close it.

Suggestions

  • get_portal_identity (client_portal/portal_auth.py:156) now has zero call sites in OSS and in the enterprise submodule — only docstrings and docs reference it. Vulture's <80% confidence threshold missed it; candidate for the next sweep, not this PR.
  • tests/unit/test_1073_voip_media_stream_ticket.py:127 patches audio_mod.pop_frames for a path twilio_media_stream never calls — leftover scaffolding.

Findings produced by /validate-pr (Claude Code).

@vybe
vybe merged commit 0e8b750 into dev Sep 3, 2026
23 of 24 checks passed
vybe pushed a commit that referenced this pull request Sep 3, 2026
Brings the branch onto dev at 36e6e1a: #2509 (main reconcile), #2479 (zod 4),
#2513 (pull: scheduled work reaches the durable queue), plus #2500/#2480. The
only shared file with #2513 is task_execution_service.py; git merged the import
block cleanly and the four scrub blocks in _write_terminal_and_gate /
apply_result are untouched (#2513 adds build_pull_queue_payload above them and
widens _admission_gate, which the ent#279 allowlist already names).
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.

3 participants