mirror of
https://bitbucket.org/siakitem/my-pi.git
synced 2026-08-28 08:35:57 +00:00
47 lines
2.5 KiB
Markdown
47 lines
2.5 KiB
Markdown
---
|
|
issue: 127
|
|
issue_title: "refactor: extract SessionLogger interface to unify logging + notification"
|
|
---
|
|
|
|
# Retro: #127 — extract SessionLogger interface
|
|
|
|
## Final Retrospective (2026-05-08T01:10:00Z)
|
|
|
|
### Session summary
|
|
|
|
Extracted three separate `HandlerDeps` logging/notification fields (`writeDebugLog`, `writeReviewLog`, `notifyWarning`) into a `SessionLogger` interface with `debug`/`review`/`warn` methods.
|
|
Created `src/session-logger.ts` with the interface and `createSessionLogger()` factory, updated all handler source files and 6 test `makeDeps()` factories.
|
|
Shipped as v5.8.0 with zero behavioral change. 7 new unit tests; total suite 1252 tests across 56 files.
|
|
|
|
### Observations
|
|
|
|
#### What went well
|
|
|
|
- **Three-phase pipeline completed cleanly.**
|
|
Plan → TDD (4 cycles) → ship in one session with no user corrections needed.
|
|
- **Plan-to-code translation was nearly 1:1.**
|
|
The `SessionLogger` interface, `createSessionLogger()` factory, `HandlerDeps` change, and handler migration all matched the plan exactly.
|
|
The mechanical find-and-replace nature of the change made the TDD steps predictable.
|
|
- **Test factory updates were trivially correct.**
|
|
Replacing 3 fields with 1 nested object (`logger: { debug: vi.fn(), review: vi.fn(), warn: vi.fn() }`) worked identically across all 6 files.
|
|
|
|
#### What caused friction (agent side)
|
|
|
|
- `wrong-abstraction` — Plan listed `src/handlers/gates/runner.ts` as needing `deps.writeReviewLog` → `deps.logger.review` changes, but `runner.ts` uses `GateRunnerDeps` (a separate interface explicitly scoped as a non-goal), not `HandlerDeps`.
|
|
The plan confused the parameter name `deps` (which appears in both `runGateCheck` and handler functions) with the `HandlerDeps` type.
|
|
Self-identified during step 2 by reading the import at the top of `runner.ts` before editing.
|
|
Impact: added friction but no rework — no incorrect edit was made.
|
|
|
|
- `missing-context` — Multi-block edit on `src/handlers/types.ts` accidentally introduced a `/** @deprecated Use logger.warn instead. */` comment above `logResolvedConfigPaths()`.
|
|
The third edit block was intended to remove `notifyWarning` and its JSDoc, but the replacement text included a stray deprecation annotation that attached to the wrong field.
|
|
Self-identified by re-reading the file immediately after the edit.
|
|
Impact: one follow-up edit to remove the stray comment, ~30 seconds of rework, no incorrect commit landed.
|
|
|
|
#### What caused friction (user side)
|
|
|
|
- None observed.
|
|
|
|
### Changes made
|
|
|
|
1. Created `docs/retro/0127-extract-session-logger.md` (this file).
|