Skip to content

Commit 2755136

Browse files
neoneyeneoneye
authored
Merge pull request #472 from PlanExeOrg/clean-up-roadmap
Codex's roast of PlanExe and proposal for fixes
2 parents 0097ed2 + d34f6cf commit 2755136

1 file changed

Lines changed: 229 additions & 0 deletions

File tree

‎docs/proposals/131-some-title.md‎

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
---
2+
title: "Codebase Cleanliness Remediation Roadmap"
3+
date: 2026-03-31
4+
status: Proposal
5+
author: PlanExe Team
6+
---
7+
8+
# Codebase Cleanliness Remediation Roadmap
9+
10+
**Author:** PlanExe Team
11+
**Date:** 2026-03-31
12+
**Status:** Proposal
13+
**Tags:** `codebase`, `refactor`, `maintainability`, `testing`, `architecture`
14+
15+
---
16+
17+
## Pitch
18+
19+
PlanExe has strong architectural intent, good repo-level guardrails, and a meaningful test footprint, but several core services have accumulated large load-bearing modules, mixed production and experimental code, and uneven operational hygiene. This proposal defines a cleanup program that improves maintainability without breaking service contracts or slowing ongoing product work.
20+
21+
## Problem
22+
23+
The codebase is not chaotic, but it is carrying too much complexity in too few files. The main issues found during inspection were:
24+
25+
1. Giant modules that concentrate unrelated responsibilities.
26+
2. Debug and operational scaffolding mixed into production entrypoints.
27+
3. Experimental and proof-of-concept code living too close to production paths.
28+
4. Broad exception handling that weakens failure diagnosis.
29+
5. Uneven logging and runtime hygiene across services.
30+
6. Incomplete test coverage in some high-risk areas, especially the multi-user frontend.
31+
32+
This matters because PlanExe is evolving into a multi-service execution engine. Large files and mixed concerns slow down review, increase regression risk, and make it harder for both humans and autonomous agents to safely change the system.
33+
34+
## Feasibility
35+
36+
This cleanup is feasible now because the repo already has several advantages:
37+
38+
1. Service boundaries are documented in package-level `AGENTS.md` files.
39+
2. Shared contracts are called out explicitly for `worker_plan`, `database_api`, `worker_plan_api`, `mcp_cloud`, and the frontends.
40+
3. There is already a meaningful unit-test base across `worker_plan`, `mcp_cloud`, `mcp_local`, and shared utilities.
41+
42+
The main constraint is backward compatibility. We should not redesign public APIs while cleaning internals. The cleanup must preserve:
43+
44+
1. `worker_plan` request and response shapes.
45+
2. `mcp_cloud` and `mcp_local` tool contracts.
46+
3. Shared DB models and legacy compatibility behavior.
47+
48+
## Proposal
49+
50+
Define a staged remediation program focused on six concrete hygiene issues.
51+
52+
## Issue 1: Giant Load-Bearing Modules
53+
54+
### Evidence
55+
56+
Several files are too large and own too many responsibilities:
57+
58+
1. `worker_plan/worker_plan_internal/plan/run_plan_pipeline.py` at 4,288 lines.
59+
2. `frontend_multi_user/src/app.py` at 3,857 lines.
60+
3. `worker_plan_database/app.py` at 1,520 lines.
61+
4. `mcp_cloud/http_server.py` at 1,431 lines.
62+
63+
These files are not just large. They mix routing, orchestration, validation, operational policy, artifact handling, billing, auth, or workflow logic in the same module.
64+
65+
### Why it is a problem
66+
67+
Large modules make the code harder to reason about, harder to test in isolation, and easier to break when adding unrelated features. They also push reviewers toward shallow approval because a single diff can span too many concerns.
68+
69+
### Fix steps
70+
71+
1. Split `frontend_multi_user/src/app.py` by concern into `auth`, `billing`, `admin`, `downloads`, `account`, and `plan_routes`.
72+
2. Split `mcp_cloud/http_server.py` into `middleware`, `route_registration`, `tool_http_bridge`, and `server_boot`.
73+
3. Convert `worker_plan/worker_plan_internal/plan/run_plan_pipeline.py` from a giant task registry file into a thin pipeline assembly module plus task-specific modules grouped by stage.
74+
4. Extract reusable orchestration helpers from `worker_plan_database/app.py` into focused worker, billing, and queue modules.
75+
5. Set an internal size target for service modules. As a starting rule, new files should stay below roughly 500 lines unless there is a strong reason not to.
76+
77+
## Issue 2: Debug Scaffolding in Production Entrypoints
78+
79+
### Evidence
80+
81+
Production-facing files still contain direct startup `print()` diagnostics or ad hoc debugging traces, for example in:
82+
83+
1. `mcp_cloud/http_server.py`
84+
2. `worker_plan/worker_plan_internal/llm_factory.py`
85+
3. Several task modules that print query and response payloads in executable paths
86+
87+
### Why it is a problem
88+
89+
Direct prints are sometimes useful during incident response, but they are not a coherent observability strategy. They create inconsistent runtime output, complicate log filtering, and encourage one-off diagnostics instead of structured instrumentation.
90+
91+
### Fix steps
92+
93+
1. Replace entrypoint `print()` startup breadcrumbs with structured `logging` calls at `INFO` or `DEBUG`.
94+
2. Gate verbose diagnostics behind explicit env vars such as `PLANEXE_DEBUG_STARTUP` or module-specific debug flags.
95+
3. Move sample-driver code and debugging helpers into `if __name__ == "__main__":` blocks or dedicated scripts.
96+
4. Add a lightweight test or lint-like assertion that production modules do not contain uncategorized top-level `print()` calls.
97+
98+
## Issue 3: Experimental Code Mixed with Production Code
99+
100+
### Evidence
101+
102+
The repo contains `worker_plan/worker_plan_internal/proof_of_concepts/` with 19 Python files plus several `experimental_premise_attack*.py` modules under production-adjacent diagnostics paths.
103+
104+
### Why it is a problem
105+
106+
Experimental work is good. Leaving it in production-adjacent trees without strong boundaries makes discovery noisier, encourages accidental coupling, and makes the production surface look less intentional than it is.
107+
108+
### Fix steps
109+
110+
1. Move proof-of-concept code into a clearly isolated top-level `experiments/` or `research/` area, or explicitly mark it as non-production in module names and docs.
111+
2. Move experimental diagnostics variants out of the default runtime namespace unless they are active candidates for shipping.
112+
3. Add a short README in each experimental area defining its status, owner, and graduation criteria.
113+
4. Ensure production imports never depend on experimental modules.
114+
115+
## Issue 4: Broad Exception Handling
116+
117+
### Evidence
118+
119+
The repo contains many `except Exception:` blocks across service and runtime code, including in:
120+
121+
1. `frontend_multi_user/src/app.py`
122+
2. `worker_plan_database/app.py`
123+
3. `mcp_cloud/http_server.py`
124+
4. `worker_plan/app.py`
125+
5. `worker_plan_internal/llm_util/*`
126+
127+
Some are reasonable containment boundaries, but many are too broad to preserve actionable failure context.
128+
129+
### Why it is a problem
130+
131+
Broad exception handling hides root causes, weakens retry logic, and makes it easier for bugs to silently degrade behavior rather than fail in a controlled and visible way.
132+
133+
### Fix steps
134+
135+
1. Audit all `except Exception:` blocks and classify them as `intentional boundary`, `temporary workaround`, or `should narrow`.
136+
2. Narrow handlers to specific exception classes where possible.
137+
3. Where a broad boundary is required, log structured context and re-raise domain-specific exceptions rather than generic failures.
138+
4. Add tests for failure classification in critical paths such as MCP request handling, billing, downloads, and pipeline stop/retry logic.
139+
140+
## Issue 5: Uneven Logging and Runtime Hygiene
141+
142+
### Evidence
143+
144+
The codebase contains many local `logging.basicConfig(...)` calls spread across services and executable modules, plus a mix of logging styles and one-off debug behavior.
145+
146+
### Why it is a problem
147+
148+
Distributed `basicConfig` calls make runtime behavior inconsistent and harder to control. They also blur the line between library code, service entrypoints, and local scripts.
149+
150+
### Fix steps
151+
152+
1. Restrict `logging.basicConfig(...)` to service entrypoints and dedicated CLI scripts.
153+
2. Remove logging configuration from reusable library modules.
154+
3. Define a small shared logging helper for PlanExe service startup so format and level handling are consistent.
155+
4. Standardize logger naming and expected levels for normal operation, diagnostics, and failure cases.
156+
157+
## Issue 6: Test Gaps in High-Risk Service Areas
158+
159+
### Evidence
160+
161+
The overall repo has a respectable test footprint, but `frontend_multi_user` explicitly notes that no automated tests currently exist for many UI or DB flows.
162+
163+
### Why it is a problem
164+
165+
The multi-user frontend handles auth, admin flows, billing, downloads, and user account state. That is too much business risk to leave mostly protected by manual confidence and good intentions.
166+
167+
### Fix steps
168+
169+
1. Add focused unit tests around billing, account state, admin user resolution, plan retry, and artifact download behavior.
170+
2. Add tests for helpers extracted from `frontend_multi_user/src/app.py` as part of the module split.
171+
3. Prioritize tests for failure paths, not just success paths.
172+
4. Keep tests close to the logic they protect so refactors remain cheap.
173+
174+
## Implementation Plan
175+
176+
### Phase 1: Inventory and Safety Rails
177+
178+
1. Create an inventory of oversized modules, broad exception handlers, and top-level print/debug usage.
179+
2. Tag each item as `refactor now`, `refactor when touched`, or `leave as boundary`.
180+
3. Add lightweight tests around current behavior before moving code in the largest modules.
181+
182+
### Phase 2: Split the Worst Offenders
183+
184+
1. Refactor `frontend_multi_user/src/app.py` first because it mixes the most distinct business concerns.
185+
2. Refactor `mcp_cloud/http_server.py` second because it sits on a public protocol boundary.
186+
3. Refactor `worker_plan_database/app.py` and `run_plan_pipeline.py` in smaller slices to avoid destabilizing the execution engine.
187+
188+
### Phase 3: Remove Operational Noise
189+
190+
1. Replace production `print()` usage with structured logging.
191+
2. Centralize startup logging setup per service.
192+
3. Move or label experimental modules so the production tree is easier to navigate.
193+
194+
### Phase 4: Exception and Test Cleanup
195+
196+
1. Narrow broad exception handlers in the services touched during earlier phases.
197+
2. Add targeted regression tests for every cleanup area.
198+
3. Update docs where module entrypoints or development workflows change.
199+
200+
## Integration Points
201+
202+
This proposal integrates with existing PlanExe boundaries rather than fighting them:
203+
204+
1. `worker_plan/AGENTS.md` already defines the public worker API and internal separation rules.
205+
2. `mcp_cloud/AGENTS.md` already documents the internal split that `http_server.py` should better reflect in code.
206+
3. `frontend_multi_user/AGENTS.md` already calls out DB and artifact invariants that should survive route extraction.
207+
4. The existing `python test.py` convention can remain the top-level test entrypoint while coverage expands.
208+
209+
## Success Metrics
210+
211+
1. No production-facing Python module above 1,500 lines after the first cleanup wave.
212+
2. `frontend_multi_user/src/app.py` reduced by at least 50% through route and helper extraction.
213+
3. `mcp_cloud/http_server.py` reduced to a focused HTTP assembly module rather than a mixed implementation file.
214+
4. Zero uncategorized top-level `print()` statements in production service modules.
215+
5. Documented justification for all remaining `except Exception:` boundaries in service code.
216+
6. New automated tests covering multi-user billing, retry, and download flows.
217+
218+
## Risks
219+
220+
1. Refactors may accidentally break backward compatibility across services.
221+
Mitigation: keep public contracts frozen and add regression tests before moving code.
222+
2. Cleanup work may turn into an endless style exercise with no shipping value.
223+
Mitigation: prioritize only high-leverage areas with operational or review cost.
224+
3. Pipeline refactors may destabilize long-running plan generation.
225+
Mitigation: split orchestration carefully and preserve task behavior while moving code.
226+
227+
## Why Now
228+
229+
PlanExe is already at the point where architectural cleanliness affects product velocity. The codebase has enough quality and enough structure to justify a cleanup pass, but it also has enough scale that delaying the work will make later changes slower, riskier, and more expensive. This is the right time to pay down the structural debt while the service boundaries are still understandable and before more execution-engine features pile on top.

0 commit comments

Comments
 (0)