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

coilysiren commented

2026-08-15 17:16:42 +00:00

Owner

I don't recall if agents will follow READMEmd as closely as they do AGENTSmd

But if they dont, then update AGENTSmd with this info as well

I don't recall if agents will follow READMEmd as closely as they do AGENTSmd But if they dont, then update AGENTSmd with this info as well

coilyco-ops added the

labels

2026-08-15 17:24:32 +00:00

coilyco-ops commented

2026-08-15 17:26:17 +00:00

Member

Triage note. Darren (director seat), 2026-08-15.

Labelled priority/P2 headless. Headless because the ask is concrete and mechanical: gather the numbers into internal/community/config.go, give each the standard env override, and point the docs at that file. No design fork in it.

This is the mechanical half of #361, which is open at priority/P4 consult and splits the same work in two:

all the numbers into a single file - this will be purely mechanical

less numbers - this will require judgement. some numbers can be derived from others [...] for those cases, eat the product behavior change cost and collapse them

This issue is step 1. Step 2 stays on #361, where it is correctly consult because collapsing two near-identical numbers into one changes product behaviour and that is Kai's call, not an engineer's.

Worth doing in that order regardless: the collapse judgement is much easier to make once every number is visible in one file, which is the reason #361 lists them in that sequence.

On the README question in the body - agents read AGENTS.md reliably and README.md unevenly, so put the pointer in AGENTS.md and let the README carry the human-facing version. The repo's own trifecta convention already treats those as different audiences.

**Triage note. Darren (director seat), 2026-08-15.** Labelled `priority/P2` `headless`. Headless because the ask is concrete and mechanical: gather the numbers into `internal/community/config.go`, give each the standard env override, and point the docs at that file. No design fork in it. **This is the mechanical half of #361**, which is open at `priority/P4` `consult` and splits the same work in two: > 1. all the numbers into a single file - this will be purely mechanical > 2. less numbers - this will require judgement. some numbers can be derived from others [...] for those cases, eat the product behavior change cost and collapse them This issue is step 1. Step 2 stays on #361, where it is correctly `consult` because collapsing two near-identical numbers into one changes product behaviour and that is Kai's call, not an engineer's. Worth doing in that order regardless: the collapse judgement is much easier to make once every number is visible in one file, which is the reason #361 lists them in that sequence. **On the README question in the body** - agents read `AGENTS.md` reliably and `README.md` unevenly, so put the pointer in `AGENTS.md` and let the README carry the human-facing version. The repo's own trifecta convention already treats those as different audiences.

coilyco-ops referenced this issue from a commit

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

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

coilyco-ops referenced this issue

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

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

coilyco-ops referenced this issue from a commit

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

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

coilyco-ops commented

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

Member

Sweep done, in #835. Angie (ENG, claude seat). Your one-line requirement is in as a second commit.

The one-line shape

Every overridable knob now declares its environment variable and its default at the value's own home:

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

The value is not restated anywhere. overridable records the pair and returns the default, so that line stays the single home, 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.

Three tests pin it: declaration against the write-through table in both directions, and the declared default against the value the variable actually holds.

What the sweep found

Seven knobs had drifted back out of tuning.go since it was consolidated - maxWorklogRows, maxRedactedBlocks, inventedChannelRunes, maxJobContentMessages, maxJobContentWindow, maxProxyToolNameBytes, maxProgressWaitLines. All moved back, and TestEveryTuningNumberLivesInTuningGo now holds the line.

Worth saying plainly: I found four of those with the test, not by reading. My first grep and the test's first draft had the same blind spot - neither matched a top-level const x = 6. The test caught them the moment I fixed its regex.

SIRENS_ECHO_SHUTDOWN_GRACE was half-wired. Read in config.go, absent from the override table, so setting it moved the Config field and left the package global that agent.go and shutdown.go fall back to on its compiled 15s. Fixed.

A documented claim was false. sirens-echo-tuning-overrides.md says "a bad value keeps the default". True for five names, not for the three LoadConfig also parses - those fail the load instead. Corrected rather than deleted. The same doc said seven overridable names while the table held eight, so a test now pins the doc and the table together.

On the premise

It is two files by design, and docs/sirens-echo-tuning.md already argues for the split: config.go for what the deployment supplies, tuning.go for what the service ships. I kept it and made it honest and enforced rather than merging them, since merging collapses a distinction the repo made deliberately.

You did not pick from the scope options, so I have not merged the two files and not extended overrides to the other numbers. Both are still open if you want them - the question stands.

Pointers

README.md gains the two homes in Configuration. AGENTS.md gains a Where numbers live section, stating the rule rather than linking to it, since you were right that agents follow that file more closely.

Conflict note: #835 and #826 both touch AGENTS.md, in different sections. Whichever lands second may want a merge.

**Sweep done, in #835.** Angie (ENG, `claude` seat). Your one-line requirement is in as a second commit. ## The one-line shape Every overridable knob now declares its environment variable and its default at the value's own home: ```go defaultQueueTimeout = overridable("SIRENS_ECHO_QUEUE_TIMEOUT", 30*time.Second) ``` The value is not restated anywhere. `overridable` records the pair and returns the default, so that line stays the single home, 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. Three tests pin it: declaration against the write-through table in both directions, and the declared default against the value the variable actually holds. ## What the sweep found **Seven knobs had drifted back out of `tuning.go`** since it was consolidated - `maxWorklogRows`, `maxRedactedBlocks`, `inventedChannelRunes`, `maxJobContentMessages`, `maxJobContentWindow`, `maxProxyToolNameBytes`, `maxProgressWaitLines`. All moved back, and `TestEveryTuningNumberLivesInTuningGo` now holds the line. Worth saying plainly: **I found four of those with the test, not by reading.** My first grep and the test's first draft had the same blind spot - neither matched a top-level `const x = 6`. The test caught them the moment I fixed its regex. **`SIRENS_ECHO_SHUTDOWN_GRACE` was half-wired.** Read in `config.go`, absent from the override table, so setting it moved the `Config` field and left the package global that `agent.go` and `shutdown.go` fall back to on its compiled 15s. Fixed. **A documented claim was false.** `sirens-echo-tuning-overrides.md` says "a bad value keeps the default". True for five names, not for the three `LoadConfig` also parses - those fail the load instead. Corrected rather than deleted. The same doc said seven overridable names while the table held eight, so a test now pins the doc and the table together. ## On the premise It is two files by design, and `docs/sirens-echo-tuning.md` already argues for the split: `config.go` for what the deployment supplies, `tuning.go` for what the service ships. I kept it and made it honest and enforced rather than merging them, since merging collapses a distinction the repo made deliberately. You did not pick from the scope options, so I have **not** merged the two files and **not** extended overrides to the other numbers. Both are still open if you want them - the question stands. ## Pointers `README.md` gains the two homes in Configuration. `AGENTS.md` gains a **Where numbers live** section, stating the rule rather than linking to it, since you were right that agents follow that file more closely. **Conflict note:** #835 and #826 both touch `AGENTS.md`, in different sections. Whichever lands second may want a merge.

coilyco-ops referenced this issue from a commit

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

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

coilyco-ops referenced this issue from a commit

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

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

coilyco-ops added the

role/engineer

label

2026-08-15 21:11:47 +00:00

coilyco-ops commented

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

Member

Claiming the residual half — Angie (ENG, claude seat). The sweep in #835 landed; this is the gap it left.

#835 moved seven drifted knobs back and put TestEveryTuningNumberLivesInConfigGo behind them, and both pointers are in (README.md Configuration, AGENTS.md Where numbers live). So the body's ask is largely satisfied and the triage note's step 1 is done.

What is not done is the guard itself, and it fails in exactly the way #835 reported hitting twice. The test finds a stray only when its name matches ^(max|min|default)[A-Z] or ends in one of nine suffixes. #835's own comment says four of seven were found by the test only after its regex was fixed, which is the tell: detection by naming convention misses whatever the next author does not name that way.

Re-running that check by hand against the tree it currently guards, one knob is invisible to it today:

commandmcps.go:26 const mcpsReplyBudget = 1800 — a send budget under Discord's 1990-rune interaction bound. A cap by AGENTS.md's own list, outside config.go, settable by nobody, and named neither max* nor *Limit, so the test is silent on it.

Three more are correctly outside config.go but survive on naming luck rather than on a recorded reason: opaqueSecretRunes, scratchPermissions / scratchFilePermissions, workspacePermissions, unboundedReply.

Taking: replace the name-pattern scan with a go/ast walk over every package-level const/var with a numeric value, so a number is a stray unless elsewhereByDesign names it with a reason. Move mcpsReplyBudget into the table. Record the five exemptions. Detection stops depending on what the author called it, which is the property the issue actually wants.

Not taking: step 2 of #361, the collapse. That stays consult and Kai's.

**Claiming the residual half — Angie (ENG, `claude` seat).** The sweep in #835 landed; this is the gap it left. #835 moved seven drifted knobs back and put `TestEveryTuningNumberLivesInConfigGo` behind them, and both pointers are in (`README.md` Configuration, `AGENTS.md` **Where numbers live**). So the body's ask is largely satisfied and the triage note's step 1 is done. **What is not done is the guard itself, and it fails in exactly the way #835 reported hitting twice.** The test finds a stray only when its *name* matches `^(max|min|default)[A-Z]` or ends in one of nine suffixes. #835's own comment says four of seven were found by the test only after its regex was fixed, which is the tell: detection by naming convention misses whatever the next author does not name that way. Re-running that check by hand against the tree it currently guards, one knob is invisible to it today: * `commandmcps.go:26` `const mcpsReplyBudget = 1800` — a send budget under Discord's 1990-rune interaction bound. A cap by AGENTS.md's own list, outside `config.go`, settable by nobody, and named neither `max*` nor `*Limit`, so the test is silent on it. Three more are correctly outside `config.go` but survive on naming luck rather than on a recorded reason: `opaqueSecretRunes`, `scratchPermissions` / `scratchFilePermissions`, `workspacePermissions`, `unboundedReply`. **Taking:** replace the name-pattern scan with a `go/ast` walk over every package-level `const`/`var` with a numeric value, so a number is a stray unless `elsewhereByDesign` names it with a reason. Move `mcpsReplyBudget` into the table. Record the five exemptions. Detection stops depending on what the author called it, which is the property the issue actually wants. **Not taking:** step 2 of #361, the collapse. That stays `consult` and Kai's.

coilyco-ops referenced this issue from a pull request that will close it,

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

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

coilyco-ops commented

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

Member

PR #876 opened. Angie (ENG, claude seat).

The guard now parses the package instead of matching names, so a stray is found by shape. Verified with a probe file carrying the three declaration shapes the old regex could not see (odd name, var x int = 42, 0o755) - all three reported, all three previously invisible.

mcpsReplyBudget is in the table as SIRENS_ECHO_MCPS_REPLY_BUDGET. Five numbers that are correctly outside config.go now carry a written reason instead of surviving on naming luck. just gate passes.

Step 2 of #361, the collapse, is untouched and still Kai's.

**PR #876 opened.** Angie (ENG, `claude` seat). The guard now parses the package instead of matching names, so a stray is found by shape. Verified with a probe file carrying the three declaration shapes the old regex could not see (odd name, `var x int = 42`, `0o755`) - all three reported, all three previously invisible. `mcpsReplyBudget` is in the table as `SIRENS_ECHO_MCPS_REPLY_BUDGET`. Five numbers that are correctly outside `config.go` now carry a written reason instead of surviving on naming luck. `just gate` passes. Step 2 of #361, the collapse, is untouched and still Kai's.

coilysiren closed this issue

2026-08-17 02:32:19 +00:00

Rows
Columns

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

The one-line shape

What the sweep found

On the premise

Pointers

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