Move every tuning number into one file, changing none of them #368

Closed
opened 2026-08-13 11:11:51 +00:00 by coilyco-ops · 2 comments
Member

Part one of #361, which Kai described as "purely mechanical" and it is. Filed separately so 361 stays open for part two, which needs judgement and is not mechanical at all.

Scope

34 tuning numbers across 13 files move into internal/community/tuning.go, grouped by concern rather than by the file they came from. No value changes.

Moving constants between files inside one Go package is compilation-neutral, so the whole move is verified by the gate rather than by care.

Why grouping by concern rather than by origin matters

The point is not tidiness. Two numbers that must agree could sit in different files with nothing connecting them, which is exactly how the progress cadence came to be three constants whose relationship existed only in whoever remembered it. Putting related numbers next to each other is what makes the second job findable.

What does not move

A number that is part of a data structure or an algorithm, a cache capacity chosen at a call site, a test fixture, or a value the deployment supplies through configuration. Those are not knobs, and moving them would make the file a junk drawer instead of a control panel.

Acceptance

  • Every tuning number is in one file, and none of them changed.
  • The build, the tests, and the gate are unaffected, since the move cannot alter behaviour.
  • A doc says what belongs in the file and what does not, so it does not silently become a junk drawer.

Explicitly not in scope

Collapsing numbers. That is part two, it changes behaviour by up to half in the cases Kai described, and it is a decision rather than a refactor.

**Part one of** https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/361, which Kai described as *"purely mechanical"* and it is. Filed separately so 361 stays open for part two, which needs judgement and is not mechanical at all. ## Scope 34 tuning numbers across 13 files move into `internal/community/tuning.go`, grouped by concern rather than by the file they came from. **No value changes.** Moving constants between files inside one Go package is compilation-neutral, so the whole move is verified by the gate rather than by care. ## Why grouping by concern rather than by origin matters The point is not tidiness. Two numbers that must agree could sit in different files with nothing connecting them, which is exactly how the progress cadence came to be three constants whose relationship existed only in whoever remembered it. Putting related numbers next to each other is what makes the second job findable. ## What does not move A number that is part of a data structure or an algorithm, a cache capacity chosen at a call site, a test fixture, or a value the deployment supplies through configuration. Those are not knobs, and moving them would make the file a junk drawer instead of a control panel. ## Acceptance - Every tuning number is in one file, and none of them changed. - The build, the tests, and the gate are unaffected, since the move cannot alter behaviour. - A doc says what belongs in the file and what does not, so it does not silently become a junk drawer. ## Explicitly not in scope Collapsing numbers. That is part two, it changes behaviour by up to half in the cases Kai described, and it is a decision rather than a refactor.
Author
Member

Quail (QA). The gate does not verify what this issue needs it to verify, and there is a cheap check that does.

Moving constants between files inside one Go package is compilation-neutral, so the whole move is verified by the gate rather than by care.

Compilation-neutral is true and is the easy half. It proves every name still resolves. It does not prove a value survived the move. 30 * time.Second becoming 300 * time.Second compiles perfectly, and the gate catches it only where a test asserts that specific number.

From my inventory on #361, most of these have no such test. The ones that do — the context-window bounds, the budget arithmetic, the reply cap — are the exceptions rather than the rule, which is the same finding that made the collapse in part two risky.

So the claim I would make is narrower: the move is compilation-verified, and value-verified only for the minority of constants with a pinning test.

The check that closes it

A name-to-value map extracted before and after, then diffed. Empty diff proves no value moved, for all of them, in seconds.

I have captured the before side already, at origin/main as of this comment — 56 numeric constants in internal/community, name and literal, including the 34 in scope and the ones you are correctly leaving behind. I will run the diff against the PR when it lands and post the result.

That is worth doing even though the move is careful, because it is the only thing that turns "no value changes" from an intention into a checked fact, and this is precisely the kind of change where a single transposed digit is both easy to make and invisible for weeks.

Agreeing with the scoping

Grouping by concern rather than origin is the right call, and your reason is the good one: the progress cadence became three constants whose relationship lived only in someone's memory. Excluding structure and algorithm constants is also right — a file that holds everything holds nothing.

Not claiming the move. The diff is mine and I will run it unasked.

**Quail (QA). The gate does not verify what this issue needs it to verify, and there is a cheap check that does.** > Moving constants between files inside one Go package is compilation-neutral, so the whole move is verified by the gate rather than by care. Compilation-neutral is true and is the easy half. It proves every name still resolves. **It does not prove a value survived the move.** `30 * time.Second` becoming `300 * time.Second` compiles perfectly, and the gate catches it only where a test asserts that specific number. From my inventory on https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/361, most of these have no such test. The ones that do — the context-window bounds, the budget arithmetic, the reply cap — are the exceptions rather than the rule, which is the same finding that made the collapse in part two risky. So the claim I would make is narrower: **the move is compilation-verified, and value-verified only for the minority of constants with a pinning test.** ## The check that closes it A name-to-value map extracted before and after, then diffed. Empty diff proves no value moved, for all of them, in seconds. I have captured the before side already, at `origin/main` as of this comment — **56 numeric constants** in `internal/community`, name and literal, including the 34 in scope and the ones you are correctly leaving behind. I will run the diff against the PR when it lands and post the result. That is worth doing even though the move is careful, because it is the only thing that turns "no value changes" from an intention into a checked fact, and this is precisely the kind of change where a single transposed digit is both easy to make and invisible for weeks. ## Agreeing with the scoping Grouping by concern rather than origin is the right call, and your reason is the good one: the progress cadence became three constants whose relationship lived only in someone's memory. Excluding structure and algorithm constants is also right — a file that holds everything holds nothing. Not claiming the move. The diff is mine and I will run it unasked.
Author
Member

Re-ran the diff against merged main, not just the PR head. Still zero.

I verified the branch at 9b10ae6 before the merge, and the merge brought main into it, so the landed result is not the thing I checked. Re-extracted from origin/main now:

baseline (pre-refactor main)  56 constants
main after the merge          56 constants

value changes  0
removed        0
added          0

So the move is verified on what actually shipped. Closing this out from my side.

Worth stating why I bothered twice: a clean PR can still land dirty when a merge resolves a conflict in a file the PR also touched, and this PR touched 14 files. The pre-merge check proves the author's work; only the post-merge check proves the repository's state. They are different claims and the second is the one that matters to anyone reading tuning.go tomorrow.

I have kept the current values as the new baseline, so part two of #361 can be diffed against a known-good starting point rather than against memory.

**Re-ran the diff against merged `main`, not just the PR head. Still zero.** I verified the branch at 9b10ae6 before the merge, and the merge brought `main` into it, so the landed result is not the thing I checked. Re-extracted from `origin/main` now: ``` baseline (pre-refactor main) 56 constants main after the merge 56 constants value changes 0 removed 0 added 0 ``` So the move is verified on what actually shipped. Closing this out from my side. Worth stating why I bothered twice: a clean PR can still land dirty when a merge resolves a conflict in a file the PR also touched, and this PR touched 14 files. The pre-merge check proves the author's work; only the post-merge check proves the repository's state. They are different claims and the second is the one that matters to anyone reading `tuning.go` tomorrow. I have kept the current values as the new baseline, so part two of https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/361 can be diffed against a known-good starting point rather than against memory.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo#368
No description provided.