Skip to content

The #138 refactor was undone within hours — add a size ratchet so decomposition holds #302

Description

@serge-ivo

The evidence

packages/sdk/src/voice/use-voice.ts, measured over one day:

When LOC
#138 filed 1127
this morning 1441
after f194a72 (the #138 refactor) 1315
now, hours later 1532

The refactor removed 126 lines and was fully undone within hours — the file is now larger than
it was when the refactor started, and 36% above the number in the ticket that asked for it.

f194a72 was good work. Its commit body explains, with measurements, why the three sub-hooks the
ticket named were the wrong seams (ttsRef is read at ~20 sites because "is the agent speaking" is
an input to the guard model, so a useTts would hand the refs straight back out) and extracts what
genuinely separates instead: cues.ts and voice-audio.ts, with tests.

None of that survives contact with the next feature. There is no mechanism keeping it.

The general shape

The largest files are also the untested ones:

1695  workers/api/src/routes/instances.ts
1646  workers/api/src/routes/coding.ts
1532  packages/sdk/src/voice/use-voice.ts      no test file
1216  agents/coder/web/src/CodingTab.tsx       no test file
1148  store/console/src/tabs/SettingsTab.tsx   no test file
1141  store/console/src/pages/InstanceDetail.tsx

That correlation is not incidental — a file too large to test is where behaviour goes to hide. Two
of today's bugs were in exactly these files: #240 (stale tab data / cross-agent save,
InstanceDetail) and #241 (unrecoverable offline state, CodingTab). Both were found by
reading, because nothing else could find them.

Proposal: a ratchet, not another refactor ticket

A size check in CI over a small allowlist of known-large files, each pinned at its current LOC:

  • exceeding the pin fails, with a message naming the file and the ticket for splitting it;
  • lowering the pin is a normal part of any PR that shrinks the file;
  • new files over a threshold need an entry, which makes adding one a decision.

This is the same shape as the guard added for #231 (a test that scans the tree for raw
UPDATE agent_instances SET config, because "a reviewer cannot be expected to catch the
seventeenth by eye"
). Same reasoning: the fix holds only if something keeps it held.

Cheap, and it converts "we should refactor that someday" into "you cannot make it worse without
saying so."

Not proposed

Reopening #138. Its conclusion — that the remaining code is imperative sequencing over shared
mutable state and does not decompose into the hooks that were imagined — was correct and measured.
The problem is not that the split was wrong; it is that nothing defends the ground it took.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions