Pseudo-functional TypeScript: brief for a refactor roadmap #

Status: proposal — not started. This document is the input to a roadmap, not the roadmap. The convention it aims at is already recorded in AGENTS.md §"Shape of the code"; what is missing is a plan for bringing ~11,000 existing lines up to it.

Branch: functional-refactor. Nothing here is implemented yet.

Goal #

Move the harness toward the style the convention states: small pure functions composed with array combinators rather than imperative iteration, immutable data, discriminated unions where a shape has more than one form, no any/as/! escape hatches, and performance treated as a design input rather than a later cleanup. This is a behaviour-preserving refactor: the tests are the specification, and no function's observable results may change.

What the code base looks like today #

36 tracked .ts files, about 11,400 lines: extensions/ (pi runtime hooks — subagent, gemini-reviewer, review-ledger, shared renderers), scripts/ (CLI tooling and the review ledger), scripts/__tests__/.

The good news is that the structure is already functional: zero classes, 127 interface/type declarations, 659 readonly occurrences, and 180 map/filter/flatMap calls. This refactor is about expression-level shape, not about restructuring the modules.

Where it diverges, measured over all tracked .ts files:

signal	count	reading
map/filter/flatMap	180	already the default in places
for...of	~163	the main remaining iteration form
index-based for (…;…;…)	23	mostly argv walking and cursor scanners
let	145	reassignment, usually accumulators
.push(	91	building arrays in place
.reduce(	5	under-used relative to the push sites
as T casts	48	23 of them in tests
any	4	all at the pi / model-runtime boundary
non-null !	2	rare, both justified at a glance
while	6	cursor loops in parsers and renderers

Hotspots, ranked by loops + let + push (this is a smell score, not a defect list):

file	loops	let	push	lines
scripts/review-capture.ts	22	14	11	829
scripts/review-stats.ts	14	4	14	423
scripts/review-matrix.ts	9	8	12	611
extensions/subagent/format.ts	7	7	13	281
extensions/subagent/runner.ts	2	16	7	440
scripts/review-score.ts	9	8	3	359
extensions/subagent/render.ts	10	10	0	405
scripts/review-assess.ts	6	4	5	413

Specific shapes worth naming #

1. Sequential parsers accumulate with let + push. review-capture.ts walks JSONL session files line by line, review-stats.ts aggregates the ledger, review-matrix.ts rewrites marked regions in place. Each is a fold over a sequence wearing a loop: the state that carries between lines is exactly a reduce accumulator, and making that state an explicit named type would also make the parsers testable one line at a time.
2. any sits where the pi API meets our types. Four sites: extensions/subagent/format.ts:54 (themeFg: (color: any, …)), extensions/subagent/runner.ts:259 (let event: any), runner.ts:305 ((part: any)), and scripts/review-score.ts:185 (as Model<any>). The first three are the same boundary — pi streams untyped event objects and we pattern-match parts — so a single narrowing adapter returning a discriminated union would delete all three anys and the casts around them.
3. Casts cluster in the tests. 23 of 48 as T casts are fixtures building partial records. A small typed fixture factory would remove most of them, at the cost of one more file in __tests__/helpers/.
4. Renderers build strings by push. format.ts and render.ts assemble output chunks. Some of this should become join/flatMap composition; some should not (see below).

Where functional style should stop #

This is the part worth being opinionated about, because "prefer combinators" applied literally would damage these files:

• Streaming and per-event rendering. runner.ts and render.ts run once per delta. Three intermediate arrays per token is the wrong trade; these stay loops, and that is a deliberate exception rather than an oversight.
• Cursor scanners in review-capture.ts and review-matrix.ts, where the index is genuinely the state (skip a region, resume mid-line). Converting those to reduce over lines often reads worse; the honest target there is to extract the body into a named pure function, not to eliminate the loop.
• CLI argument parsing, where early exit on a recognised flag is the whole point.
• Anything that measures slower. A combinator chain that is clearer at equal speed wins; at 2× the cost on a hot path, it does not.

Constraints #

• Node 24 type stripping: erasable syntax only. No enum, no namespace, no parameter properties, no import =. A Result-style type must be a discriminated union, not a class hierarchy.
• No build step and no runtime dependencies beyond typebox. Shared combinators must be plain functions in our own tree, not a library.
• Explicit .ts import specifiers — any new module must be imported the same way.
• tsc --noEmit and node --test "scripts/__tests__/*.test.ts" stay green, and both must be run together: Node erases types, so tests passing does not mean it type-checks.
• The pi extension API is fixed. Extension entry points keep their existing signatures.

Questions for the roadmap #

1. Sequencing. Which files first? My instinct is the parsers (review-capture, review-stats) before the renderers, because their state is data and the tests are already behavioural — but the renderers may be the bigger readability win per unit of risk.
2. Shared combinators: yes or no? A pipe/flow, a typed groupBy, and a small Result for the parsers that currently signal failure with undefined and a comment. Or is the stdlib plus named intermediate functions enough, so that we add no shared abstraction at all?
3. The pi boundary. Narrow once in an adapter, or keep the anys at the exact call sites and cast immediately? The adapter is cleaner but adds a file that must track pi's event shape.
4. A fitness function. The repo already has scripts/review-matrix.ts --check as a precedent for a script that fails CI when generated state drifts. Should this refactor ship with an equivalent — a counter for let/push/any/as per file that fails on regression — or is that a metric to game?
5. Commit and PR shape. One file per commit, or one pattern per commit across files? What keeps each step independently revertable while still being reviewable as a whole?
6. Success measures. Which numbers are worth moving (loops→combinators, let→const, push→reduce, any→0, casts in tests → under 10), and which floors are traps — e.g. capture.ts's cursor loop, which should still be a loop when the refactor is done.

What to send back #

A phased roadmap: for each phase, the files, the shape of the change, the risk, the verification, and what "done" looks like. Concrete and independently landable, so that each phase can be reviewed as its own change rather than as one enormous rewrite. Where the answer is "leave this loop alone", say so explicitly — the value of the roadmap is as much in its exclusions as in its targets.