Skip to content

refactor: remove dead leaf modules (v1 domain cleanup, part 1)#598

Merged
frankbria merged 1 commit into
mainfrom
refactor/remove-v1-domain-layer
May 27, 2026
Merged

refactor: remove dead leaf modules (v1 domain cleanup, part 1)#598
frankbria merged 1 commit into
mainfrom
refactor/remove-v1-domain-layer

Conversation

@frankbria
Copy link
Copy Markdown
Owner

@frankbria frankbria commented May 27, 2026

Summary

PR 3 of the v1 removal (after #596 serve fix, #597 web layer). Removes confirmed zero-importer dead modules + their tests (~11.6k lines). Every target was verified to have no live importer (reachability graph from cli/app.py + ui/server.py, plus exact-path grep).

Deleted

  • core/{credential_audit, credential_validator, phase_manager, project_status}.py
  • git/github_issue_sync.py, lib/{quality_gate_tool, sdk_hooks}.py
  • ui/auth.py, agents/subagent_generator.py
  • notifications/{router, desktop}.py — legacy desktop notifications; the live webhook notifications (Phase 5.3) are untouched
  • codeframe/tasks/ and codeframe/context/ — empty/dead packages
  • codeframe/persistence/database.py.backup — committed junk
  • 12 test files covering the above; surgically trimmed only the github_issue_sync section from the still-live tests/core/test_reconciliation.py

Validation

  • No dangling references to deleted modules in codeframe/ (non-test).
  • App boots: TestClient(app) lifespan → /health 200.
  • Collection clean (pytest --collect-only → 0 errors, 5081 collected).
  • Full uv run pytest -m v22920 passed, 0 failed.

Still remaining (dedicated follow-up)

The tangled v1 domain cluster — the multi-agent lead_agent/worker_agent/factory classes, core/project, and the deployment/discovery/enforcement/indexing/providers/testing dirs they import — is kept "reachable" via agents/__init__.py and cli/__init__.py. Removing it requires cutting those __init__ threads first and confirming no live cf command uses the old multi-agent execution model, then deleting the cluster + slimming the Database class methods. Plus PR 4: rename persistence/platform_store/ and fix CLAUDE.md's stale repo-structure notes.

Kept (live): E2B cloud exec, worktree sandbox, cf dashboard TUI, agents/dependency_resolver, live webhook notifications, the slim global control-plane DB.

Summary by CodeRabbit

  • Chores
    • Removed agent YAML generation functionality
    • Removed credential audit and validation systems
    • Removed project phase and status management
    • Removed GitHub issue synchronization
    • Removed quality gates tool
    • Removed SDK tool safety hooks and metrics logging
    • Removed notification delivery (desktop and webhook)
    • Removed blocker expiration job

Review Change Stack

Delete confirmed zero-importer dead modules + their tests:
- core/{credential_audit,credential_validator,phase_manager,project_status}.py
- git/github_issue_sync.py, lib/{quality_gate_tool,sdk_hooks}.py
- ui/auth.py, agents/subagent_generator.py
- notifications/{router,desktop}.py (legacy desktop notifications; live
  webhook notifications are unaffected)
- codeframe/tasks/ and codeframe/context/ (empty/dead packages)
- codeframe/persistence/database.py.backup (committed junk)
- 12 test files covering the above; trimmed the github_issue_sync section
  out of the still-live tests/core/test_reconciliation.py

Verified: no dangling refs, app boots (TestClient lifespan /health 200),
collection clean, full -m v2 green (2920 passed, 0 failed).

The tangled v1 domain cluster (multi-agent lead/worker classes, core/project,
and the deployment/discovery/enforcement/indexing/providers/testing dirs they
pull in, plus the Database-class method slimming) remains for a dedicated
follow-up — it's kept reachable via agents/__init__ and cli/__init__ and needs
careful untangling.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7ea13222-dfeb-4810-a100-92f356519c2b

📥 Commits

Reviewing files that changed from the base of the PR and between 66241c2 and 12a06c6.

📒 Files selected for processing (28)
  • codeframe/agents/subagent_generator.py
  • codeframe/context/__init__.py
  • codeframe/core/credential_audit.py
  • codeframe/core/credential_validator.py
  • codeframe/core/phase_manager.py
  • codeframe/core/project_status.py
  • codeframe/git/github_issue_sync.py
  • codeframe/lib/quality_gate_tool.py
  • codeframe/lib/sdk_hooks.py
  • codeframe/notifications/desktop.py
  • codeframe/notifications/router.py
  • codeframe/persistence/database.py.backup
  • codeframe/tasks/__init__.py
  • codeframe/tasks/expire_blockers.py
  • codeframe/ui/auth.py
  • tests/agents/test_subagent_generator.py
  • tests/api/test_discovery_restart.py
  • tests/blockers/test_blocker_expiration.py
  • tests/blockers/test_blocker_expiration_cron.py
  • tests/core/test_credential_audit.py
  • tests/core/test_credential_validator.py
  • tests/core/test_reconciliation.py
  • tests/integration/test_mvp_completion_workflow.py
  • tests/integration/test_notification_workflow.py
  • tests/lib/test_quality_gate_tool.py
  • tests/lib/test_sdk_hooks.py
  • tests/notifications/test_desktop.py
  • tests/notifications/test_router.py
💤 Files with no reviewable changes (25)
  • codeframe/ui/auth.py
  • codeframe/notifications/desktop.py
  • codeframe/lib/quality_gate_tool.py
  • tests/core/test_reconciliation.py
  • tests/notifications/test_router.py
  • codeframe/core/phase_manager.py
  • tests/integration/test_mvp_completion_workflow.py
  • tests/core/test_credential_audit.py
  • codeframe/tasks/expire_blockers.py
  • tests/lib/test_sdk_hooks.py
  • codeframe/git/github_issue_sync.py
  • codeframe/core/project_status.py
  • codeframe/core/credential_validator.py
  • codeframe/agents/subagent_generator.py
  • tests/notifications/test_desktop.py
  • codeframe/lib/sdk_hooks.py
  • tests/integration/test_notification_workflow.py
  • codeframe/notifications/router.py
  • tests/blockers/test_blocker_expiration_cron.py
  • tests/agents/test_subagent_generator.py
  • tests/lib/test_quality_gate_tool.py
  • tests/core/test_credential_validator.py
  • tests/blockers/test_blocker_expiration.py
  • codeframe/core/credential_audit.py
  • tests/api/test_discovery_restart.py

Walkthrough

This pull request removes multiple deprecated and superseded modules across the codebase. The removals span agent generation, credential validation, phase management, project status tracking, notification systems, quality gate tooling, GitHub issue synchronization, and a deprecated authentication module. Associated test files covering these features are also deleted, with two explicit test cleanup ranges documented.

Changes

Feature and Module Cleanup

Layer / File(s) Summary
Notification system removal
codeframe/notifications/desktop.py, codeframe/notifications/router.py, tests/notifications/test_desktop.py, tests/notifications/test_router.py
Removes desktop notification service with platform-specific message delivery (macOS/Linux/Windows) and the multi-channel notification router that coordinated desktop and webhook notifications.
Credential validation framework removal
codeframe/core/credential_audit.py, codeframe/core/credential_validator.py, tests/core/test_credential_audit.py, tests/core/test_credential_validator.py
Removes audit logging for credential store/retrieve/delete/rotate/validate operations and the workflow credential validation system with fail-fast checks and structured error reporting.
Phase management and project status removal
codeframe/core/phase_manager.py, codeframe/core/project_status.py, tests/api/test_discovery_restart.py
Removes phase transition validation, workspace session state management (task counts, progress metrics, session state persistence), and API tests for discovery restart and phase manager transitions.
SDK tooling and GitHub integration removal
codeframe/lib/quality_gate_tool.py, codeframe/lib/sdk_hooks.py, codeframe/git/github_issue_sync.py, tests/lib/test_quality_gate_tool.py, tests/lib/test_sdk_hooks.py, tests/core/test_reconciliation.py (partial), tests/blockers/test_blocker_expiration*.py
Removes quality gate MCP tool, SDK pre/post tool safety hooks, GitHub issue state fetching, blocker expiration cron job, and associated test coverage for these systems.
Agent generation and deprecated auth removal
codeframe/agents/subagent_generator.py, codeframe/ui/auth.py, codeframe/tasks/expire_blockers.py, tests/agents/test_subagent_generator.py, tests/integration/test_mvp_completion_workflow.py, tests/integration/test_notification_workflow.py
Removes YAML-to-SDK agent definition loader, deprecated auth module, stale blocker expiration job, and integration tests for MVP completion workflow and notification routing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Whiskers twitching with delight
Old modules hop away from sight,
Tests vanish like morning dew,
Clean slate, fresh code in view!
CodeFrame's spring cleaning done right. 🌱

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: removal of dead leaf modules as part of v1 domain cleanup. It is specific, accurate, and directly reflects the changeset scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/remove-v1-domain-layer

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown

claude Bot commented May 27, 2026

Code Review — PR #598: Dead leaf module removal (v1 cleanup, part 1)

Summary: Pure deletion PR — 15 source files + 12 test files + 1 surgical test edit, ~11.6k lines removed, 0 added. The validation story in the PR description is solid and this is clearly safe to merge.


What's done well

  • Verification methodology is explicit and reproducible: reachability graph from both entry points (cli/app.py + ui/server.py) plus exact-path grep, then test collection check, then full v2 suite run. That is the right checklist for a deletion PR.
  • codeframe/ui/auth.py was already a tombstone (unconditional raise ImportError), so removing it eliminates dead-but-dangerous code that could silently mask import errors in edge cases.
  • Notification split is correctly handled: desktop/router (legacy) removed, Phase 5.3 webhook path (core/conductor.py, core/blockers.py, ui/routers/pr_v2.py) untouched. The PR description calls this out explicitly.
  • database.py.backup removal: committed backup files are a credentials/secrets risk surface. Good catch.
  • Surgical edit to tests/core/test_reconciliation.py: only TestGitHubIssueSync removed, the rest of the live reconciliation tests preserved. Correct approach.

One functional question to document

Blocker expiration: codeframe/tasks/expire_blockers.py was a cron-based hourly job that expired stale blockers and fired WebSocket notifications. Its tests (test_blocker_expiration.py, test_blocker_expiration_cron.py) are also removed here.

Is this functionality now covered in v2? A quick check of codeframe/core/blockers.py should show whether there is a TTL/expiry path. If it is genuinely unported, that is not a blocker for this PR (the code was unreachable), but it is worth tracking in the follow-up PR scope so it does not get lost. If it is already handled in v2, a one-liner in the PR description would close the loop.


Minor observations

  • codeframe/tasks/ package is now gone: both __init__.py (empty) and expire_blockers.py deleted. The v2 tasks module lives at codeframe/core/tasks.py and is unaffected. Flagging for the follow-up PR author in case anything tries to import from codeframe.tasks vs codeframe.core.tasks.
  • codeframe/context/ package is now gone: __init__.py was empty (empty blob e69de29). Fine to remove, but if there are any string-based dynamic imports referencing codeframe.context they will now get a ModuleNotFoundError. The grep pass should have caught this.
  • subagent_generator.py had a mutable module-level dict (YAML_TO_SDK_TOOL_MAPPING) with list values — moot now that it is deleted, but noted for context if this module is ever revived.

Overall

Clean execution of the CLAUDE.md roadmap. The remaining v1 domain cluster (lead_agent/worker_agent/factory + core/project + deployment/discovery/enforcement dirs) is correctly deferred to a follow-up that addresses the __init__ import threads first. Ready to merge pending the blocker-expiration question above.

@frankbria frankbria merged commit bf10f32 into main May 27, 2026
11 checks passed
@frankbria frankbria deleted the refactor/remove-v1-domain-layer branch May 27, 2026 16:21
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.

1 participant