theirongolddev/amc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
c5b1fb3a803925e97726a7fbb665fa469769094d
amc/PROPOSED_CODE_FILE_REORGANIZATION_PLAN.md
teernisse 1fb4a82b39 refactor(server): extract context.py into focused modules 
Split the monolithic context.py (117 lines) into five purpose-specific
modules following single-responsibility principle:

- config.py: Server-level constants (DATA_DIR, SESSIONS_DIR, PORT,
  STALE_EVENT_AGE, _state_lock)
- agents.py: Agent-specific paths and caches (CLAUDE_PROJECTS_DIR,
  CODEX_SESSIONS_DIR, discovery caches)
- auth.py: Authentication token generation/validation for spawn endpoint
- spawn_config.py: Spawn feature configuration (PENDING_SPAWNS_DIR,
  rate limiting, projects watcher thread)
- zellij.py: Zellij binary resolution and session management constants

This refactoring improves:
- Code navigation: Find relevant constants by domain, not alphabetically
- Testing: Each module can be tested in isolation
- Import clarity: Mixins import only what they need
- Future maintenance: Changes to one domain don't risk breaking others

All mixins updated to import from new module locations. Tests updated
to use new import paths.

Includes PROPOSED_CODE_FILE_REORGANIZATION_PLAN.md documenting the
rationale and mapping from old to new locations.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-02-28 00:47:15 -05:00

317 lines
16 KiB
Markdown
Raw Blame History

# Proposed Code File Reorganization Plan
## Executive Summary
After reading every source file in the project, analyzing all import graphs, and understanding how each module fits into the architecture, my assessment is: **the project is already reasonably well-organized**. The mixin-based decomposition of the handler, the dashboard's `components/utils/lib` split, and the test structure that mirrors source all reflect sound engineering.
That said, there is one clear structural problem and a few smaller wins. This plan proposes **surgical, high-value changes** rather than a gratuitous restructure. The guiding principle: every change must make it easier for a developer (or agent) to find things and understand the architecture.
---
## Current Structure (Annotated)
```
amc/
amc_server/ # Python backend (2,571 LOC)
__init__.py # Package init, exports main
server.py # Server startup/shutdown (38 LOC)
handler.py # Handler class composed from mixins (31 LOC)
context.py # ** PROBLEM ** All config, constants, caches, locks, auth (121 LOC)
logging_utils.py # Logging + signal handlers (31 LOC)
mixins/ # Handler mixins (one per concern)
__init__.py # Package comment (1 LOC)
http.py # HTTP routing, static file serving (173 LOC)
state.py # State aggregation, SSE, session collection, cleanup (440 LOC)
conversation.py # Conversation parsing for Claude/Codex (278 LOC)
control.py # Session dismiss/respond, Zellij pane injection (295 LOC)
discovery.py # Codex session discovery, pane matching (347 LOC)
parsing.py # JSONL parsing, context usage extraction (274 LOC)
skills.py # Skill enumeration for autocomplete (184 LOC)
spawn.py # Agent spawning in Zellij tabs (358 LOC)
dashboard/ # Preact frontend (2,564 LOC)
index.html # Entry HTML with Tailwind config
main.js # App mount point (7 LOC)
styles.css # Custom styles
lib/ # Third-party/shared
preact.js # Preact re-exports
markdown.js # Markdown rendering + syntax highlighting (159 LOC)
utils/ # Pure utility functions
api.js # API constants + fetch helpers (39 LOC)
formatting.js # Time/token formatting (66 LOC)
status.js # Status metadata + session grouping (79 LOC)
autocomplete.js # Autocomplete trigger detection (48 LOC)
components/ # UI components
App.js # Root component (616 LOC)
Sidebar.js # Project nav sidebar (102 LOC)
SessionCard.js # Session card (176 LOC)
Modal.js # Full-screen modal wrapper (79 LOC)
ChatMessages.js # Message list (39 LOC)
MessageBubble.js # Individual message (54 LOC)
QuestionBlock.js # AskUserQuestion UI (228 LOC)
SimpleInput.js # Freeform text input (228 LOC)
OptionButton.js # Option button (24 LOC)
AgentActivityIndicator.js # Turn timer (115 LOC)
SpawnModal.js # Spawn dropdown (241 LOC)
Toast.js # Toast notifications (125 LOC)
EmptyState.js # Empty state (18 LOC)
Header.js # ** DEAD CODE ** (58 LOC, zero imports)
SessionGroup.js # ** DEAD CODE ** (56 LOC, zero imports)
bin/ # Shell/Python scripts
amc # Launcher (start/stop/status)
amc-hook # Hook script (standalone, writes session state)
amc-server # Server launch script
amc-server-restart # Server restart helper
tests/ # Test suite (mirrors mixin structure)
test_context.py # Context tests
test_control.py # Control mixin tests
test_conversation.py # Conversation parsing tests
test_conversation_mtime.py # Conversation mtime tests
test_discovery.py # Discovery mixin tests
test_hook.py # Hook script tests
test_http.py # HTTP mixin tests
test_parsing.py # Parsing mixin tests
test_skills.py # Skills mixin tests
test_spawn.py # Spawn mixin tests
test_state.py # State mixin tests
test_zellij_metadata.py # Zellij metadata tests
e2e/ # End-to-end tests
__init__.py
test_skills_endpoint.py
test_autocomplete_workflow.js
e2e_spawn.sh # Spawn E2E script
```
---
## Proposed Changes
### Change 1: Split `context.py` into Focused Modules (HIGH VALUE)
**Problem:** `context.py` is the classic "junk drawer" module. It contains:
- Path constants for the server, Zellij, Claude, and Codex
- Server configuration (port, timeouts)
- 5 independent caches with their own size limits
- 2 threading locks for unrelated concerns
- Auth token generation/validation
- Zellij binary resolution
- Spawn-related config
- Background thread management for projects cache
Every mixin imports from it, but each only needs a subset. When a developer asks "where is the spawn rate limit configured?", they have to scan through an unrelated grab-bag of constants. When they ask "where's the Codex transcript cache?", same problem.
**Proposed split:**
```
amc_server/
config.py # Server-level constants: PORT, DATA_DIR, SESSIONS_DIR, EVENTS_DIR,
# DASHBOARD_DIR, PROJECT_DIR, STALE_EVENT_AGE, STALE_STARTING_AGE
# These are the "universal" constants every module might need.
zellij.py # Zellij integration: ZELLIJ_BIN resolution, ZELLIJ_PLUGIN path,
# ZELLIJ_SESSION, _zellij_cache (sessions cache + expiry)
# Rationale: All Zellij-specific constants and helpers in one place.
# Any developer working on Zellij integration knows exactly where to look.
agents.py # Agent-specific paths and caches:
# CLAUDE_PROJECTS_DIR, CODEX_SESSIONS_DIR, CODEX_ACTIVE_WINDOW,
# _codex_pane_cache, _codex_transcript_cache, _CODEX_CACHE_MAX,
# _context_usage_cache, _CONTEXT_CACHE_MAX,
# _dismissed_codex_ids, _DISMISSED_MAX
# Rationale: Agent data source configuration and caches that are only
# relevant to discovery/parsing mixins, not the whole server.
auth.py # Auth token: generate_auth_token(), validate_auth_token(), _auth_token
# Rationale: Security-sensitive code in its own module. Small, but
# architecturally clean separation from general config.
spawn_config.py # Spawn feature config:
# PROJECTS_DIR, PENDING_SPAWNS_DIR, PENDING_SPAWN_TTL,
# _spawn_lock, _spawn_timestamps, SPAWN_COOLDOWN_SEC
# + start_projects_watcher() (background refresh thread)
# Rationale: Spawn feature has its own set of constants, lock, and
# background thread. Currently scattered between context.py and spawn.py.
# Consolidating makes the spawn feature self-contained.
```
Kept from current structure (unchanged):
- `_state_lock` moves to `config.py` (it's a server-level concern)
**Import changes required:**
| File | Current import from `context` | New import from |
|------|------|------|
| `server.py` | `DATA_DIR, PORT, generate_auth_token, start_projects_watcher` | `config.DATA_DIR, config.PORT`, `auth.generate_auth_token`, `spawn_config.start_projects_watcher` |
| `handler.py` | (none, uses mixins) | (unchanged) |
| `mixins/http.py` | `DASHBOARD_DIR`, `ctx._auth_token` | `config.DASHBOARD_DIR`, `auth._auth_token` |
| `mixins/state.py` | `EVENTS_DIR, SESSIONS_DIR, STALE_*, ZELLIJ_BIN, _state_lock, _zellij_cache` | `config.*`, `zellij.ZELLIJ_BIN, zellij._zellij_cache` |
| `mixins/conversation.py` | `EVENTS_DIR` | `config.EVENTS_DIR` |
| `mixins/control.py` | `SESSIONS_DIR, ZELLIJ_BIN, ZELLIJ_PLUGIN, _DISMISSED_MAX, _dismissed_codex_ids` | `config.SESSIONS_DIR`, `zellij.*`, `agents._DISMISSED_MAX, agents._dismissed_codex_ids` |
| `mixins/discovery.py` | `CODEX_*, PENDING_SPAWNS_DIR, SESSIONS_DIR, _codex_*` | `agents.*`, `spawn_config.PENDING_SPAWNS_DIR`, `config.SESSIONS_DIR` |
| `mixins/parsing.py` | `CLAUDE_PROJECTS_DIR, CODEX_SESSIONS_DIR, _*_cache, _*_MAX` | `agents.*` |
| `mixins/spawn.py` | `PENDING_SPAWNS_DIR, PROJECTS_DIR, SESSIONS_DIR, ZELLIJ_*, _spawn_*, validate_auth_token` | `spawn_config.*`, `config.SESSIONS_DIR`, `zellij.*`, `auth.validate_auth_token` |
**Why this is the right split:**
1. **By domain, not by size.** Each new module groups constants + caches + helpers that serve one architectural concern. A developer working on Zellij integration opens `zellij.py`. Working on Codex discovery? `agents.py`. Spawn feature? `spawn_config.py`.
2. **No circular imports.** The dependency graph is DAG: `config.py` is a leaf (imported by everything, imports nothing from `amc_server`). `zellij.py`, `agents.py`, `auth.py`, `spawn_config.py` import only from `config.py` (if at all). Mixins import from these.
3. **No behavioral change.** Module-level caches and singletons work the same way whether they're in one file or five.
---
### Change 2: Delete Dead Dashboard Components (LOW EFFORT, HIGH CLARITY)
**Problem:** `Header.js` (58 LOC) and `SessionGroup.js` (56 LOC) are completely unused. Zero imports anywhere in the codebase. They were replaced by the current Sidebar + grid layout but never cleaned up.
**Action:** Delete both files.
**Import changes required:** None (nothing imports them).
**Rationale:** Dead code is noise. Anyone exploring the `components/` directory would reasonably assume these are active and try to understand how they fit. Removing them prevents that confusion.
---
### Change 3: No Changes to Dashboard Structure
The dashboard is already well-organized:
- `components/` - All React-like components
- `utils/` - Pure utility functions (formatting, API, status, autocomplete)
- `lib/` - Third-party wrappers (Preact, markdown rendering)
This is a standard and intuitive layout. The `components/` directory has 13 files (15 before dead code removal), which is manageable. Creating sub-directories (e.g., `components/session/`, `components/layout/`) would add nesting without meaningful benefit at this scale.
---
### Change 4: No Changes to `mixins/` Structure
The mixin decomposition is the project's architectural backbone. Each mixin handles one concern:
| Mixin | Responsibility |
|-------|---------------|
| `http.py` | HTTP routing, static file serving, CORS |
| `state.py` | State aggregation, SSE streaming, session collection |
| `conversation.py` | Conversation history parsing (Claude + Codex JSONL) |
| `control.py` | Session dismiss/respond, Zellij pane injection |
| `discovery.py` | Codex session auto-discovery, pane matching |
| `parsing.py` | JSONL tail reading, context usage extraction, caching |
| `skills.py` | Skill enumeration for Claude/Codex autocomplete |
| `spawn.py` | Agent spawning in Zellij tabs |
All are 170-440 lines, which is reasonable. The largest (`state.py` at 440 lines) could theoretically be split, but its methods are tightly coupled around session collection. Splitting would create artificial seams.
---
### Change 5: No Changes to `tests/` Structure
Tests already mirror the source structure (`test_state.py` tests `mixins/state.py`, etc.). This is the correct pattern.
**One consideration:** After splitting `context.py`, `test_context.py` may need updates to import from the new module locations. The test file is small (755 bytes) and covers basic context constants, so the update would be trivial.
---
### Change 6: No Changes to `bin/` Scripts
The `amc-hook` script intentionally duplicates `DATA_DIR`, `SESSIONS_DIR`, `EVENTS_DIR` from `context.py`. This is correct: the hook runs as a standalone process launched by Claude Code, not as part of the server. It must be self-contained with zero dependencies on the server package. Sharing code would create a fragile coupling.
---
## What I Explicitly Decided NOT to Do
1. **Not creating a `src/` directory.** The project root is clean. Adding `src/` would be an extra nesting level with no benefit.
2. **Not splitting any mixins.** `state.py` (440 LOC) and `spawn.py` (358 LOC) are the largest, but their methods are cohesive. Splitting would scatter related logic across files.
3. **Not merging small files.** `EmptyState.js` (18 LOC), `OptionButton.js` (24 LOC), and `ChatMessages.js` (39 LOC) are tiny but each has a clear purpose and is imported independently. Merging them would violate component-per-file convention.
4. **Not reorganizing dashboard components into sub-folders.** With 13 components, flat is fine. Sub-folders like `components/session/` and `components/layout/` become necessary at ~25+ components.
5. **Not consolidating `api.js` + `formatting.js` + `status.js` + `autocomplete.js`.** Each is focused and independently imported. A combined `utils.js` would be a grab-bag (the exact problem we're fixing in `context.py`).
6. **Not moving `markdown.js` out of `lib/`.** It uses third-party dependencies and provides rendering utilities. `lib/` is the correct location.
---
## Proposed Final Structure
```
amc/
amc_server/
__init__.py # (unchanged)
server.py # (updated imports)
handler.py # (unchanged)
config.py # NEW: Server constants, DATA_DIR, SESSIONS_DIR, EVENTS_DIR, PORT, etc.
zellij.py # NEW: Zellij binary resolution, ZELLIJ_PLUGIN, ZELLIJ_SESSION, cache
agents.py # NEW: Agent paths (Claude/Codex), transcript caches, dismissed cache
auth.py # NEW: Auth token generation/validation
spawn_config.py # NEW: Spawn constants, locks, rate limiting, projects watcher
logging_utils.py # (unchanged)
mixins/ # (unchanged structure, updated imports)
__init__.py
http.py
state.py
conversation.py
control.py
discovery.py
parsing.py
skills.py
spawn.py
dashboard/
index.html # (unchanged)
main.js # (unchanged)
styles.css # (unchanged)
lib/
preact.js # (unchanged)
markdown.js # (unchanged)
utils/
api.js # (unchanged)
formatting.js # (unchanged)
status.js # (unchanged)
autocomplete.js # (unchanged)
components/
App.js # (unchanged)
Sidebar.js # (unchanged)
SessionCard.js # (unchanged)
Modal.js # (unchanged)
ChatMessages.js # (unchanged)
MessageBubble.js # (unchanged)
QuestionBlock.js # (unchanged)
SimpleInput.js # (unchanged)
OptionButton.js # (unchanged)
AgentActivityIndicator.js # (unchanged)
SpawnModal.js # (unchanged)
Toast.js # (unchanged)
EmptyState.js # (unchanged)
[DELETED] Header.js
[DELETED] SessionGroup.js
bin/ # (unchanged)
tests/ # (minor import updates in test_context.py)
```
---
## Implementation Order
1. **Delete dead dashboard components** (`Header.js`, `SessionGroup.js`) - zero risk, instant clarity
2. **Create new Python modules** (`config.py`, `zellij.py`, `agents.py`, `auth.py`, `spawn_config.py`) with the correct constants/functions
3. **Update all mixin imports** to use new module locations
4. **Update `server.py`** imports
5. **Delete `context.py`**
6. **Run full test suite** to verify nothing broke
7. **Update `test_context.py`** if needed
---
## Risk Assessment
- **Risk of breaking imports:** MEDIUM. There are many import statements to update across 8 mixin files + `server.py`. Mitigated by running the full test suite after changes.
- **Risk of circular imports:** LOW. The new modules form a clean DAG (config <- zellij/agents/auth/spawn_config <- mixins).
- **Risk to `bin/amc-hook`:** NONE. The hook is standalone and doesn't import from `amc_server`.
- **Risk to dashboard:** NONE for dead code deletion. Zero imports to either file.
Reference in New Issue View Git Blame Copy Permalink