maintainability audit: 103 duplicated blocks, 267 functions with 6+ params, and a 16.9k-line file

[JSONbored]

Umbrella for the maintainability audit. Motivation: "our app is extremely heavy and has a lot of pitfalls, making it very hard to maintain." This is the measured version of that, with the specific instances split into their own issues.

Measurements

metric	value
source LOC (src/** + packages/*/src/**)	227,477
test LOC	461,366 (2.0×)
src/queue/processors.ts	16,913 lines — 7.4% of all source in one file
src/db/repositories.ts	9,052
src/api/routes.ts	6,382
functions with ≥6 positional parameters	267
worst offender	persistDecisionRecord — 13 positional parameters
cross-file duplicated blocks (≥8 non-comment lines)	103

Why this is not abstract — four incidents in one session traced to it

1. P1: manual-review label flaps every pass — a plan can contain both an add and a remove of it #10164 (P1, label flap in production). The same 8-line manual-review-hold block existed in three transforms. The third (calibration: randomized ε-holdout on would-close PRs with propensity logging #8831, added later) diverged from the other two, and nobody could see it because there was no single definition to compare against. Cost: a label cycling every 90 seconds on live PRs.

2. perf(webhook): four of five coalesce families are keyed but windowless, so they never coalesce #10127 (webhook coalescing inert). Key and window were computed by two separate functions. Four of five families had a key and no window, so they could never coalesce — 1,727 check_suite.completed deliveries in 1.5 days and 2.9 gate passes per head SHA. Fixed by making one function return both, so the state is unrepresentable.

3. cost: estimateNeurons is duplicated 4x and the copies drop the calls multiplier, under-reporting AI spend #10169 (AI spend under-reported). estimateNeurons exists four times; the canonical one is exported and takes a calls multiplier, the three private copies do not. A retrying service reports half its real spend into the budget backstop.

4. gate: let maintainers be exempt from the missing-linked-issue penalty without weakening it for contributors #10158 (config knob). Adding one gate knob required edits in seven places. Four separate exhaustiveness guards caught it half-registered — and two of those failures were silent in production (parses but warns "unknown key"; drops on serialization round-trip). The guards are excellent; needing seven edits is the problem.

The common shape: two or more places that must agree, with nothing enforcing it. Every one of the above was invisible until it caused a production symptom.

Concrete, bounded wins

• estimateNeurons / extractAiText — 4 and 3 copies. → cost: estimateNeurons is duplicated 4x and the copies drop the calls multiplier, under-reporting AI spend #10169
• Configured-repo-set resolution — the listRepositories + listConvergenceRepos merge is duplicated 5 times (3 of them inside processors.ts alone), byte-identical across pr-reconciliation.ts, sweep-watchdog.ts, and processors.ts. One helper, five call sites.
• persistDecisionRecord(…13 positional params…) — call sites already pass undefined placeholders positionally. An options object removes a whole class of silent same-type argument-order bugs.
• gateCheckPolicy — 7 positional parameters; gate: let maintainers be exempt from the missing-linked-issue penalty without weakening it for contributors #10158 deliberately routed around it rather than adding an eighth, which is a smell that the next feature will hit too.

processors.ts

16,913 lines is the single largest structural cost. Today it meant hunting four gateCheckPolicy call sites across the file and finding that author context was in scope at only one of them — the direct cause of #10158 needing a shared resolver. It also holds 3 of the 5 repo-set duplicates.

Not proposing a big-bang split. The tractable version is extracting cohesive, already-duplicated units (the repo-set resolver above is a clean first one) and letting the file shrink as a side effect of removing duplication, rather than as a refactor for its own sake.

Deliberately NOT flagged

The 2:1 test-to-source ratio is not bloat — the exhaustiveness guards and invariant tests in this repo caught real, silent bugs repeatedly today. Several of the 103 duplicate blocks are type declarations or coincidental structure; the ones listed above were each read and confirmed as genuine copy-paste.

[linear-code]

JSO-7103

[JSONbored]

Verified the headline numbers before acting on them. Two of the three hold; the flagship example does not.

persistDecisionRecord does not have 13 parameters — it has 6, and only 2 call sites.

persistDecisionRecord(env, record, recordDigest, attempts = 3, reevaluation?, holdCause?)

The "13" is measurement error. Reproducing the audit with a naive param counter gives 13 for it and 267-ish overall; the same counter with comments stripped first gives 6 for it and 242 overall. The gap is commas inside the JSDoc written between parameters — persistDecisionRecord documents reevaluation and holdCause inline, and every comma in that prose counted as a parameter.

That matters beyond one wrong number: the metric penalises exactly the documentation this codebase's style demands. The functions that look worst are disproportionately the best-commented ones, so a ranked worklist built on it would send someone at the wrong targets first. addPullRequestFindings (17 → 12) and capturePage (14 → dropped off the top ten) moved the same way.

Corrected top offenders (comments stripped; still a heuristic, ±1 on generics/trailing commas):

params	function	file
13	addPullRequestFindings	src/rules/advisory.ts
13	runWorkersOpinion	src/services/ai-review.ts
12	reuseOrRefreshLiveCiAggregate	src/queue/ci-resolution.ts
10	fetchPagedSegment	src/github/backfill.ts
10	maybeEnqueueSiblingRegateForMergedPr	src/queue/processors.ts
10	resolvePerRepoContributorCapMatch	src/queue/processors.ts
10	maybePublishPrPublicSurface	src/queue/processors.ts

What still holds: the bulk count (242, not 267, but the same order), and processors.ts at 16,981 lines — it has grown ~80 lines since this issue was filed, so the trend is still the wrong way.

Suggested re-scope. addPullRequestFindings is the right first target and is not arbitrary: it is one of the two deliberately-divergent advisory twins (src/rules/advisory.ts and packages/loopover-engine/src/advisory/gate-advisory.ts), so its positional list has to be threaded identically in two places every time it grows — I added a 12th argument to both sides in #10205 and the parity burden is real and recurring. Converting that one pair to an options object removes an entire class of future drift, and test:engine-parity already guards the result.

Re-running the audit with comments stripped before ranking would be worth doing before anyone picks the next target.

[JSONbored]

Re-baseline: the parameter-count figures in this issue are wrong, measured properly

The 267 functions with ≥6 positional parameters and persistDecisionRecord — 13 positional parameters headline numbers come from a text-based count that includes JSDoc commas — the same artifact #10210's correction identified for addPullRequestFindings. Re-measured with the TypeScript AST (ts.createSourceFile, counting node.parameters.length on every function/method/arrow across src/** + packages/loopover-engine/src/**, 702 files):

	issue says	actual
functions with ≥6 params	267	177
worst offender	persistDecisionRecord (13)	runWorkersOpinion (12)
persistDecisionRecord	13	6

persistDecisionRecord is not an offender at all and should be struck from the list — it takes (env, record, recordDigest, attempts = 3, reevaluation?, holdCause?). Anyone picking it up would find nothing to fix.

Distribution of the real 177: {6: 95, 7: 56, 8: 17, 9: 6, 11: 2, 12: 1}. So the tail is thin — 26 functions at 8+, and that is the tractable target, not 267.

The real worst offenders

12  runWorkersOpinion                    src/services/ai-review.ts:1613
11  fetchPagedSegment                    src/github/backfill.ts:1703
11  reuseOrRefreshLiveCiAggregate        src/queue/ci-resolution.ts:448
 9  maybeEnqueueSiblingRegateForMergedPr src/queue/processors.ts:1415
 9  resolvePerRepoContributorCapMatch    src/queue/processors.ts:3005
 9  resolveContributorCapMatch           src/queue/processors.ts:3055
 9  maybePublishPrPublicSurface          src/queue/processors.ts:10186
 9  capturePage                          src/review/visual/capture.ts:366
 9  runDualAiTieBreakJudgeCall           src/services/ai-review.ts:2752

runWorkersOpinion is the natural successor to #10210

It carries exactly the hazard that issue described, and it is not hypothetical:

async function runWorkersOpinion(
  env, primary, fallback, system, user, maxTokens,
  diagnostics = [], systemAppend = "", correlation?,
  images?,                      // #4111 — no caller passes it
  bodyTruncated = false,        // #8961
  prHasTestEvidence = false,    // #8833
)

• Two adjacent same-typed booleans at the tail (bodyTruncated, prHasTestEvidence). Swap them and nothing fails to compile, nothing fails a type check, and the resulting demotion behaviour is silently wrong in both directions.
• images? has no caller (its own comment says so), so every one of the 4+ call sites that wants the two trailing booleans must thread an undefined placeholder past it — the exact "call sites already pass undefined placeholders positionally" smell recorded here for persistDecisionRecord, except it is true of this one.
• Each of the last three parameters was appended by a separate issue (feat(review): advisory-only AI-vision analysis of before/after visual captures #4111, Review-input truncation causes false 'missing screenshot evidence' closes #8961, gate: move every objectively-checkable criterion from LLM judgment to deterministic checks #8833). The next signal will append a thirteenth.

Same fix shape as #10210: keep the genuinely positional head, convert the accidental tail to an options object.

Method note

Counting was done by AST, not regex, and the script counts parameters.length on FunctionDeclaration / MethodDeclaration / FunctionExpression / ArrowFunction. That includes anonymous arrows, so the 177 is if anything an over-count of named offenders — the real named-function figure is lower still.

The 103 cross-file duplicated blocks figure has not been re-verified and may carry the same class of error. Two of the concrete wins listed in this issue are already resolved and should be struck: estimateNeurons/extractAiText (#10169, closed) and the configured-repo-set duplication (now src/review/configured-repo-set.ts).