refactor(tuning): return seven drifted numbers home and pin the arrangement

coilyco-ops commented

2026-08-15 17:30:28 +00:00

Member

Refs #829. Kai answered the open question from the first version: all of it in config.go, all of it through one helper. Both halves are done.

One file

tuning.go is gone. Its numbers live in config.go beside what the deployment supplies. TestEveryTuningNumberLivesInConfigGo holds the arrangement, and TestEveryNumberInConfigGoIsInTheTable catches a number declared in the file that skipped the helper.

One helper

Every number is declared on one line binding three things:

overridable(&defaultQueueTimeout, "SIRENS_ECHO_QUEUE_TIMEOUT", 30*time.Second)
overridable(&maxToolRounds, "SIRENS_ECHO_TOOL_ROUNDS", 6)

The helper is generic over the two shapes a number takes here, a count and a duration, so a count is as settable as a timeout and neither gets a parser of its own. 72 knobs, up from 8.

The second hand-maintained table of pointers is gone. Adding a number to the table is now the only way to add one, which is what keeps the list complete, and TestEveryKnobStartsOnItsDeclaredDefault reads each variable back to confirm the declared default is the one it holds.

One behaviour, where there were two

REQUEST_TIMEOUT, QUEUE_TIMEOUT, and SHUTDOWN_GRACE were parsed a second time to fill a Config field. That second reader refused a bad value and failed the load while the table applied one silently - one name, two readers, two answers, which is the false claim the first version of this PR found in the doc.

Config now takes what the knob pass produced, so a bad value keeps the default everywhere. Silence there would read as a working override, so applied and rejected names are reported on the capabilities log line at startup.

The list is generated, not maintained

Seventy-two names cannot live in prose without going stale, and the workspace rule is to derive an inventory from its owner rather than keep a second copy. ward exec knobs writes docs/sirens-echo-knobs.md from the table; ward exec knobs-check and a unit test both fail when it is stale. The prose page explains the mechanism and links it.

Two carve-outs, both narrower than before

Three derived values. turnProgressEvery, turnLongReplyAfter, and replyAttachmentBytes are expressions of another number, so they are set by setting their input and recomputed after the overrides land. A test drives all three.

Two algorithm floors. minNormalizedIDDigits and minEncodedGuardBytes decide what counts as an identifier rather than how much of one to allow. They are not configuration and no name reaches them.

maxProxyToolNameBytes is no longer exempt. The previous version grouped it with those two as a security floor. It is a bound on a served tool name, not a floor on what counts as a match, and it lived in tuning.go under that file's own rule. It is a knob now. If you disagree, that is one line to put back.

Worth saying plainly

Some of these are Discord's numbers rather than ours - the command-shape bounds, the history page size, the reply limit. Raising one past what Discord accepts fails at Discord as a rejected send rather than here as a startup error. They are settable because everything is, not because moving them is a good idea, and the doc says so where an operator reads it.

Verification

ward gate PASS on all six steps, on the rebased tree. go test -race clean, which matters more than it did: seventy-two package globals are now written by the knob pass, so the override tests restore the defaults through t.Cleanup rather than by hand.

The int-shaped numbers stopped being untyped constants, so seven call sites that fed them to io.LimitReader, http.MaxBytesReader, or an int64 comparison gained an explicit conversion.

Refs #829. Kai answered the open question from the first version: **all of it in `config.go`, all of it through one helper.** Both halves are done. ## One file `tuning.go` is gone. Its numbers live in `config.go` beside what the deployment supplies. `TestEveryTuningNumberLivesInConfigGo` holds the arrangement, and `TestEveryNumberInConfigGoIsInTheTable` catches a number declared in the file that skipped the helper. ## One helper Every number is declared on one line binding three things: ```go overridable(&defaultQueueTimeout, "SIRENS_ECHO_QUEUE_TIMEOUT", 30*time.Second) overridable(&maxToolRounds, "SIRENS_ECHO_TOOL_ROUNDS", 6) ``` The helper is generic over the two shapes a number takes here, a count and a duration, so a count is as settable as a timeout and neither gets a parser of its own. **72 knobs**, up from 8. The second hand-maintained table of pointers is gone. Adding a number to the table is now the only way to add one, which is what keeps the list complete, and `TestEveryKnobStartsOnItsDeclaredDefault` reads each variable back to confirm the declared default is the one it holds. ## One behaviour, where there were two `REQUEST_TIMEOUT`, `QUEUE_TIMEOUT`, and `SHUTDOWN_GRACE` were parsed a second time to fill a `Config` field. That second reader **refused** a bad value and failed the load while the table applied one **silently** - one name, two readers, two answers, which is the false claim the first version of this PR found in the doc. `Config` now takes what the knob pass produced, so a bad value keeps the default everywhere. Silence there would read as a working override, so applied and rejected names are reported on the `capabilities` log line at startup. ## The list is generated, not maintained Seventy-two names cannot live in prose without going stale, and the workspace rule is to derive an inventory from its owner rather than keep a second copy. `ward exec knobs` writes `docs/sirens-echo-knobs.md` from the table; `ward exec knobs-check` and a unit test both fail when it is stale. The prose page explains the mechanism and links it. ## Two carve-outs, both narrower than before **Three derived values.** `turnProgressEvery`, `turnLongReplyAfter`, and `replyAttachmentBytes` are expressions of another number, so they are set by setting their input and recomputed after the overrides land. A test drives all three. **Two algorithm floors.** `minNormalizedIDDigits` and `minEncodedGuardBytes` decide what counts as an identifier rather than how much of one to allow. They are not configuration and no name reaches them. **`maxProxyToolNameBytes` is no longer exempt.** The previous version grouped it with those two as a security floor. It is a bound on a served tool name, not a floor on what counts as a match, and it lived in `tuning.go` under that file's own rule. It is a knob now. If you disagree, that is one line to put back. ## Worth saying plainly Some of these are Discord's numbers rather than ours - the command-shape bounds, the history page size, the reply limit. Raising one past what Discord accepts fails at Discord as a rejected send rather than here as a startup error. They are settable because everything is, not because moving them is a good idea, and the doc says so where an operator reads it. ## Verification `ward gate` PASS on all six steps, on the rebased tree. `go test -race` clean, which matters more than it did: seventy-two package globals are now written by the knob pass, so the override tests restore the defaults through `t.Cleanup` rather than by hand. The int-shaped numbers stopped being untyped constants, so seven call sites that fed them to `io.LimitReader`, `http.MaxBytesReader`, or an `int64` comparison gained an explicit conversion.

coilyco-ops added 1 commit

2026-08-15 17:30:29 +00:00

refactor(tuning): return seven drifted numbers home and pin the arrangement

ci / image-build (pull_request) Successful in 27s

Details

ci / test (pull_request) Successful in 43s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

e0024047f6

The sweep found the numeric-config story is two files rather than one.
config.go reads what the deployment supplies through the environment;
tuning.go holds every tuning number the service ships, eight of which take
an environment override. Neither was wrong, and nothing was holding either.

Seven knobs had drifted back out of tuning.go since it was consolidated:
maxWorklogRows, maxRedactedBlocks, inventedChannelRunes,
maxJobContentMessages, maxJobContentWindow, maxProxyToolNameBytes, and
maxProgressWaitLines. Moved. Constants moving between files in one Go
package changes nothing at compile time, so the gate verifies the move.

TestEveryTuningNumberLivesInTuningGo now holds it. A number that reads as
a knob and is not takes a named exemption with a reason, which is how the
two security floors in identifiers.go stay where they belong.

SIRENS_ECHO_SHUTDOWN_GRACE was read in config.go and missing from the
override table, so the Config field moved and the package global every
fallback path reads did not. Added, which makes the three turn timeouts
behave alike.

The overrides doc said seven and the table held eight, so a second test
now pins the doc and the table against each other.

One documented claim was false and is corrected rather than removed: "a
bad value keeps the default" does not hold for the three names LoadConfig
also parses, because those fail the load instead.

README and AGENTS.md now say where a number goes.

`ward gate` PASS on all six steps.

Refs #829

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-15 17:30:29 +00:00

coilyco-ops added 1 commit

2026-08-15 20:16:38 +00:00

refactor(tuning): declare each override's env var and default on one line

ci / image-build (pull_request) Successful in 28s

Details

ci / test (pull_request) Successful in 46s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

e2e03671e9

Per Kai on #829: reading the line that names an environment variable
should also tell you what it falls back to.

Every overridable knob now declares both at the value's own home:

	defaultQueueTimeout = overridable("SIRENS_ECHO_QUEUE_TIMEOUT", 30*time.Second)

The value is not restated. `overridable` records the pair and returns the
default, so the declaration stays the one place the number lives, and the
grouping by concern that tuning.go is organised around is untouched.

config.go's three split reads are collapsed the same way, so the env var
and its fallback sit together there too.

Two tests pin the declaration against the write-through table in both
directions, and a third pins the declared default against the value the
variable actually holds. A name can no longer be declared and never
written, or written and never documented.

`ward gate` PASS on all six steps.

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops referenced this pull request

2026-08-15 20:16:55 +00:00

Do a sweep to make sure every numeric configuration is in internal/community/config.go, with the classic env var override, update READMEmd to point at that file #829

coilyco-ops force-pushed aos/claude/bk79-numeric-config from e2e03671e9

ci / image-build (pull_request) Successful in 28s

Details

ci / test (pull_request) Successful in 46s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

to ffa9cfbf90

ci / image-build (pull_request) Successful in 25s

Details

ci / test (pull_request) Successful in 42s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

2026-08-15 21:08:59 +00:00

Compare

coilyco-ops referenced this pull request

2026-08-15 21:15:19 +00:00

feat(style): admit object emoji in a neutral reply, bounded at three #840

coilysiren commented

2026-08-16 01:53:38 +00:00

Owner

given to an engineer just now

coilyco-ops added 1 commit

2026-08-16 01:58:43 +00:00

Merge main, resolving the tuning.go delete and the ward.yaml move

ci / image-build (pull_request) Successful in 25s

Details

ci / test (pull_request) Successful in 43s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

61a7af0301

Two conflicts, both from this branch and main changing the same things
from opposite directions.

`tuning.go` is deleted here and main added `maxObjectEmoji` to it for
sirens-echo#203. Kept the deletion and carried the number into config.go
through `overridable`, so the emoji cap is now a knob like the other 72
rather than the one constant that survived the move. It reads
SIRENS_ECHO_OBJECT_EMOJI and appears in the generated reference.

`.ward/ward.yaml` lost its whole `commands:` block on main when #853
moved the dev verbs to a justfile. Took main's file and moved this
branch's two verbs, `knobs` and `knobs-check`, to justfile recipes.

Six places still said `ward exec knobs`, including the header the
generator writes into the tracked reference, so the generated file
carried an instruction that no longer worked. All now say `just knobs`,
and the reference is regenerated.

`just gate` PASS on all six steps. `just knobs-check` exits 0, 73 knobs.

Co-authored-by: coilyco-ops[bot] <coilyco-ops@coilysiren.me>
Co-authored-by: Claude <noreply@anthropic.com>

coilysiren merged commit 3c1cf8f033 into main

2026-08-16 02:09:03 +00:00

coilysiren referenced this pull request from a commit

2026-08-16 02:09:03 +00:00

Merge pull request 'refactor(tuning): return seven drifted numbers home and pin the arrangement' (#835) from aos/claude/bk79-numeric-config into main

coilyco-ops referenced this pull request

2026-08-17 02:22:57 +00:00

Do a sweep to make sure every numeric configuration is in internal/community/config.go, with the classic env var override, update READMEmd to point at that file #829

coilyco-ops referenced this pull request

2026-08-17 02:29:39 +00:00

test(knobs): find a stray number by shape, not by its name #876