* Add architecture refactor plan Synthesizes a six-part architectural review into phased, gate-validated cleanup work. Targets LLM-maintainability: one home per feature, no god-files, explicit checked contracts, no copy-paste dispatch. No code changes yet — the plan only. * Refactor phase 0: dead code + isolated bugs Pure cleanup ahead of the structural phases (docs/architecture-refactor-plan.md). No behavior change except the two bug fixes, which are covered by a regression row. Dead code (all verified zero-reference or overridden): - core-resolve / core-satisfies? / core-type->str seed stubs + bindings — resolve and satisfies? are interned by install-stateful-fns! (the seed copies were shadowed); type->str was an inert SCI stub with no callers. - find defined twice in 20-coll.clj; the dead copy returned a plain vector (wrong — the live def at :787 returns a real map-entry) with a comment that contradicted it. - mark-hint (passes.clj), phs-to-struct (phm), shape-vals / ns-imports-fn (types) — unreferenced. - redundant local pad2 in javatime (module-level one already in scope). Bugs: - File.toURL stored :url but every :jolt/url method reads :spec, so a URL from (.toURL file) returned nil from all its methods. Now stores :spec (+ spec row). - pl-rest had a no-op (if (plist? r) r r); collapsed to r. - :map-shapes? was missing from the deps-image cache key — two runs differing only in map-shapes could reuse each other's image. Also dropped read-quote's unused pos param. Full gate green. --------- Co-authored-by: Yogthos <yogthos@gmail.com>
15 KiB
Architecture Refactor Plan
Goal: make the jolt codebase easier to understand and safe to change — for human maintainers and, specifically, for LLM agents. An agent should be able to find where a feature lives, see its related code, and make a change without scanning 3000-line files or keeping invisible global state in mind.
This plan synthesizes a six-part architectural review (one reviewer per subsystem). It is organized as independent, gate-validated phases, ordered by value-to-risk. Each phase is a PR-sized unit. Nothing here changes behavior; it is pure reorganization plus a small set of dead-code/bug deletions.
Non-negotiable constraints (apply to every phase)
- Every phase passes the full gate:
jpm testgreen (conformance ×3 modes,clojure-test-suite≥ baseline, bench back-to-back vs main, real exit code).rm -rf build && jpm cleanbefore trusting the binary. - Load order is load-bearing. The Janet seed (
src/jolt/*.janet) and the Clojure overlay tiers (jolt-core/clojure/core/NN-*.clj) load in a fixed order; a module may only use what loads before it. Splitting a file must preserve the net load order (a new file is imported where the old code ran). - Seed-tier discipline (the jolt-tzo rules in CLAUDE.md): nothing the analyzer/IR use may move below the kernel tier; a tier may only use macros from earlier tiers; expander-called fns stay in 00-syntax.
- One concern per PR. Do not combine a file split with a behavior fix. Dead-code/bug deletions (Phase 0) land first and separately so later diffs are pure moves.
Guiding principles (the target state)
- One obvious home per feature. Adding a
.someMethodinterop shim, a reader macro, or a clojure.core fn should have exactly one file an agent edits. - Files map to concerns, not to history. No 3000-line grab-bags; no module
whose name lies about its contents (
javatime.janetis 80% not java.time;phm.janetcontains LazySeq;types.janetholds seven concerns). - Make implicit contracts explicit and checked. The seed↔overlay split, the ctx-shaping env-knob list, and the IR op set are tribal knowledge today; turn each into a single source of truth with a drift check.
- No copy-paste dispatch. Where the same op-set / member-dispatch / cache dance is hand-written N times, extract one combinator.
Phase 0 — Dead code & concrete bugs (low risk, do first)
Pure deletions and small fixes, each with a regression row. These remove actively misleading code (comments that contradict behavior) before any moves.
| Item | Location | Note |
|---|---|---|
find defined twice; the dead copy returns a plain vector and is wrong |
jolt-core/clojure/core/20-coll.clj:347-349 |
live def at :787 returns a real map-entry; delete the dead one |
core-satisfies? always returns false (latent bug + misleading comment) |
src/jolt/core.janet:2412 |
either implement over the protocol registry or document why inert; fix the eval-list comment that claims it's an overlay fn |
File.toURL stores :url but every :jolt/url method reads :spec |
src/jolt/javatime.janet:636 |
broken shim; use :spec, add a spec row |
core-type->str — zero references |
src/jolt/core.janet:2416 |
delete defn + binding |
core-resolve — unreachable (overridden by install-stateful-fns!) |
src/jolt/core.janet:2365 |
delete defn + binding; fix stale comment |
mark-hint — unreachable |
jolt-core/jolt/passes.clj:836 |
delete |
pad2 defined twice |
src/jolt/javatime.janet:41,522 |
keep one |
phs-to-struct, shape-vals, ns-imports-fn — zero references |
phm.janet:302, types.janet:655, types.janet:527 |
delete unless reserved API |
pl-rest no-op (if (plist? r) r r) |
src/jolt/plist.janet:62 |
collapse / fix; regression check |
read-quote unused pos param |
src/jolt/reader.janet:608 |
drop |
Stale/contradictory comments (extend, orphan ;; trampoline: / ;; rand-int: headers, migration breadcrumbs) |
30-macros.clj:402-404, 20-coll.clj:558-560,780,790 |
sweep |
:map-shapes? missing from the deps-image cache key (possible stale-image correctness gap) |
src/jolt/main.janet:440-448 |
add to key; confirm with a test |
Phase 1 — Extract the host-interop subsystem (highest value)
Problem (corroborated by 3 reviewers). The JVM-emulation shim layer is the single worst sprawl, and it is exactly where the recent hiccup/markdown/malli work landed ad hoc. A single class is split across up to three files:
- registry machinery (
class-statics/tagged-methods/class-ctors+register-*!) lives inevaluator.janet:624-651; - four static tables (
Math/Thread/System/Long/Number) are hardcoded inevaluator.janet:537-619and dispatched outside the registry inresolve-sym(780-794); instance-method tables (string/number/object-methods, 698-775) are likewise inline; - the bulk of the shims are in
javatime.janet(674 lines, ~80% not java.time: java.io/util/net/nio/sql/text/math all in oneinstall-io!); api.janet:18-56wires the malli statics +set-coll-interop!as load-time side effects in the public-API module;core.janetholdsFile/JDBC constructors;types.janetholds the:jolt/instrepresentation.
Target. A src/jolt/interop/ directory, one mechanism file + one file per JDK
area, each owning a class's full surface (ctor + statics + methods + instance?
predicate + canonical name) and exposing one install!:
src/jolt/interop/
registry.janet # MOVED from evaluator: class-statics/tagged-methods/class-ctors,
# register-*!, canonical-names, value-overrides, instance? registry
coerce.janet # shared: chr, char->byte, render-piece/writer-piece, pad2,
# the date-format token walker (kills the pad2/render-piece dups)
java_time.janet java_io.janet java_net.janet java_util.janet
java_lang.janet # absorbs evaluator's inline Math/Thread/System/Long/Number +
# string/number/object method tables — the registry becomes the
# ONLY static/method-dispatch mechanism
jdbc.janet
install.janet # (install-all! ctx) — the ONE place api.janet calls (replaces api 18-56)
evaluator.janet's resolve-sym and the dot-dispatch consult only the registry.
regex.janet and async.janet stay put — they are engines/library-ports consumed
by interop, not shims.
Cheap 80% if the full split is too big for one PR: rename
javatime.janet→host_interop.janet, pull the four inline static tables + three
method tables out of evaluator.janet into it, move api.janet:18-56 into it,
dedup pad2/chr/render-piece. Collapses the scatter from 5 files to 2.
Risk: medium — touches the hot dot-dispatch path; load order must keep the
registry available before any install!. Validate with host-interop-spec +
the hiccup/markdown/malli example apps + full gate.
Phase 2 — Decompose the god-files
The three biggest interpreter/runtime files are the top LLM-navigability tax. Split along the cohesive clusters the review mapped (line ranges in the review notes). Each split is a mechanical move + import; behavior unchanged.
2a. evaluator.janet (2681 → ~5 files)
special_forms.janet— explode the 680-lineeval-list(1921-2599) into namedeval-<form>fns (eval-fn*,eval-let*,eval-try,eval-dot, …) + a dispatch table. This is the highest-leverage single change: today "where istryhandled" means scanning a 680-line body.resolution.janet—resolve-sym/resolve-var/binding/destructuring.ns_loader.janet— module/bridge plumbing + require/in-ns/use/import/refer.runtime_registration.janet— protocol/defmulti/deftype/reify setup +install-stateful-fns!(1506-1909) split into per-domain registration fns.- (host-interop already left in Phase 1.)
- De-dup the two
.methoddot arms (2456-2507 vs 2512-2550): extract onedispatch-member [target field-name args]used by both. They are copy-pasted today and must be hand-synced on every interop change.
2b. core.janet (3017 → ~6 leaf files)
Clusters are already sequential and mostly leaf — low risk:
core-types (predicates/eq/arith/bits), core-coll (assoc/seq/transducers/lazy/
transients), core-print (pr-str/str), core-io, core-refs (atoms/vars/delays/
arrays/type), core-bindings (the table + init-core! + a labeled stub
section). The core-bindings table stays the single registration point.
2c. passes.clj (1487 → 3-4 files behind one façade)
The Louvain communities and the cycle analysis agree: the IR-rewriting passes and
the type subsystem are weakly coupled (only run-passes + dirty shared).
jolt/passes.clj(keep, ~150 lines):run-passes+ shared state + re-exports — the only file the back end imports.jolt/passes/fold.clj— const-fold (always-on).jolt/passes/inline.clj— inline + flatten-lets + scalar-replace (share the alpha-rename invariant).jolt/passes/types.clj— type lattice +infer+ success-checker + driver API (kept as one module to respect the inference↔checker cycle; nodeclaregymnastics). Extract theinfer:invokearm (1051-1160) into per-shape helpers regardless of the split.- Fix the stale ns docstring (lists 4 passes, omits the type system that is
50% of the file).
Phase 3 — Kill the structural duplication
3a. One IR op-walk combinator
There are 11 hand-written recursive walks over the IR op set (const-fold,
inline, subst, body-closed?, pure?, flatten-lets, local-escapes?, subst-lookup,
scalar-replace, infer, backend emit). Adding an IR op means editing all of them,
and the "unknown ops pass through" promise is only partly kept. Introduce one
map-ir-children (in ir.clj or jolt/ir/walk.clj) that knows each op's child
positions; rewrite the walks as (map-ir-children f node) + their few specials.
Collapses ~400 lines and makes adding an op a one-site change.
3b. IR shape hygiene
:let/:loop bindings are [name init] vectors, :map pairs are [k v], :fn
arities are maps; optional keys are present-or-absent, which is why the backend
needs norm-node/phm-densification everywhere. Make ir.clj constructors always
emit optional keys (nil-valued); delete the defensive norm-node calls.
3c. Smaller dedups
read-delimiteddriver for the 4 collection readers (read-list/vec/set/kvs) inreader.janet(the skip/splice logic already drifted once).bucket-index-offor the 5 stride-2 bucket scans inphm.janet.- Unify the kw-lookup head-matching reimplemented in analyzer/passes/backend (a kw-lookup the inference tags but the backend doesn't specialize is a silent miss) behind one shared predicate + fn-name table.
Phase 4 — Config & caching coherence
4a. Lift run-mode/config resolution into config.janet
config.janet is 15 lines (one constant); meanwhile main.janet:585-630 holds
~45 lines of pure env-knob policy (open-mode?/dl/optimize?/shape/whole-program
gates) that can't be unit-tested without the CLI and that the cache keys need to
share. Promote it: config/resolve-run-mode [argv env] → {:direct-linking? :inline? :shapes? :map-shapes? :whole-program? :direct-link-auto?}, plus a canonical
ctx-shaping-env-vars list. main shrinks to: parse argv → resolve → install →
dispatch.
4b. One ctx-image module, two policies
init-cached (core image, api.janet) and the deps-image (main.janet) duplicate the
fork→validate→reinstall-print-cb→atomic-publish dance, each with a hand-built
positional %q|... cache key that silently misaligns if a knob is added.
Extract ctx_image.janet: load-image [path predicate], save-image [ctx path],
ctx-cache-key [pairs] (derived from ctx-shaping-env-vars, impossible to
misalign). Both callers differ only in the validity predicate (source-fingerprint
vs mtime-manifest). This also closes the cache-key footgun for good and resolves
aot.janet's status (fold its marshal helpers in, or delete it — it is exercised
only by one integration test and is on no run path).
Phase 5 — Data structures, reader, types, and the seed↔overlay index
5a. types.janet (699 → 3-4 files)
Seven concerns share a generic name. Split: value.janet (char/inst/uuid),
keep the cohesive var+ns+ctx core, protocols.janet (the protocol/type
registry, type-satisfies?), records.janet (shape-records). Minimum win:
extract the protocol registry + shape-records (the two least "types"-like).
5b. phm.janet (303 → 3 files)
Split out lazyseq.janet (LazySeq has nothing to do with hash maps — pure
mislabel) and phs.janet (PersistentHashSet). phm.janet keeps the map.
5c. Internal collection protocol
phm/pv/plist each re-implement count/seq/conj/predicate/meta with diverging
naming, and core.janet dispatches on them with giant per-op conds (every new
structure edits every cond). A minimal :jolt/type-keyed vtable (-count -seq -conj) lets core dispatch once. Normalize the trio's naming (pv vs pvec,
->/-to-, EMPTY vs EMPTY-PLIST).
5d. Make the seed↔overlay boundary self-documenting
Five fns exist in both the seed (core-X) and overlay (X) as dispatch twins with
no cross-reference; nothing indexes which copy is authoritative (transduce is
overlay-public but into is seed-public — surprising and undocumented). Add:
- a generated
REGISTRY(name → home → public-source → seed-twin? → dispatch-only?) with a build-time drift check; SEED-TWIN:provenance comments on each twin (greppable);- a distinct prefix for dispatch-only seed helpers so they don't read like public ones. Mirrors the existing "delete the seed defn + binding in the same change" rule.
5e. Boundary doc-comments
Add rep-vs-API pointers between the data structures and core.janet (e.g. "the
persistent vector trie lives in pv.janet; Clojure-facing vector ops and
tuple/pvec polymorphism live in core.janet"), so an agent grepping "vector" in
core knows where the representation is.
Sequencing & rationale
Phase 0 (dead code/bugs) — independent, do first, unblocks clean diffs
Phase 1 (host-interop extract) — highest value; isolates the recent shim sprawl
Phase 2 (god-file splits) — biggest navigability win; 2a/2b/2c independent
Phase 3 (op-walk + IR hygiene) — removes the largest single duplication tax
Phase 4 (config + caching) — fixes the cache-key footgun; makes boot legible
Phase 5 (data/reader/types + index) — finishes the "one home per feature" goal
Phases are independent; within a phase the sub-items (2a/2b/2c, etc.) are separate
PRs. Highest LLM-friendliness per unit risk: Phase 1, 2a (eval-list split),
3a (op-walk combinator), and 5d (seed↔overlay index).
Each PR: one concern, full gate green, no behavior change (Phase 0 deletions carry a regression row).