chore(review): codebase review findings + ESLint/typedoc guardrails

[Luis85]

Summary

Multi-agent review of the 2026-04-24 codebase consolidated into an actionable plan at docs/plans/2026-04-24-codebase-review-findings.md, with Track A (guardrails + the meaningful refactors that fall out of those guardrails) landed live. The codebase is exceptionally disciplined — determinism is fully enforced, no default exports or enums in src/, no any, all skills return Result<T, E>. The findings are mostly stale docs, convention-only guardrails, and ratchet opportunities.

What ships in this PR

ESLint — architecture, size, and quality

Previously enforced by convention only, now enforced by CI:

Architecture

• ExportDefaultDeclaration banned (with carve-out for config files)
• TSEnumDeclaration banned (STYLE_GUIDE.md: as const unions)
• no-restricted-imports: core (src/**, excluding adapter/port/integration folders) cannot import excalibur, @tensorflow/*, js-son-agent, mistreevous, @anthropic-ai/sdk, openai, or sim-ecs

Complexity & size (agent navigability)

Rule	Level	Threshold
max-lines	error	350 (skip blank/comments)
max-lines-per-function	warn	150
complexity	warn	15
max-depth	warn	4
max-params	warn	5
max-nested-callbacks	warn	3

Quality

• no-console (allow warn/error), no-debugger, no-var, prefer-const, eqeqeq, no-throw-literal, no-duplicate-imports, no-unneeded-ternary, object-shorthand, @typescript-eslint/no-explicit-any, @typescript-eslint/switch-exhaustiveness-check, prefer-template, no-useless-concat, no-useless-rename, radix, default-case-last, no-lonely-if — all error
• Determinism win: @typescript-eslint/require-array-sort-compare — [].sort() without a comparator uses locale-dependent UTF-16 ordering, which silently breaks byte-equal trace replay. Locked in.
• @typescript-eslint/prefer-nullish-coalescing, prefer-optional-chain, consistent-type-exports — error
• @typescript-eslint/prefer-readonly — warn
• @typescript-eslint/no-non-null-assertion — warn (idiomatic under noUncheckedIndexedAccess: true)

Agent.ts split (–110 effective LOC: 562 → 484)

Reviewer flagged that overriding max-lines for Agent.ts is a smell. Three self-contained methods extracted into focused helpers under src/agent/internal/:

• Agent.restore() → RestoreCoordinator.ts (runRestore)
• Agent.die() → DeathCoordinator.ts (runDeath)
• Agent.snapshot() → SnapshotAssembler.ts (assembleSnapshot)

Plus a small applyLifecycleSnapshot() method on Agent so the coordinator never reaches into protected fields, and the duplicated isInMemoryMemoryAdapter typeguard moved next to the class it guards (src/memory/InMemoryMemoryAdapter.ts).

The remaining hotspot is tick() itself (~150 effective LOC of stage orchestration). Splitting it cleanly is its own focused PR (Track C2 of the plan). Until then, Agent.ts has a per-file max-lines: 500 override (down from the previous 700) so the cap is tighter than a global lift would be and the legacy outlier stays visible in CI.

Express skill consolidation (D1)

Three near-identical 24-line skills (ExpressMeowSkill, ExpressSadSkill, ExpressSleepySkill) collapsed onto a single createExpressionSkill(id, label, expression, fxHint) factory in src/skills/defaults/ExpressionSkill.ts. The three exports become one-line declarations sharing the same execution path. No caller-visible change.

Typedoc wired into CI

• Added missing adapter entry points (mistreevous, js-son, tfjs) — they were in package.json#exports but not typedoc.json.
• New docs job in .github/workflows/ci.yml, parallel with format/lint/typecheck/actionlint. Test job depends on it.
• npm run docs added to the verify script so local gate matches CI.
• Output continues to go to docs/api/ (already gitignored; sits alongside how-to/, plans/, specs/). Uploaded as artifact (api-docs, 14-day retention).

Test sort comparators

Four test files had .sort() without a comparator — now passes explicit code-point comparators. Locks in the same determinism discipline the library code follows.

Stale-prone metrics scrubbed from docs

Counts like "~300 tests" (CLAUDE.md — already 50% off), "10 default skills" (README, CLAUDE.md, docs/specs/vision.md), and "~80 KB unminified ESM" (README) replaced with descriptive phrasing or pointers to the source of truth (size-limit config, CI logs). These go stale every PR.

Verify gate after this PR

• format:check — clean
• lint — 0 errors, 11 ratchet warnings (Track C targets, all visible)
• typecheck — clean
• test — all passing
• build — every entry under its size-limit budget
• docs — typedoc emits 5 entry points, 0 errors

Punch list for follow-up PRs (in the plan, not in this PR)

Each = its own topic branch from develop per CLAUDE.md.

• Track B: README pre-release version mismatch, docs/plans/…polish-and-harden.md items already merged but marked "Not started", MEMORY.md reference, .changeset/config.json:baseBranch: "main" vs CLAUDE.md "PRs target develop", stale gray-matter external in vite.config.ts.
• Track C: Split tick() into stage helpers (drops the Agent.ts override), createAgent complexity 59 (extract per-subsystem composition), MockLlmProvider.completeSync complexity 25.
• Track D: result.ts helpers missing JSDoc, IntentionCandidate.discriminant semantic docs, SkillRegistry.invoke() JSDoc clarify throws-on-unregistered.
• Track E: Vitest coverage thresholds; tighten peer deps from * to real ranges.

Full detail with file:line refs in docs/plans/2026-04-24-codebase-review-findings.md.

Test plan

• npm run verify green locally (format/lint/typecheck/test/build/docs)
• eslint exits 0 errors / 11 ratchet warnings (was 9 errors before refactor; the extractions resolved 5 of those)
• Typedoc generates all 5 entry points with 0 errors
• Size budget unchanged
• All test suites still green
• CI green on the new docs job

https://claude.ai/code/session_01HMuHymBXZ8JqVEnpdcmnnU

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1c9971d8d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Recompute catch-up dt from current timeScale each chunk

runRestore now snapshots timeScale once (const ts = agent.getTimeScale()) and reuses it for every catch-up step. During runCatchUp, each step calls agent.tick(...), and reactive handlers/modules can legally call setTimeScale() between chunks; with a cached ts, later chunks advance the wrong virtual time (over/under-shooting elapsed offline time). Before this change, the divider read this.timeScale per chunk, so setTimeScale() applied on the next tick as documented. Please read the current scale inside the step callback instead of caching it once.

Useful? React with 👍 / 👎.

[Luis85]

Good catch — fixed in 3473b9a. The pre-refactor code read this.timeScale live both at restore entry (for the total budget) and inside the per-chunk callback (for the realDt divisor), so a reactive handler that called setTimeScale() between chunks applied on the next chunk per the documented "takes effect on the NEXT tick" semantics. My refactor cached it once and used it for both, which broke the per-chunk live-read.

Now getTimeScale() is re-read inside the step callback. The total elapsedSec budget is still computed once at restore entry — that's the correct snapshot of "how much virtual time to catch up to" and is bounded so catch-up terminates regardless of intra-chunk scale tweaks.

---

Generated by Claude Code