Private
Public Access
conductor(plan): Mark Phase 4 complete - app_controller fastapi import removal + _require_warmed lift
This commit is contained in:
@@ -61,22 +61,29 @@ swaps to `_require_warmed(name)`.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Remove top-level FastAPI imports from `src/api_hooks.py` (TDD)
|
||||
## Phase 4: Remove top-level FastAPI imports from `src/app_controller.py` (TDD)
|
||||
|
||||
**DEVIATION FROM ORIGINAL SPEC**: The original spec/plan stated the fastapi
|
||||
imports were in `src/api_hooks.py`. After Phase 3 completion, audit revealed
|
||||
the actual fastapi top-level imports live in `src/app_controller.py` (lines
|
||||
17 and 21: `from fastapi import FastAPI, Depends, HTTPException` and
|
||||
`from fastapi.security.api_key import APIKeyHeader`). `src/api_hooks.py` does
|
||||
not import fastapi at all (it uses stdlib `http.server.ThreadingHTTPServer`).
|
||||
Phase 4 target is therefore corrected to `src/app_controller.py`.
|
||||
|
||||
Same pattern as Phase 3, for the FastAPI imports.
|
||||
|
||||
- [ ] **T4.1 (Red)** Write `tests/test_hook_server_no_top_level_fastapi.py`:
|
||||
- `test_hook_server_does_not_import_fastapi_at_module_level`: subprocess test
|
||||
- `test_hook_server_does_not_import_fastapi_security_at_module_level`
|
||||
- Confirm FAIL
|
||||
- [ ] **T4.2 (Green)** In `src/api_hooks.py`:
|
||||
- Remove `from fastapi import ...`, `from fastapi.security.api_key import ...` from top
|
||||
- Add `_require_warmed(name)` calls inside the methods that need them (FastAPI app construction, route registration)
|
||||
- [ ] **T4.3** Run existing `tests/test_api_hooks.py`; fix breakage (similar fallback strategy as Phase 3)
|
||||
- [ ] **T4.4** Confirm T4.1 tests PASS
|
||||
- [ ] **T4.5** Commit: `refactor(api_hooks): remove top-level fastapi imports; use _require_warmed` + git note
|
||||
- [x] **T4.1 (Red)** Write `tests/test_app_controller_no_top_level_fastapi.py` (4 tests). Commit pending.
|
||||
- [x] **T4.2 (Green)** Refactor done in commit 3849d304:
|
||||
- Created `src/module_loader.py` (shared home of `_require_warmed`)
|
||||
- `src/ai_client.py` re-exports `_require_warmed` for backwards compat
|
||||
- `src/app_controller.py`: added `from __future__ import annotations`; removed top-level fastapi imports; added lookups in `create_api()` and 7 `_api_*` helpers (`_api_get_key`, `_api_generate`, `_api_stream`, `_api_confirm_action`, `_api_get_session`, `_api_delete_session`, `_api_get_context`).
|
||||
- Import: `from src.module_loader import _require_warmed` (clean separation, not via ai_client)
|
||||
- [x] **T4.3** No new breakage. Pre-existing `test_generate_endpoint` failure in `test_headless_service.py` is a google.genai circular-import issue (reproduces on stashed pre-Phase-4 state) - not a regression. Documented in commit message.
|
||||
- [x] **T4.4** T4.1 tests PASS (4/4 green). T3.1 tests still pass (9/9, re-export works).
|
||||
- [x] **T4.5** Commit: `refactor(app_controller): remove top-level fastapi imports; lift _require_warmed to shared module` (commit 3849d304) + git note.
|
||||
|
||||
**Phase 4 checkpoint:** `from src.api_hooks import HookServer` does not import fastapi. The HookServer is fully constructed only after AppController's warmup has loaded fastapi (or after `_require_warmed("fastapi")` triggers the import in test mode).
|
||||
**Phase 4 checkpoint (target):** `import src.app_controller` does not trigger a fastapi import. The `create_api()` method uses `_require_warmed` to access FastAPI on demand. For non-web / non-`--enable-test-hooks` runs, fastapi is never loaded (saves ~470ms). For `--enable-test-hooks` runs, warmup pre-loads fastapi so the lookup is instant.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user