refactor: split cmd/server/ monolith files (store.go 7389L, routes.go 2714L, db.go 2346L) — contributor onboarding cliff

[Kpa-clawbot]

Background

This is the second time a serious contributor has bounced off the code shape. The "we're so unfriendly to contributors people are talking about a from-scratch rewrite" failure mode is a maintainer-level problem, not a style preference.

Concrete current state

cmd/server/store.go      7389 lines  ← in-memory store, every feature
cmd/server/routes.go     2714 lines  ← all HTTP handlers
cmd/server/db.go         2346 lines  ← SQLite reads + schema reads
cmd/server/types.go       991 lines  ← all wire/protocol types
cmd/ingestor/main.go     1223 lines  ← ingestor entry + handlers
cmd/ingestor/db.go        991 lines  ← ingestor inserts
total                  15,654 lines  across 6 files

For comparison, the entire cmd/server/ package would fit comfortably in 30-40 focused files of <500 lines each — typical Go service shape.

What's wrong with the current shape

1. Every recent PR (5-6 this week alone — bug(paths-through-node): 1-byte prefix collision resolves arbitrarily — attributes packets to wrong node #1352, fix(#1356): WCAG 2.2 AA map a11y — cluster bubbles, role pills, multi-byte labels #1357, bug(channels): channel view shows stale messages — latest packets hidden by dedupe keeping first-observed timestamp + insertion order #1366, fix(#1366): channels view shows latest message time — backend emits LatestSeen, not FirstSeen #1368, fix(#1370): revert ingestor envelope-timestamp path — server ingest time for packet/observation storage (counters #1233) #1372, bug(channels): /api/channels emits ghost 'unknown' bucket for encrypted-no-key packets — shows up alongside real channels after PSK add #1373) touches the same 2-3 monolith files. Merge conflicts on master are constant. Worktree-based parallel work is harder than it should be.
2. Cognitive load to make a one-line change: contributors must scroll a 7400-line file to find the right function, then keep ambient state about what else lives in that file in their head while editing.
3. No package boundaries inside the service — every function in cmd/server/store.go is in package main. There's no compiler-enforced "channels code can't reach into multibyte internals" — everything is one big lump of state.
4. AI-coding-friendly only by accident: large-context models tolerate this; humans don't. Filtering for "humans can contribute" is the higher bar.
5. Specific reviewer feedback: external contributors have publicly said it's why they're not contributing / are considering forks.

Proposed direction (high-level — design pass before any code)

Split cmd/server/store.go and cmd/server/db.go along feature axes. Suggested first cut (subject to design):

• cmd/server/store/store.go — core PacketStore struct + Load/Save lifecycle (~300 lines)
• cmd/server/store/channels.go — GetChannels, GetChannelMessages, channel-list aggregation (~600 lines)
• cmd/server/store/multibyte.go — computeMultiByteCapability, hash-size analytics (~500 lines)
• cmd/server/store/paths.go — nodeInResolvedPath, paths-through-node, path-hop index (~800 lines)
• cmd/server/store/analytics.go — GetAnalyticsChannels, payload-mix, RF analytics (~700 lines)
• cmd/server/store/topology.go — neighbor-graph build, topology, edges (~600 lines)
• cmd/server/store/observers.go — observer registry, region resolution (~400 lines)
• cmd/server/store/disambig.go — resolveWithContext, buildPrefixMap, fallback resolution (~500 lines)
• cmd/server/store/dedup.go — broadcastTxs, dedupe key logic, cache invalidation (~400 lines)
• residual catch-all for genuinely shared helpers (~500 lines)

Same axis for routes.go → handlers/<domain>.go (channels, analytics, nodes, packets, live, multibyte, paths, …).

For db.go (DB-backed sibling), mirror the same split.

Optional: introduce a internal/store/ package to make the splits compiler-enforced (cross-domain calls become explicit imports).

Acceptance criteria (design phase only)

This issue is a DESIGN issue — file PR(s) only after the design lands here in comments.

• A design comment lands on this issue with: (a) the proposed file split (names + responsibilities), (b) the proposed package boundary (single package main vs internal/store/), (c) the migration ordering (which files split first, what's a single PR vs a series), (d) the merge-conflict mitigation plan (announce + freeze window for in-flight branches).
• Expert review on the design (Munger for architecture, Torvalds for the "is this actually simpler" check) BEFORE implementation.
• Implementation PR series cleared with the project, in order:
  • PR 1: tooling only — add a make refactor-precheck (or equivalent) script that asserts git mv-based moves preserve go test ./... -count=1 byte-equality on test output. No code moves yet.
  • PR 2: extract one domain (suggest: channels.go) using only git mv of function blocks. CI green proof. Zero behavior change.
  • PR 3+: subsequent domain extractions, one per PR, each behind a CI green gate.
• Each refactor PR MUST be pure — no behavior changes piggybacked. Reviewed strictly under AGENTS.md's "pure refactor" exemption: existing tests byte-unchanged + still green.
• After refactor: file count goes from ~6 monoliths to ~30-40 focused files; no single file in cmd/server/ exceeds 1000 lines.

Out of scope

• Rewriting the PacketStore API surface.
• Switching to a different language / framework.
• Splitting the binary into multiple processes.
• Touching cmd/ingestor/ in the first refactor wave (file separately if needed).

Risks

• Massive merge conflicts for in-flight branches. Mitigation: pick a freeze window, announce, do the first split fast.
• Loss of git blame lineage. Mitigation: every move uses git mv (preserves blame across renames) — never copy+delete.
• Reviewer fatigue: 10+ refactor PRs is a lot. Mitigation: make each PR a single domain, tag clearly, batch reviews.

Refs

• External contributor feedback relayed via Discord on 2026-05-22 (translated above).
• cmd/server/store.go 7389 lines (master HEAD).
• cmd/server/routes.go 2714 lines.
• AGENTS.md "Pure refactor" exemption rule — defines the test-byte-unchanged gate.

[Kpa-clawbot]

Torvalds code-quality review

Worth doing. Don't screw it up by turning it into an abstraction festival.

1. Does this reduce complexity, or just move it?

Mostly reduces — if you keep package main. The functions in cmd/server/store.go are already weakly coupled: most operate on *PacketStore and a handful of cached maps. Moving computeAnalyticsRF out of the file does not require touching GetChannels. There is no hidden cross-talk that file-splitting will turn into a glue layer. That is the test, and this passes it.

It moves complexity (and adds it) the moment you do the optional internal/store/ package. That forces every helper currently used by two domains (buildPrefixMap, pickBestObservation, txToMap, parsePathJSON, haversineKm) into either (a) an exported API on the package or (b) a duplicated copy or (c) a third "shared" package. All three are worse than the status quo. Recommendation: do NOT introduce internal/store/ in the first pass. Keep package main, split into files only. If you later discover a domain that genuinely has zero shared helpers, then extract that one into its own package. Compiler-enforced boundaries are not free; you pay for them with API surface.

2. The "good taste" check — three functions that clearly don't belong in store.go

By name, with line numbers (file is now 9287 lines, not the 7389 cited in the issue — it grew during the discussion, which is itself a data point):

• computeMultiByteCapability (L8031) — 165 lines of protocol capability analytics. Reads MultiByteCapEntry. Nothing else in the file calls it except its public wrapper. Belongs in multibyte.go. Done.
• computeHashCollisions (L7535, ~288 lines) — hash-collision detection. Self-contained, single caller, owns its own data structures. Belongs in hash_collisions.go (or fold into hash_sizes.go).
• computeAnalyticsTopology (L6226, ~518 lines) — neighbor-graph aggregation. Already lives next to neighbor_graph.go which is its own file. Why is the analytics sitting in store.go when the graph it consumes is already extracted? Move it.

That's your proof. Three functions, ~970 lines, zero cross-domain entanglement, each one a clean git mv of a function block. If the first PR can't do this in 30 minutes, the design is wrong.

3. Hard cap on file size?

Yes. 1500 lines, soft. 2000 lines, hard.

Not 500 — that's cargo-culted from microservice fashion and will produce 80 files where 30 was right. Go files routinely run 1000-1500 lines without pain when the contents are cohesive (see runtime/mgc.go, cmd/compile/internal/ssa/rewrite*.go). The number you want is "small enough that a contributor doesn't dread opening it." 1500 is the lived-experience threshold; 2000 is where it becomes hostile.

Enforcement: preflight gate, not a lint rule. A lint rule will get bypassed with //nolint once and stay bypassed forever. A preflight check that fails with cmd/server/foo.go: 2147 lines exceeds 2000 hard cap and prints the three function names that pushed it over the line will force the conversation. Put it in make refactor-precheck and run it in CI on every PR. AGENTS.md gets a one-line rule: "No file in cmd/server/ may exceed 2000 lines without explicit waiver in the PR body."

4. The thing the implementer will be tempted to do that I will reject

type ChannelService interface { GetChannels(...); GetChannelMessages(...) } with one implementation. "For testability." No.

You don't need an interface to test GetChannels. You need a PacketStore with the relevant maps populated. That's already how every existing test works. An interface here exists to make the author feel like they did "real architecture" — it adds an indirection, a vtable lookup, an import to manage, a fake-implementation file in tests, and not one tested behavior that wasn't already testable. Reject on sight.

Related rejections: a Service struct per domain (ChannelService, TopologyService, etc.) whose only field is *PacketStore. That's not encapsulation, that's a rename with extra steps. Methods stay on *PacketStore; files change; struct doesn't.

5. Bare minimum the first refactor PR has to prove

One PR. One domain. git mv of function blocks only. Specifically:

• Extract computeMultiByteCapability and its two helpers into cmd/server/multibyte.go.
• Show git log --follow --oneline cmd/server/multibyte.go traces back through store.go (proves git mv was used, blame is preserved).
• Show go test ./... -count=1 output is byte-identical to master. Diff the test output files. Paste the empty diff in the PR body.
• Show go vet ./... clean.
• Show git diff master -- cmd/server/store.go cmd/server/multibyte.go is pure motion: every removed line in store.go appears, character-for-character, in multibyte.go. No reformatting, no rename, no "while I'm here" cleanup.

If that PR can't be reviewed in under 10 minutes, the pattern is wrong. If it can, the next 8 PRs are templates of this one and the review burden is per-PR ~5 minutes.

6. Is this the right problem?

Mostly yes. But push back on one thing.

The Discord quote was "AI slop." That word is doing more work than "file is long." AI-generated Go often has other tells that no amount of file-splitting fixes:

• map[string]interface{} everywhere instead of typed structs (this codebase has it badly — see the return type of half the functions above).
• Helpers named txToMap, txToMapWithRP, enrichObs — verbs glued to nouns with no domain meaning. Renaming the file these live in doesn't fix the names.
• Functions that take 7+ parameters because nobody wanted to define a struct (see pm.resolveWithContext).
• Copy-paste between GetAnalyticsChannels / GetAnalyticsRF / GetAnalyticsTopology / GetAnalyticsDistance / GetAnalyticsHashSizes — they all follow the same Get → GetWithWindow → compute triplet pattern. Is that a real abstraction or four people copy-pasting?

The file split is necessary but not sufficient. Land it because constant merge conflicts on a 9000-line file is a real, measurable cost. But the next issue after this one needs to be "stop returning map[string]interface{} from analytics functions, define result types." That's the actual fix for the "AI slop" perception, and it's a bigger lift than moving lines between files.

Verdict

Do the split. Files only, no package boundary. Pick multibyte.go first because it's the cleanest demonstration. Hard cap at 2000 lines in a preflight gate. Reject any PR that introduces an interface "for testability" or a Service wrapper struct. Open a follow-up issue for the map[string]interface{} problem before you forget it exists.

[Kpa-clawbot]

Munger architecture review

"All I want to know is where I'm going to die, so I'll never go there."

The proposed direction is sound. The problem is real (15K lines across 3 files is an onboarding cliff, not a style preference). My job here is not to ratify the happy path — it is to mark the cliffs the implementer will walk off if no one points at them.

0. Concrete state I'm reviewing against

origin/master HEAD, today:

file	lines	func decls
cmd/server/store.go	9287	139
cmd/server/routes.go	3359	81
cmd/server/db.go	2737	59

The issue body cites 7389 / 2714 / 2346 — those numbers were already stale by ~2000 lines when the issue was opened. That alone is evidence the bleeding is fast and the freeze-window risk is real.

---

1. Domain boundary list — my version

The criterion I'm applying: "when a single feature changes, what set of functions changes together, and what set is guaranteed not to?" Cohesion is measured by co-change, not by topical similarity.

I ratify ~80% of the proposed split and adjust the rest. Concrete function-level groupings from store.go:

domain	core funcs (line numbers in current store.go)	rough loc
store/core.go	NewPacketStore, Load, loadChunk, loadBackgroundChunks, MaxTransmissionID, MaxObservationID, helpers pathLen, cacheTTLSec	~600
store/ingest.go	IngestNewFromDB (1996), IngestNewObservations (2405), trackAdvertPubkey, untrackAdvertPubkey, pickBestObservation	~900
store/index.go	indexByNode, addToByNode, buildSubpathIndex (3434), buildPathHopIndex (3449), addTxToPathHopIndex, removeTxFromPathHopIndex, addTxToRelayTimeIndex, addTxToSubpathIndexFull, removeTxFromSubpathIndexFull, buildDistanceIndex (3681), updateDistanceIndexForTxs	~900
store/eviction.go	estimateStoreTxBytes, estimateStoreObsBytes, estimatedMemoryMB, trackedMemoryMB, evictionCandidateTxIDs, EvictStale, evictStaleInternal, RunEviction, StartEvictionTicker	~600
store/query.go	QueryPackets, QueryGroupedPackets, QueryMultiNodePackets, filterPackets, pagePacketResult, groupedTxsToPage, GetTransmissionByID, GetPacketByHash, GetPacketByID, GetTimestamps	~900
store/channels.go	GetChannels (4435), GetEncryptedChannels (4599), GetChannelMessages (4677), packetMatchesChannel, channelNameMatchesHash, isPlaceholderName	~700
store/multibyte.go	GetAnalyticsHashSizes (7098), computeAnalyticsHashSizesWithCapability, computeAnalyticsHashSizes, GetAnalyticsHashCollisions, computeHashCollisions, GetNodeHashSizeInfo, computeNodeHashSizeInfo, GetMultiByteCapMap, computeMultiByteCapability, EnrichNodeWithHashSize, EnrichNodeWithMultiByte	~1500
store/paths.go	GetAnalyticsSubpaths, GetAnalyticsSubpathsBulk, computeAnalyticsSubpaths, computeSubpathsSlow, rankSubpaths, GetSubpathDetail, GetAnalyticsDistance, computeAnalyticsDistance, computeDistancesForTx, subpath/distance index helpers	~1500
store/topology.go	GetAnalyticsTopology, GetAnalyticsTopologyWithWindow, computeAnalyticsTopology, NeighborGraph glue	~700
store/rf.go	GetAnalyticsRF, computeAnalyticsRF	~700
store/disambig.go	buildPrefixMap, prefixMap.resolve, resolveWithContext, buildHopContextPubkeys, buildAggregateHopContextPubkeys, hopResolverPerTx, betterByObsCount, getCachedNodesAndPM, InvalidateNodeCache	~800
store/regions.go	resolveRegionObservers, fetchAndCacheRegionObs, resolveAreaNodes, iataMatchesRegion, computeNodeHomeRegions, transmissionsForObserver	~400
store/nodehealth.go	GetBulkHealth, GetNodeHealth, GetNodeAnalytics	~700
store/cache.go	invalidateCachesFor, applyCacheInvalidation, invalidateRelayStatsCache, GetCacheStats, GetCacheStatsTyped	~300
store/stats.go	GetStoreStats, GetPerfStoreStats, GetPerfStoreStatsTyped	~400
store/util.go	txToMap, txToMapWithRP, enrichObs, strOrNil, intPtrOrNil, normalizeTimestamp, hasGarbageChars, countNonPrintable, isHexLower, hasUpperASCII, parsePathJSON, haversineKm, geoDistApprox, txGetParsedPath, resolvePayloadTypeName	~400

Deltas vs the issue's split:

• Add store/ingest.go. IngestNewFromDB (~400 lines) + IngestNewObservations (~300 lines) are the hottest co-change surface on the repo. Burying them inside "core" is the mistake.
• Add store/eviction.go. Memory eviction is its own concern with its own failure mode (OOM) and its own tickers. Co-changes almost never overlap with channels/multibyte/paths.
• Add store/index.go. All buildXxxIndex / addTxToXxxIndex / removeTxFromXxxIndex co-change together when index shape changes, and almost never with the consumers.
• Add store/nodehealth.go. GetBulkHealth + GetNodeHealth + GetNodeAnalytics are three of the largest funcs in the file and they co-change as a unit.
• paths absorbs distance. Distance index, subpath index, and nodeInResolvedPath change together — distance is derived from paths, not a separate domain.
• util.go is a known wart, but make it small and frozen. ~15 helpers, mostly type coercions. If util.go grows past 500 lines it has stopped being util and you've recreated the monolith. That's a CI-checked cap (see §6).

Mirror the same axes in routes.go → handlers/{channels,multibyte,paths,topology,rf,nodes,packets,observers,analytics,config,health,traces}.go and in db.go → db/{channels,nodes,packets,observers,traces,schema}.go. The route-handler axis and the store-domain axis already line up — every handleAnalyticsX calls exactly one store.GetAnalyticsX. Use that.

---

2. Inversion check — the worst split that still looks correct

The worst split is "by HTTP route shape" (one file per /api/analytics/* endpoint). It looks tidy on routes.go, it satisfies "no file > 1000 lines", and it is wrong. Here's why:

• It cuts orthogonal to co-change. handleAnalyticsHashSizes + handleAnalyticsHashCollisions + the bulk-health hash-size enrichment all change together when multibyte-capability detection changes — they live in three different routes but one domain.
• It guarantees the next refactor. Six months from now someone notices the same multibyte logic in 4 route files and asks for another refactor.
• It pushes complexity into the store layer instead of fixing it.

Other anti-patterns to name explicitly so no one drifts:

• Split by file size. "Chop store.go in half at line 4500." Trivially achieves the line-count target, destroys cohesion, makes every future PR worse.
• Split by struct-receiver-vs-free-function. Tempting because it's mechanical. Useless because both groups span every domain.
• Split by "what feels logically grouped" without checking co-change history. git log --follow --pretty=format: --name-only -- cmd/server/store.go | sort | uniq -c is the empirical check. Do it before locking the split.
• Premature interface extraction. Splitting PacketStore into ChannelStore interface { ... } + MultiByteStore interface { ... } to "decouple" them. There is one implementation. Interfaces with one implementation are a tax with no benefit. (See §6 hard NO.)

---

3. Single-package cmd/server/store/*.go vs multi-package internal/store/<domain>/

Pick single-package: cmd/server/store/ with one package store across all files. Defer the multi-package option until a domain has demonstrably stabilized.

Rationale:

• The compiler-enforced boundary the multi-package option promises is illusory at this stage. Cross-domain calls already exist and are correct (multibyte enriches node health; topology consumes the disambig prefix map; paths consumes the distance index). Forcing those into exported APIs immediately means inventing 30+ public method signatures with no second consumer to validate them. That is the textbook "man with a hammer" — solving a future problem you have not seen with the only tool you know.
• The win the issue actually asks for is "humans can find code." That win is captured 95% by file-level split alone. The remaining 5% (compiler-enforced isolation) is not worth blocking the 95%.
• Reversibility is asymmetric. Single-package → multi-package later is a git mv + add package foo + add exports. Multi-package → single-package later is hard because external callers may have started depending on the boundary. Pick the cheaper-to-reverse option now.
• Munger on premature abstraction applies here, full stop. "All internal" is not the lower-cost option until you've seen the real boundaries hold up under at least two months of real PRs. Until then it's architecture theater.

Revisit in 6 months: if store/multibyte.go has zero churn that requires reaching outside the multibyte-prefixed funcs, that is the signal to promote it to internal/store/multibyte/. Not before.

---

4. Migration ordering and risk-ranking

First extract: store/channels.go. Reasons (in the order they matter):

1. Smallest blast radius. Channels-domain funcs are largely self-contained — GetChannels, GetEncryptedChannels, GetChannelMessages, plus the small predicates packetMatchesChannel / channelNameMatchesHash / isPlaceholderName. The only outward dependency is reading s.byChannel and the cache. No outbound calls into multibyte/paths/topology.
2. Cleanest seam. All five functions sit in a contiguous block (store.go lines 4435–4935 modulo two predicates that pull up cleanly).
3. Best demo of the pattern. Channels is a domain every reviewer mentally recognizes ("channel messages, channel list"). If the pattern reads well for channels, it reads well for everything else. If it reads badly for channels, halt — the pattern is wrong, not the domain.
4. Lowest concurrent-PR-conflict risk. Recent PR activity (per the issue's list) clusters around topology/multibyte/observers, not channels. Channels has been quiet. Extracting a quiet domain first minimizes the freeze-window pain.

Next three, in order:

order	domain	risk	why this rank
2	store/eviction.go	LOW	Self-contained, no consumers outside RunEviction. Pure mechanical move. Validates the pattern survives a domain with tickers/goroutines.
3	store/multibyte.go	MEDIUM	~1500 lines, three public entry points, but consumers (enrichNodeWithMultiByte) are well-defined. Risk = test coverage for hash-size analytics is the densest in the repo; any move that breaks go test ./... will be loud. That's a feature, not a bug.
4	store/paths.go	HIGH	Largest domain, most cross-cutting (consumed by topology, distance, node health). Do it 4th, not later — the discomfort of carrying paths-in-monolith while everything else is split is what creates "good enough, leave it" inertia. The HIGH risk is exactly why you don't leave it for last; do it while attention is high.

Defer to the back half: topology, rf, disambig, regions, index, ingest, nodehealth, util, cache, stats. routes.go and db.go splits happen in parallel as separate PR tracks once channels proves the pattern across all three files (so the channels extract is actually a 3-PR mini-series: store/channels.go, handlers/channels.go, db/channels.go).

---

5. Failure modes the design MUST prevent

Each failure mode + the specific mechanism:

1. Blame lineage loss.
   Mechanism: every move uses git mv of the FILE, not function-block copy-paste. When extracting a subset of store.go, the technique is: git mv store.go store/channels.go, then in the SAME commit move the non-channels code back into a new store/core.go. git log --follow then preserves history for both. Verify with git log --follow --oneline cmd/server/store/channels.go | head -20 showing pre-refactor commits.

2. In-flight branch conflict storm.
   Mechanism: publish a freeze window in a pinned issue at least 48h before the first extract PR opens. The first extract PR (channels) MUST merge in ≤4 hours of opening, and the next extract MUST NOT open until the previous is merged. No parallel extract PRs. This sounds slow; it is the only thing that prevents week-long rebase hell on every open feature branch.

3. Circular import after split.
   Mechanism: only relevant if §3 flips to multi-package. While single-package, this failure mode does not exist — explicitly call that out in the design doc as a reason single-package is the safer first step.

4. Test fixture coupling / "tests still compile but no longer test the thing".
   Mechanism: enforce the "pure refactor" gate per AGENTS.md. Test files MUST be byte-unchanged across each extract PR (git diff --stat origin/master -- '*_test.go' returns empty). If a test file must change, the PR is not a pure refactor and must be rejected and re-split.

5. "Refactor PR also fixes a bug" piggyback.
   Mechanism: PR template gate. The refactor PR description MUST contain the exact line "Behavior change: none. Tests byte-unchanged." If reviewer cannot verify both by git diff alone, send it back. No exceptions for "trivial cleanup while I'm here."

6. Hidden cross-domain dependency surfaces during extract.
   Mechanism: before extracting a domain, run grep -nE 'func \(s \*PacketStore\)' cmd/server/store.go and produce a per-domain dependency matrix (which domain's funcs call which other domain's funcs). If the matrix shows a domain with >5 inbound cross-domain calls, that domain is mis-cut — re-split before any code moves.

7. File-size cap regression.
   Mechanism: add a CI check (scripts/check-file-sizes.sh) that fails if any file in cmd/server/ exceeds 1000 lines. Without enforcement, monolith re-formation is the default outcome over 12 months. (Predictable incentive: PRs take the path of least resistance, which is "append to the file you opened.")

---

6. Hard NO list

1. MUST NOT introduce interfaces to "decouple" domains. One implementation = no interface. If a future need surfaces, add the interface then. Speculative interfaces are how Go services rot into Java.
2. MUST NOT rename functions during the refactor. Move only. Renames go in a follow-up PR after the move is merged. This protects git log --follow and makes review trivial.
3. MUST NOT introduce new abstractions (helper structs, builder patterns, generics, options structs) during the refactor. Pure mechanical moves only.
4. MUST NOT split based on what's easy to extract; split based on the dependency matrix from §5 mechanism 6. "Channels was easy, let's also pull out these 50 random funcs while we're in there" is the path to a worse end state.
5. MUST NOT touch cmd/ingestor/ in this refactor wave. Issue says out of scope; honor it. Mixing scopes is the Feed panel overflow:hidden silently clips items instead of scrolling #1 way refactors become unreviewable.
6. MUST NOT skip the precheck-script PR. PR 1 (refactor-precheck tooling) exists because every subsequent PR depends on its assertions. Skipping it means the "tests byte-unchanged" gate is enforced by reviewer attention instead of CI, which is the same as not enforced.
7. MUST NOT open parallel extract PRs. One in flight at a time, each merged inside its declared freeze window.

---

TL;DR

Ratify the issue's split with these adjustments: add store/ingest.go, store/eviction.go, store/index.go, store/nodehealth.go; absorb distance into paths. Use single-package cmd/server/store/ not internal/store/<domain>/ — the compiler-enforced boundary is premature abstraction at this stage and is cheaply added later if a domain stabilizes. Extract channels first (smallest blast radius, cleanest seam, quietest recent PR activity), gated by a CI file-size cap and a hard "pure refactor: tests byte-unchanged" rule.

The cliff to not walk off: the worst-looking-correct alternative is splitting by HTTP route shape or by file-size midpoint. Both satisfy the surface metric and destroy cohesion. Don't go there.

[Kpa-clawbot]

Locked design — synthesis of Munger + Torvalds reviews

Both reviewers independently converge on the same shape; this comment locks it in so implementation PRs can reference one source of truth.

Agreed points

1. Do the split. Both reviewers ratify the issue's premise — the cross-file coupling in cmd/server/store.go is genuinely weak (not architecturally tangled), so a file-axis split reduces complexity rather than moving it. The 9287-line current state (drifted from 7389 cited in the issue body — Munger caught the stale count) is past the point where one-file-per-domain stops paying off.

2. Single-package, file-only split. Both explicitly reject internal/store/<domain>/ multi-package boundaries. Rationale: premature abstraction with one consumer, expensive to undo, cheap to add later if any single domain stabilizes enough to deserve it. Use cmd/server/store/<domain>.go (still package main).

3. First extract = channels (smallest blast radius, cleanest seam) — but Torvalds counter-proposes computeMultiByteCapability as a self-contained alternative. Either is defensible; locking in channels because the recent PR storm has heavily exercised it (bug(channels): channel view shows stale messages — latest packets hidden by dedupe keeping first-observed timestamp + insertion order #1366, fix(#1366): channels view shows latest message time — backend emits LatestSeen, not FirstSeen #1368, fix(#1370): revert ingestor envelope-timestamp path — server ingest time for packet/observation storage (counters #1233) #1372) so we know its surface and the seams are sharp.

4. Hard size cap with preflight gate. Munger: <1000 LOC per file enforced by CI. Torvalds: 2000 LOC cap via preflight (not lint). Locking in 1500 LOC per file as a hard preflight gate — splits the difference, generous enough not to false-positive on legitimate large files, tight enough to drive future splits.

5. Pure-refactor discipline (mandatory). AGENTS.md "pure refactor" exemption applies — existing tests byte-unchanged AND still green. Verified by an explicit make refactor-precheck script (or equivalent) that:

   • asserts git diff --name-only origin/master touches NO test files,
   • runs go test ./... -count=1 and asserts byte-equal output vs baseline.

Hard NO list (consolidated from both)

• ❌ NO internal/store/<domain>/ multi-package split (premature).
• ❌ NO Service wrapper struct per domain (over-architecture).
• ❌ NO one-implementation interfaces "for testability" (over-architecture).
• ❌ NO behavior changes piggybacked on refactor PRs (violates AGENTS.md exemption).
• ❌ NO copy+delete (use git mv so blame survives).
• ❌ NO splitting by HTTP-route shape (split by data domain — that's what Munger's inversion check flagged).
• ❌ NO refactor PRs racing with in-flight feature branches (announce + freeze window).

Domain split (locked v1)

From Munger's file-by-file grep (139 funcs in store.go, 81 in routes.go, 59 in db.go), the v1 split targets:

File	Approx lines	Anchor functions
store/store.go	~300	PacketStore struct, Load, Save, lifecycle
store/channels.go	~700	GetChannels, GetChannelMessages, channel-list aggregation
store/multibyte.go	~600	computeMultiByteCapability, hash-size analytics
store/paths.go	~900	nodeInResolvedPath, paths-through-node, path-hop index + distance
store/analytics.go	~800	GetAnalyticsChannels, payload-mix, RF analytics
store/topology.go	~700	neighbor-graph build, topology, edges
store/observers.go	~400	observer registry, region resolution
store/disambig.go	~500	resolveWithContext, buildPrefixMap
store/dedup.go	~400	broadcastTxs, dedupe keys, cache invalidation
store/ingest.go	~400	in-memory append path
store/eviction.go	~300	retention, prune, eviction policy
store/index.go	~400	byPathHop, byPayloadType, etc.
store/nodehealth.go	~400	bulk-health, per-node health computation
residual catch-all	~500	genuinely-shared helpers only

Same axis for routes.go → handlers/<domain>.go.

Migration ordering (locked)

Wave	Scope	Risk
PR-1	Tooling only — scripts/refactor-precheck.sh + Makefile target + preflight integration	Low
PR-2	Extract store/channels.go (pure git mv of channel functions) — proves the pattern	Low
PR-3	Extract store/multibyte.go	Low (Torvalds' suggested seed)
PR-4	Extract store/observers.go, store/eviction.go (smallest remaining)	Low
PR-5	Extract store/disambig.go, store/index.go	Medium (touched by recent disambiguator work)
PR-6	Extract store/paths.go (largest single chunk)	Medium
PR-7	Extract store/analytics.go, store/topology.go	Medium
PR-8	Extract store/nodehealth.go, store/dedup.go, store/ingest.go	Low
PR-9	Mirror split on cmd/server/routes.go → handlers/<domain>.go	Medium
PR-10	Mirror split on cmd/server/db.go	Medium

Each PR's CI must be green before the next is opened. No racing.

Mitigation for in-flight branches

Before PR-2 lands, announce a 6-hour freeze window in the repo (Discord + a pinned GitHub issue note). During the window: no merges to master that touch cmd/server/store.go. After PR-2 lands, in-flight branches rebase against the new file layout — git mv'd functions retain blame, so rebases are mechanical for unmodified hunks and only manual for branches that genuinely modified the moved functions.

Out of scope (file separately if needed)

• The deeper "AI slop" tells Torvalds flagged: map[string]interface{} everywhere, weak typing on response shapes. File-splitting won't fix those. Separate issue.
• cmd/ingestor/ refactor — not in v1.
• Frontend refactor — not in v1.
• Performance optimization — not in v1.

Next step

Open PR-1 (tooling) as soon as the in-flight feature PRs (#1361/#1367/#1373/#1374/#1375) land and the announce window has passed. PR-2 (channels extract) immediately after PR-1 merges.

[Kpa-clawbot]

Go-style enforcer baseline audit (cmd/server/)

Mode: report-only. No code touched. Source of truth: cached mirror of Google Go Style Guide / Decisions / Best Practices under ~/.openclaw/skills/go-style-enforcer/references/. Audited 19 non-test .go files, 22,458 LOC. gofmt-clean assumed; this audit only flags what gofmt does not catch.

Summary

Category	Findings	Must-fix	Suggest
Naming (GetFoo accessors, package main sprawl)	70	70	0
Comments / doc	131	131	0
Errors (%w, failed: prefix, discarded errs)	19	1	18
Concurrency (ctx plumbing)	1 (architectural)	0	1
Tests (got vs want order)	32	0	32
Package structure (one giant package main)	1 (architectural)	1	0
Style decisions (else if chains, naked returns)	35	0	35
AI-slop antipatterns (map[string]interface{}, bare interface{})	619	619	0
Total	908	822	86

Top 10 highest-frequency rule violations

#	Rule	Count	Canonical URL
1	map[string]interface{} instead of any AND/OR a real typed struct	394	https://google.github.io/styleguide/go/decisions#use-any
2	Bare interface{} (non-map) in new code instead of any	225	https://google.github.io/styleguide/go/decisions#use-any
3	Exported type missing required doc comment	104	https://google.github.io/styleguide/go/decisions#doc-comments
4	GetFoo() accessor on idiomatic struct	70	https://google.github.io/styleguide/go/decisions#getters
5	} else if … chains that should be switch	33	https://google.github.io/styleguide/go/decisions#switch-and-break
6	Exported func/method missing doc comment	27	https://google.github.io/styleguide/go/decisions#doc-comments
7	Test failure printed as want, got instead of got, want	32	https://google.github.io/styleguide/go/decisions#got-before-want
8	Discarded error via _ with no explanatory comment	12	https://google.github.io/styleguide/go/decisions#handle-errors
9	Naked return in long function	2	https://google.github.io/styleguide/go/decisions#named-result-parameters
10	Wrap with "…failed: %w" carrying no info beyond the wrapped err	1	https://google.github.io/styleguide/go/best-practices#error-extra-info

Per-file scoreboard

File	LOC	must-fix	suggest	Top-3 violations
store.go	9287	269	19	map[string]interface{}×210; interface{}×27; GetFoo×30
routes.go	3359	78	10	map[string]interface{}×57; interface{}×18; missing doc×3
db.go	2737	133	4	map[string]interface{}×66; GetFoo×34; interface{}×31
types.go	991	245	0	interface{} field×139; missing type-doc×90; map[string]interface{}×16
clock_skew.go	748	7	1	GetFoo×3; map[string]interface{}×3; missing doc×1
neighbor_persist.go	674	0	0	(clean — strongest file in the package)
neighbor_graph.go	633	1	2	interface{}×1; else if×2
decoder.go	597	0	0	(clean)
main.go	515	0	1	else if×1
neighbor_api.go	478	6	1	missing type-doc×6; else if×1
resolved_index.go	475	1	0	interface{}×1
config.go	424	29	0	map[string]interface{}×19; missing doc×10; GetFoo×3
neighbor_debug.go	399	4	1	missing type-doc×4; else if×1
openapi.go	359	25	0	map[string]interface{}×22; interface{}×3
websocket.go	273	8	0	missing doc×7; interface{}×1; map[string]interface{}×1
compress.go	224	4	0	missing doc×3; interface{}×1
memory.go	132	0	0	(clean)
hash_migrate.go	119	0	0	(clean)
geo_filter.go	34	3	0	interface{}×3

Three files account for ~80% of must-fix volume: store.go, types.go, routes.go. This is the same set issue #1382 targets for splitting.

Top 20 priority findings (fix-impact ordered)

1. types.go:252-279,… — interface{} fields on the canonical typed response structs
   rule: https://google.github.io/styleguide/go/decisions#use-any
   severity: must-fix
   quote: "For new code, use the any alias … interface{} … is no longer preferred."
   why: TransmissionResp.RawHex/RouteType/PayloadType/SNR/RSSI/PathJSON/... are declared interface{} even though the file's own header comment says "Every API response is a typed struct — no map[string]interface{}". This is the canonical example the skill flags.
   fix: replace interface{} with concrete types (*string, *float64, json.RawMessage, []byte). 139 occurrences here alone.

2. db.go:315 — GetDBSizeStats() map[string]interface{} shadowed by GetDBSizeStatsTyped() SqliteStats two lines below
   rule: https://google.github.io/styleguide/go/decisions#getters
   severity: must-fix
   why: Both the Get prefix and the map[string]interface{} return violate two rules in one signature; the existence of …Typed proves a typed shape is known.
   fix: rename Stats()/delete the untyped variant.

3. store.go:1613,1669,1851,1863,1875,1887,4435,4677,4872,5149,6181,6758,7098,7464,8196,8377,8874,8897,9156 — 30× (s *PacketStore) GetX() map[string]interface{}
   rule: https://google.github.io/styleguide/go/decisions#getters + #use-any
   severity: must-fix
   why: Every analytics endpoint returns a map keyed by stringly-typed field names; this is the exact AI-slop antipattern listed in SKILL.md §8 row 1.
   fix: extract response structs (paired with the typed variants already present, e.g. GetCacheStatsTyped at store.go:1692 — model the rest the same way).

4. routes.go × 57 map[string]interface{} literals built from string keys for json.NewEncoder.Encode
   rule: https://google.github.io/styleguide/go/decisions#use-any (and intent: typed responses per types.go header)
   severity: must-fix
   fix: project-wide response struct discipline; this is the body of refactor: drive map[string]interface{} out of API response layer + internal data (694 occurrences in cmd/) — typed structs, codegen for frontend #1383.

5. db.go:824,950,990,1009,1306,1355,… — Get…() ([]map[string]interface{}, …) query helpers (34 total)
   rule: https://google.github.io/styleguide/go/decisions#getters + #use-any
   severity: must-fix
   why: Each row decoded from SQLite is hand-rolled into a string-keyed map instead of into a row struct.
   fix: row structs per query (matches existing Stats, Observer, MetricsSample patterns).

6. config.go:292,317,326,386 — GetHealthThresholds, GetHealthMs, ToClientMs, GetTimestampConfig
   rule: https://google.github.io/styleguide/go/decisions#getters
   severity: must-fix
   fix: drop Get prefix; idiomatic Go uses noun-named accessors.

7. Every file: missing package comment
   rule: https://google.github.io/styleguide/go/decisions#package-comments
   severity: must-fix (one occurrence; satisfied by adding one)
   quote: "Each package should have a package comment introducing the package. … Command corescope …"
   why: 0/19 files carry a package doc. For package main, the doc must begin Command <name> or Binary <name>.
   fix: add // Command corescope serves the CoreScope API. immediately above package main in main.go.

8. types.go × 90 exported types with no preceding doc comment
   rule: https://google.github.io/styleguide/go/decisions#doc-comments
   severity: must-fix
   why: TransmissionResp, StatsResponse, HealthResponse, etc. are part of the public API surface; godoc renders them.
   fix: 1-line // TypeName describes … per type. Bulk-mechanical.

9. neighbor_api.go:22-68, neighbor_debug.go:21-63, config.go:159,190,225 — 13 exported types missing doc
   rule: same as Global window._xxx functions never cleaned up in destroy() #8
   severity: must-fix
   fix: as above.

10. websocket.go:39,45,… — 7 exported Hub methods missing doc (ClientCount, Register, Unregister, Broadcast, …)
    rule: https://google.github.io/styleguide/go/decisions#doc-comments
    severity: must-fix
    fix: 1-line method docs.

11. db.go:51 — fmt.Errorf("ping failed: %w", err)
    rule: https://google.github.io/styleguide/go/best-practices#error-extra-info
    severity: must-fix
    quote: "Don't add an annotation whose only job is to say 'failed' — just return err."
    fix: return nil, err, OR replace "failed" with the operation context ("opening sqlite at %q: %w").

12. db.go:1751,1752 — text, _ := decoded["text"].(string); sender, _ := decoded["sender"].(string)
    rule: https://google.github.io/styleguide/go/decisions#handle-errors
    severity: suggest
    why: silently discarding type-assertion ok values masks ingest bugs.
    fix: handle the bool, or comment why discard is safe.

13. store.go:18 else if chains (concentrated in analytics ranking funcs)
    rule: https://google.github.io/styleguide/go/decisions#switch-and-break
    severity: suggest
    fix: switch with case predicates / switch true {}; clearer and easier to extend.

14. routes.go × 8 else if chains in handler dispatch
    rule: same
    severity: suggest

15. db.go:167, store.go:3549 — naked return at end of multi-page functions
    rule: https://google.github.io/styleguide/go/decisions#named-result-parameters
    severity: suggest
    quote: "Naked returns are acceptable only in short functions."
    fix: return values explicitly.

16. All 19 source files in package main, 22.4k LOC in one package
    rule: https://google.github.io/styleguide/go/best-practices#package-size
    severity: must-fix (architectural — this is issue refactor: split cmd/server/ monolith files (store.go 7389L, routes.go 2714L, db.go 2346L) — contributor onboarding cliff #1382)
    quote: "Splitting … into too many tiny files [or] jamming … into a single huge file [are both wrong]."
    fix: this is the file-split refactor on refactor: split cmd/server/ monolith files (store.go 7389L, routes.go 2714L, db.go 2346L) — contributor onboarding cliff #1382. store.go (9287 LOC) and routes.go (3359 LOC) are above the guidance even with charitable reading; further, cmd/server houses analytics, persistence, websocket, decoder, and HTTP routing in one binary's main, which the guide flags as a coherence smell.

17. No context.Context plumbed through cmd/server/ — 0 functions take ctx
    rule: https://google.github.io/styleguide/go/decisions#contexts
    severity: suggest (architectural)
    quote: "Most functions that take a Context should accept it as their first parameter."
    why: HTTP handlers receive r.Context() but it is never threaded into db.GetX / store.GetX. Background goroutines have no cancellation.
    fix: out of scope for an audit, but worth a tracking issue.

18. coverage_test.go & friends — t.Errorf("… = %v, want %v", …, got, want) (got before want OK), but 32 sites flip the order
    rule: https://google.github.io/styleguide/go/decisions#got-before-want
    severity: suggest
    fix: mechanically s/want %v, got %v/got %v, want %v/ after manual verification.

19. openapi.go:199,280 — ApiKeyAuth initialism mis-cased
    rule: https://google.github.io/styleguide/go/decisions#initialisms
    severity: must-fix
    quote: "Words in names that are initialisms or acronyms … have a consistent case."
    fix: APIKeyAuth (these are emitted as JSON keys — verify external compatibility before renaming).

20. geo_filter.go:1-34 — three interface{} parameters in a 34-line file (func ... interface{})
    rule: https://google.github.io/styleguide/go/decisions#use-any
    severity: must-fix (smallest, easiest concrete fix in the package)
    fix: replace with any, then ideally with the concrete Area/Region types already declared.

Themes

Theme 1 — Stringly-typed plumbing dominates the public-facing surface. 394 map[string]interface{} occurrences plus 225 bare interface{} is 619 of 908 findings (68%). The pattern is concentrated in store.go (210), db.go (66), routes.go (57), and types.go (16 + 139 interface{} fields). The file containing the comment "Every API response is a typed struct — no map[string]interface{}" (types.go:4) itself violates the rule 155 times. This is the primary substance of #1383 and is the largest single blocker to compile-time typing of API responses. The decoder series (#1280/#1291) attacked the ingest side of the same problem (concrete packet types) but did not push the typing through to the response shape — store.GetAnalyticsTopology → map[string]interface{} and routes.GET /api/analytics/topology → json.Marshal(map[string]interface{}) is the typical end-to-end shape.

Theme 2 — Get-prefix accessors are pervasive on the data-access layer. All 70 GetFoo instances live in db.go (34), store.go (30), clock_skew.go (3), config.go (3). Receiver-name discipline is, however, internally consistent (each type uses one name); the violation is on Google's anti-Java-bean rule, not on receiver consistency. This is mechanical to fix but couples to #1382 — splitting store.go is the right moment to rename, since both touch the same signatures.

Theme 3 — cmd/server/ is one package main of 22.4k LOC. Google's "package size" best practice flags both extremes; CoreScope sits on the "jammed into one giant package" end. store.go alone is 9287 LOC; routes.go is 3359. This is the exact target of #1382, and the audit data justifies the split: GetFoo accessors, map[string]interface{} payloads, and else if chains all cluster in the three megafiles, so a topic-based split (packetstore/, httpapi/, analytics/) would naturally absorb most of the typing refactor work too.

Theme 4 — Documentation hygiene is weakest where it matters most. 90 of 104 undocumented exported types are in types.go, which is godoc's billboard for the API. websocket.go has 7 undocumented exported methods on a tiny file (3.6 method-docs per 100 lines of code missing). Package comment is missing everywhere — the binary has no Command corescope … doc. By contrast, decoder.go, neighbor_persist.go, memory.go, hash_migrate.go are clean. The split between disciplined files and undisciplined ones tracks recency loosely but tracks ownership tightly: the documented files have a single dominant author per git blame.

Theme 5 — Error wrapping is, surprisingly, well-disciplined. Only 12 %w sites in the entire non-test tree; the previously predicted "excessive wrapping" did not materialize. The one "…failed: %w" site (db.go:51) is the canonical SKILL.md row-3 antipattern, but it is alone. Test-side discipline is also reasonable: zero testify/gocheck imports, t.Fatal/t.Error used per testing conventions; the only persistent failure shape is the want %v, got %v order inversion (32 sites).

Specific predictions vs reality

Prediction 1: excessive %w wrapping — REFUTED.
Evidence: 12 %w total across 19 files (db.go 6, neighbor_persist.go 5, decoder.go 1). One legitimate failed: antipattern at db.go:51. There is no "stacked wrapper at every layer" pattern — the codebase under-wraps if anything; db.go has 39 plain return …, err sites versus 6 %w sites.

Prediction 2: map[string]interface{} / interface{} for typed payloads instead of any — STRONGLY VALIDATED.
Evidence: 394 map[string]interface{} + 225 interface{} = 619 sites; 0 uses of any in the same files (a few any uses exist but only as generic upgrades, not for payload typing). types.go:252-280 defines RawHex/RouteType/PayloadType/SNR/RSSI/PathJSON/Direction/Score etc. as interface{} directly on the public response struct — exactly the bot-emitted shape SKILL.md §8 row 1 was written to catch.

Prediction 3: mixed receiver names + GetFoo accessors — PARTIALLY VALIDATED.
Evidence: GetFoo accessor count is 70, all on *DB, *PacketStore, *Config, *ClockSkewEngine — confirmed. Mixed receiver names within a type is REFUTED: each type (Server/DB/PacketStore/Hub/Client/Config/...) uses one consistent receiver name across all 297 methods. Receiver naming discipline is actually a strong point of this codebase.

Limitations

• store.go (9287 LOC) and routes.go (3359 LOC) were scanned with grep/awk; deep semantic checks (e.g. function-length-vs-naked-return, if err != nil defensive checks on values that cannot be nil, shadowing traps inside multi-block if chains) were not exhaustively performed on either. Heuristics may miss occurrences inside string literals or _test.go mirror code.
• Receiver-name consistency was checked via grep on func (recv *Type) shapes; embedded-struct method promotion and interface-receiver shapes were not analyzed.
• Test files (28 of them, ~22.9k LOC) were checked only for assertion-library import and got/want ordering. A deeper test audit (table-driven shape, t.Helper() placement, subtest naming) is a separate pass.

gofmt/goimports-clean is assumed. If make vet reports findings, they are out of scope for this style audit.