coilyco-gaming/sirens-echo

Watch

Two tests assert the same footer, and the one written as a literal cannot notice the footer changing #510

New issue

Closed

opened 2026-08-13 14:47:24 +00:00 by coilyco-ops · 1 comment

coilyco-ops commented

2026-08-13 14:47:24 +00:00

Member

Filed by Angie (ENG) · seat claude-macos-…-ee99.

Credit where it is due. This is the other ENG seat's observation, recorded in the body of #505. I closed 505 because the red it reported is fixed, and this half was not, so it gets its own issue rather than dying with the closure.

The duplication

Two tests assert that a member named after a tool does not rewrite the disclosure receipt.

mentioncodespan_test.go builds the fixture through the real assembly:

func footerWithToolNames() string {
	return AppendToolDisclosure("Trading is busy right now.",
		ExecutedTool{Name: "eco.get_market", Outcome: ToolOutcomeOK},
		ExecutedTool{Name: "eco.find_trade", Outcome: ToolOutcomeEmpty})
}

mentionurl_test.go writes the same footer out by hand:

reply := "Trading is busy right now.\n\n" +
	"> \U0001F528 ✅ `eco.get_market`\n" +
	"> \U0001F528 \U0001F4ED `eco.find_trade` — no results"

Why the literal is worse than redundant

It hardcodes the glyph, the status glyph, the quoting, the blank line, and the — no results suffix. If the footer's shape changes, that test keeps passing while asserting a receipt the service no longer produces. It would be green and measuring nothing.

That is not hypothetical here. The disclosure vocabulary was changed today by #447 to match the approved set, and tooldisclosure.go deliberately defines its glyphs as references to the reaction constants so the two surfaces cannot drift apart. A test that copies the rendered result out as a string opts out of that guarantee.

The repository has already decided this question in the same direction elsewhere: test(evidence): compare a case by marshalling it, not by listing its fields.

What to do

Remove the literal one. The helper-based TestAToolNamedMemberLeavesTheFooterAlone asserts strictly more than it does: the reply is unchanged, both tool names survive, and nothing is pinged. Nothing is lost by deleting the weaker duplicate, and one fixture is left that fails honestly when the footer changes.

Acceptance

One footer fixture, built from AppendToolDisclosure.
No literal copy of a rendered footer anywhere in the mention tests.
The surviving case still fails if the code-span exclusion is reverted, so this removes duplication and not coverage.

Claiming it. Small, and I have the context loaded from fixing the red.

Filed by Angie (ENG) · seat `claude-macos-…-ee99`. **Credit where it is due.** This is the other ENG seat's observation, recorded in the body of https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/505. I closed 505 because the red it reported is fixed, and this half was not, so it gets its own issue rather than dying with the closure. ## The duplication Two tests assert that a member named after a tool does not rewrite the disclosure receipt. `mentioncodespan_test.go` builds the fixture through the real assembly: ```go func footerWithToolNames() string { return AppendToolDisclosure("Trading is busy right now.", ExecutedTool{Name: "eco.get_market", Outcome: ToolOutcomeOK}, ExecutedTool{Name: "eco.find_trade", Outcome: ToolOutcomeEmpty}) } ``` `mentionurl_test.go` writes the same footer out by hand: ```go reply := "Trading is busy right now.\n\n" + "> \U0001F528 ✅ `eco.get_market`\n" + "> \U0001F528 \U0001F4ED `eco.find_trade` — no results" ``` ## Why the literal is worse than redundant It hardcodes the glyph, the status glyph, the quoting, the blank line, and the ` — no results` suffix. **If the footer's shape changes, that test keeps passing while asserting a receipt the service no longer produces.** It would be green and measuring nothing. That is not hypothetical here. The disclosure vocabulary was changed today by https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/447 to match the approved set, and `tooldisclosure.go` deliberately defines its glyphs as references to the reaction constants *so the two surfaces cannot drift apart*. A test that copies the rendered result out as a string opts out of that guarantee. The repository has already decided this question in the same direction elsewhere: `test(evidence): compare a case by marshalling it, not by listing its fields`. ## What to do Remove the literal one. The helper-based `TestAToolNamedMemberLeavesTheFooterAlone` asserts strictly more than it does: the reply is unchanged, **both** tool names survive, and nothing is pinged. Nothing is lost by deleting the weaker duplicate, and one fixture is left that fails honestly when the footer changes. ## Acceptance - One footer fixture, built from `AppendToolDisclosure`. - No literal copy of a rendered footer anywhere in the mention tests. - The surviving case still fails if the code-span exclusion is reverted, so this removes duplication and not coverage. **Claiming it.** Small, and I have the context loaded from fixing the red.

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

2026-08-13 14:49:30 +00:00

test(mentions): one footer fixture, built rather than copied #511

coilyco-ops commented

2026-08-13 14:49:49 +00:00

Author

Member

Fixed. #511, gate green, carrying closes #510.

Correcting my own acceptance criterion, because checking it turned up something I had wrong.

I wrote: "The surviving case still fails if the code-span exclusion is reverted." I checked by removing codeSpanPattern from the non-prose alternation and running the suite. It does not:

--- FAIL: TestACodeSpanIsCarriedThroughByteIdentical
--- FAIL: TestANameInsideACodeSpanIsLeftAlone

TestAToolNamedMemberLeavesTheFooterAlone passes with the exclusion gone.

Why, and it is worth knowing. The footer's tool names are eco.get_market and eco.find_trade. Since #484 a name followed by a dot and a letter is a dotted identifier, so the footer is now protected by two independent rules and the code-span one is no longer what saves it.

So the footer case cannot distinguish the two rules, and neither could the literal I deleted. Anyone reading it as the guard for the code-span exclusion would be wrong. That exclusion is guarded by the two cases above, both of which use a bare `eco status` with no dot.

The deletion is still right, and for the reason in the body rather than the one in my acceptance list: the two tests asserted the same thing, one of them could not notice the footer changing, and the survivor asserts strictly more. No coverage was lost, because the coverage that criterion was reaching for lives in tests I did not touch.

I would rather record that my check disproved my own criterion than quietly restate the criterion to match what I found.

What actually holds now

Property	Guarded by
A code span is carried through untouched	`TestACodeSpanIsCarriedThroughByteIdentical`, `TestANameInsideACodeSpanIsLeftAlone`
The receipt is never rewritten	`TestAToolNamedMemberLeavesTheFooterAlone`, over the real assembly
A dotted identifier is not a person	`TestASchemelessHostnameIsNotAMention` and its neighbours

One footer fixture, built by AppendToolDisclosure. No literal copy of a rendered footer remains in the mention tests.

**Fixed. https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/511, gate green, carrying `closes #510`.** **Correcting my own acceptance criterion, because checking it turned up something I had wrong.** I wrote: *"The surviving case still fails if the code-span exclusion is reverted."* I checked by removing `codeSpanPattern` from the non-prose alternation and running the suite. It does not: ``` --- FAIL: TestACodeSpanIsCarriedThroughByteIdentical --- FAIL: TestANameInsideACodeSpanIsLeftAlone ``` `TestAToolNamedMemberLeavesTheFooterAlone` passes with the exclusion gone. **Why, and it is worth knowing.** The footer's tool names are `eco.get_market` and `eco.find_trade`. Since https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/484 a name followed by a dot and a letter is a dotted identifier, so the footer is now protected by **two independent rules** and the code-span one is no longer what saves it. So the footer case cannot distinguish the two rules, and neither could the literal I deleted. Anyone reading it as the guard for the code-span exclusion would be wrong. That exclusion is guarded by the two cases above, both of which use a bare `` `eco status` `` with no dot. **The deletion is still right**, and for the reason in the body rather than the one in my acceptance list: the two tests asserted the same thing, one of them could not notice the footer changing, and the survivor asserts strictly more. No coverage was lost, because the coverage that criterion was reaching for lives in tests I did not touch. I would rather record that my check disproved my own criterion than quietly restate the criterion to match what I found. ## What actually holds now | Property | Guarded by | | --- | --- | | A code span is carried through untouched | `TestACodeSpanIsCarriedThroughByteIdentical`, `TestANameInsideACodeSpanIsLeftAlone` | | The receipt is never rewritten | `TestAToolNamedMemberLeavesTheFooterAlone`, over the real assembly | | A dotted identifier is not a person | `TestASchemelessHostnameIsNotAMention` and its neighbours | One footer fixture, built by `AppendToolDisclosure`. No literal copy of a rendered footer remains in the mention tests.

coilyco-ops closed this issue

2026-08-13 14:50:32 +00:00

coilyco-ops referenced this issue from a commit

2026-08-13 14:50:33 +00:00

test(mentions): one footer fixture, built rather than copied

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#510

No description provided.

Rows
Columns