mcp.tools.cached reads as nothing was cached when most of the roster was #534

New issue

Closed

opened 2026-08-13 15:22:56 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-13 15:22:56 +00:00

Member

Follow-up to the listing-telemetry fix that landed for #520. That fix is correct and this is a narrow reading question on top of it, not a regression.

The discovery span now sets:

attribute.Int("mcp.tools.listed", listed),
attribute.Bool("mcp.tools.cached", listed == 0),

listed counts servers that went to the network. cached is true only when none did.

The mixed case

With three servers configured, one listing and two served from cache, the span says:

mcp.tools.listed  1
mcp.tools.cached  false

A reader sees cached=false and concludes nothing was cached. Two thirds of the roster was. The bool answers "was this turn fully cached", but its name answers "was anything cached", and those diverge in the case that is most common once more than one server is configured.

This is the same shape as the defect that motivated #520: an attribute that reads clearly and means something narrower than it reads. It is milder, because listed is right there and a careful reader can see the count is nonzero.

What I am not proposing

I already had a branch that made cached an int count and I closed it. Changing a just-landed attribute's type under its existing name is worse than the ambiguity, and I nearly shipped that collision myself.

Two options that do not collide

Leave it and document the reading. The pair is sufficient if you know listed is a count and cached is an all-or-nothing flag. One paragraph in the roster doc, no code change, no telemetry churn.

Add the roster size. mcp.tools.configured as an int makes the cached count derivable as configured - listed without touching either existing attribute. Additive, no reader loses anything, and it also makes listed interpretable on its own, which today it is not: listed=1 means something different on a one-server roster than a five-server one.

I lean toward the second, and it is small. Flagging rather than doing it, because the seat that landed #520 may have deliberately chosen the bool and I would rather ask than churn their work twice in a day.

Follow-up to the listing-telemetry fix that landed for https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/520. That fix is correct and this is a narrow reading question on top of it, not a regression. The discovery span now sets: ```go attribute.Int("mcp.tools.listed", listed), attribute.Bool("mcp.tools.cached", listed == 0), ``` `listed` counts servers that went to the network. `cached` is true only when *none* did. ## The mixed case With three servers configured, one listing and two served from cache, the span says: ``` mcp.tools.listed 1 mcp.tools.cached false ``` A reader sees `cached=false` and concludes nothing was cached. Two thirds of the roster was. The bool answers "was this turn fully cached", but its name answers "was anything cached", and those diverge in the case that is most common once more than one server is configured. This is the same shape as the defect that motivated #520: an attribute that reads clearly and means something narrower than it reads. It is milder, because `listed` is right there and a careful reader can see the count is nonzero. ## What I am not proposing I already had a branch that made `cached` an int count and I closed it. Changing a just-landed attribute's *type* under its existing name is worse than the ambiguity, and I nearly shipped that collision myself. ## Two options that do not collide **Leave it and document the reading.** The pair is sufficient if you know `listed` is a count and `cached` is an all-or-nothing flag. One paragraph in the roster doc, no code change, no telemetry churn. **Add the roster size.** `mcp.tools.configured` as an int makes the cached count derivable as `configured - listed` without touching either existing attribute. Additive, no reader loses anything, and it also makes `listed` interpretable on its own, which today it is not: `listed=1` means something different on a one-server roster than a five-server one. I lean toward the second, and it is small. Flagging rather than doing it, because the seat that landed #520 may have deliberately chosen the bool and I would rather ask than churn their work twice in a day.

coilyco-ops referenced this issue

2026-08-13 15:32:00 +00:00

A total MCP outage is reported as mcp.tools.cached=true, so the new attribute asserts the comfortable answer on the one turn it matters #540

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

2026-08-13 15:35:00 +00:00

fix(telemetry): an outage is not a cache hit #545

coilyco-ops closed this issue

2026-08-13 15:40:24 +00:00

coilyco-ops referenced this issue from a commit

2026-08-13 15:40:24 +00:00

fix(telemetry): an outage is not a cache hit

coilyco-ops referenced this issue