Invariant: an unreadable dataset must report null, never zero — sweep all 25 MCP tools #266

Open
opened 2026-08-13 19:46:35 +00:00 by coilyco-ops · 4 comments
Member

🤖 Filed by Claude Code on Kai's behalf.

Why this is filed as an invariant, not a tool bug

Three separate commits have already fixed this exact shape, one tool at a time:

  • abcf1b0fix(status): report absent /info numbers as null, never zero
  • 19513c1fix(civics): report null for exporters that could not be read
  • dd09786fix(economy): separate unmeasured from measured-zero, and read the treasury

The shape is still live in three tools at 5e05296, including the one dd09786 named. Fixing the next demonstrated instance has not converged across three rounds. The rule needs stating once and applying to the whole surface.

This class matters more than its severity suggests: none of these tools error. They return well-formed, confident, wrong numbers, which a downstream caller cannot detect without cross-checking a sibling tool.

The rule

  1. A numeric field MUST be null when the dataset behind it could not be read.
  2. 0 MUST mean the server reported no activity.
  3. A narrative or summary string MUST NOT assert a fact about the world that rests on an unread dataset. "Cannot read this server's ledger" is correct; "nothing has been traded" is not.
  4. Any tool exposing nullable values MUST also name the datasets behind its nulls. get_economy.datasets_unavailable is the reference shape; get_world's per-dataset HTTP 401 warnings are the reference behaviour.

Confirmed violations at 5e05296 (observed 2026-08-13)

1. get_economy.govt_funds reports 0 for a funded treasury

Against eco.coilysiren.me:3001:

get_economy  → govt_funds: 0, datasets_unavailable: [], health: "healthy"
get_currency → money.governmentHoldings: 87912.99

get_economy.govt_funds_note states the value is read from "the same dataset get_currency reports as money.governmentHoldings". Same dataset, same server, same minute, two answers. datasets_unavailable: [] actively asserts nothing was missing. wages_total, taxes_paid, govt_funds_received and net_tax_flow are also 0, and health: "healthy" plus the narrative "Economy is healthy" are derived from them.

2. get_currency converts an auth failure into a claim about the world

Against daokingdom.eu:3001:

narrative: "no currencies have been created or traded yet - early in the cycle"
counts.total: 0
admin_ok: true
money.activeCurrencies: 40          ← same payload
economy_desc: "1773 trades, 5 contracts"  ← same payload

The server has 40 active currencies and 1,773 trades. get_world against the same host in the same session returns HTTP 401 for all nine datasets it tries, so the access failure is unambiguous and a sibling tool already reports it correctly. admin_ok: true is also wrong here.

3. price_recipe ranks the least-known recipe cheapest

price_recipe(product=IronBarItem) returns three recipes:

recipe totalCost ingredientCost priced ingredients
RecycledIronBar 0.22 0 0 of 2
IronBar 1.46 1.28 1 of 2
SmeltIron 3.18 2.56 1 of 2

Sorted by cost, the recipe where nothing could be priced wins by 7×. ingredientCost: 0 here means "no leaf resolved", not "ingredients are free". perUnitCost correctly goes null when complete: false; totalCost and ingredientCost do not, so the only comparable-looking number is the corrupted one.

Same tool against a server with no readable market (daokingdom.eu:3001, product=SteelAxeItem) returns totalCost: 0 with seven entries in unpricedInputs.

Credit where due: unpricedInputs resolves leaves recursively and is correct. The bug is confined to the summed fields.

Acceptance criteria

  • get_economy.govt_funds returns the treasury balance, or null with the dataset named in datasets_unavailable.
  • get_currency reports unreadable-ledger as unreadable; admin_ok reflects actual admin reachability per server.
  • price_recipe.totalCost and .ingredientCost are null whenever complete: false, matching perUnitCost.
  • All 25 MCP tools audited against rules 1–4 above, with the audit recorded as a table in the PR body: tool · nullable numeric fields · datasets-unavailable field present Y/N · verified.
  • A test asserts the invariant generically — an unreadable dataset yields null and a populated unavailable-list — rather than one test per tool.

Verification note

The three violations above were each confirmed by cross-checking a second tool on the same server in the same session, not inferred from reading code.

> 🤖 Filed by Claude Code on Kai's behalf. ## Why this is filed as an invariant, not a tool bug Three separate commits have already fixed this exact shape, one tool at a time: - `abcf1b0` — `fix(status): report absent /info numbers as null, never zero` - `19513c1` — `fix(civics): report null for exporters that could not be read` - `dd09786` — `fix(economy): separate unmeasured from measured-zero, and read the treasury` The shape is still live in three tools at `5e05296`, including the one `dd09786` named. Fixing the next demonstrated instance has not converged across three rounds. The rule needs stating once and applying to the whole surface. This class matters more than its severity suggests: none of these tools error. They return well-formed, confident, wrong numbers, which a downstream caller cannot detect without cross-checking a sibling tool. ## The rule 1. A numeric field MUST be `null` when the dataset behind it could not be read. 2. `0` MUST mean the server reported no activity. 3. A narrative or summary string MUST NOT assert a fact about the world that rests on an unread dataset. "Cannot read this server's ledger" is correct; "nothing has been traded" is not. 4. Any tool exposing nullable values MUST also name the datasets behind its nulls. `get_economy.datasets_unavailable` is the reference shape; `get_world`'s per-dataset `HTTP 401` warnings are the reference behaviour. ## Confirmed violations at `5e05296` (observed 2026-08-13) ### 1. `get_economy.govt_funds` reports 0 for a funded treasury Against `eco.coilysiren.me:3001`: ``` get_economy → govt_funds: 0, datasets_unavailable: [], health: "healthy" get_currency → money.governmentHoldings: 87912.99 ``` `get_economy.govt_funds_note` states the value is read from "the same dataset get_currency reports as `money.governmentHoldings`". Same dataset, same server, same minute, two answers. `datasets_unavailable: []` actively asserts nothing was missing. `wages_total`, `taxes_paid`, `govt_funds_received` and `net_tax_flow` are also 0, and `health: "healthy"` plus the narrative "Economy is healthy" are derived from them. ### 2. `get_currency` converts an auth failure into a claim about the world Against `daokingdom.eu:3001`: ``` narrative: "no currencies have been created or traded yet - early in the cycle" counts.total: 0 admin_ok: true money.activeCurrencies: 40 ← same payload economy_desc: "1773 trades, 5 contracts" ← same payload ``` The server has 40 active currencies and 1,773 trades. `get_world` against the same host in the same session returns `HTTP 401` for all nine datasets it tries, so the access failure is unambiguous and a sibling tool already reports it correctly. `admin_ok: true` is also wrong here. ### 3. `price_recipe` ranks the least-known recipe cheapest `price_recipe(product=IronBarItem)` returns three recipes: | recipe | totalCost | ingredientCost | priced ingredients | |---|---|---|---| | `RecycledIronBar` | **0.22** | **0** | 0 of 2 | | `IronBar` | 1.46 | 1.28 | 1 of 2 | | `SmeltIron` | 3.18 | 2.56 | 1 of 2 | Sorted by cost, the recipe where *nothing* could be priced wins by 7×. `ingredientCost: 0` here means "no leaf resolved", not "ingredients are free". `perUnitCost` correctly goes `null` when `complete: false`; `totalCost` and `ingredientCost` do not, so the only comparable-looking number is the corrupted one. Same tool against a server with no readable market (`daokingdom.eu:3001`, `product=SteelAxeItem`) returns `totalCost: 0` with seven entries in `unpricedInputs`. Credit where due: `unpricedInputs` resolves leaves recursively and is correct. The bug is confined to the summed fields. ## Acceptance criteria - [ ] `get_economy.govt_funds` returns the treasury balance, or `null` with the dataset named in `datasets_unavailable`. - [ ] `get_currency` reports unreadable-ledger as unreadable; `admin_ok` reflects actual admin reachability per server. - [ ] `price_recipe.totalCost` and `.ingredientCost` are `null` whenever `complete: false`, matching `perUnitCost`. - [ ] All 25 MCP tools audited against rules 1–4 above, with the audit recorded as a table in the PR body: tool · nullable numeric fields · datasets-unavailable field present Y/N · verified. - [ ] A test asserts the invariant generically — an unreadable dataset yields `null` and a populated unavailable-list — rather than one test per tool. ## Verification note The three violations above were each confirmed by cross-checking a second tool on the same server in the same session, not inferred from reading code.
Author
Member

Claiming violation 3 only — price_recipe. Angie (ENG, claude seat), 20 minutes from this comment after the one minute buffer. Not claiming the sweep.

I am being explicit about the boundary because this issue is deliberately framed as an invariant across 25 tools, and taking a slice of it without saying so would be the thing your own body warns about: "Fixing the next demonstrated instance has not converged across three rounds."

What I am taking

price_recipe(product=IronBarItem)

recipe               totalCost  ingredientCost  priced ingredients
RecycledIronBar           0.22               0  0 of 2
IronBar                   1.46            1.28  1 of 2
SmeltIron                 3.18            2.56  1 of 2

Sorted by cost, the recipe where nothing resolved wins by 7x. ingredientCost: 0 means "no leaf priced", not "ingredients are free", and it is the only comparable-looking number in the row.

The correct behaviour is already in the same payload. perUnitCost goes null when complete: false. totalCost and ingredientCost do not. So this is one function agreeing with itself rather than a new rule.

That makes it the cheapest of your three violations and the one least likely to need a design call.

Why not the other two now

get_economy.govt_funds returning 0 against a treasury of 87,912.99 needs the dataset wiring understood before I touch it, and datasets_unavailable: [] asserting nothing was missing is a second bug in the same tool. Bigger than it looks.

get_currency turning a 401 into "no currencies have been created or traded yet" spans auth handling and narrative generation, and admin_ok: true is wrong on the same payload.

Both are real and both deserve more than the end of a session.

What I will not do

Claim this issue closed. Your acceptance is an audit of all 25 tools plus a generic invariant test, and one violation fixed is not that. I will reference this issue and leave it open.

I will also report if the fix makes an existing test red rather than adjusting the test — price_recipe's ordering is presumably asserted somewhere, and if a test currently encodes the wrong ranking that is worth seeing rather than quietly rewriting.

Same defect class as #269, which I fixed a few minutes ago: a number that looks measured and is not.

**Claiming violation 3 only — `price_recipe`. Angie (ENG, `claude` seat), 20 minutes from this comment after the one minute buffer. Not claiming the sweep.** I am being explicit about the boundary because this issue is deliberately framed as an invariant across 25 tools, and taking a slice of it without saying so would be the thing your own body warns about: *"Fixing the next demonstrated instance has not converged across three rounds."* ## What I am taking ``` price_recipe(product=IronBarItem) recipe totalCost ingredientCost priced ingredients RecycledIronBar 0.22 0 0 of 2 IronBar 1.46 1.28 1 of 2 SmeltIron 3.18 2.56 1 of 2 ``` Sorted by cost, the recipe where **nothing** resolved wins by 7x. `ingredientCost: 0` means "no leaf priced", not "ingredients are free", and it is the only comparable-looking number in the row. **The correct behaviour is already in the same payload.** `perUnitCost` goes `null` when `complete: false`. `totalCost` and `ingredientCost` do not. So this is one function agreeing with itself rather than a new rule. That makes it the cheapest of your three violations and the one least likely to need a design call. ## Why not the other two now **`get_economy.govt_funds`** returning 0 against a treasury of 87,912.99 needs the dataset wiring understood before I touch it, and `datasets_unavailable: []` asserting nothing was missing is a second bug in the same tool. Bigger than it looks. **`get_currency`** turning a 401 into *"no currencies have been created or traded yet"* spans auth handling and narrative generation, and `admin_ok: true` is wrong on the same payload. Both are real and both deserve more than the end of a session. ## What I will not do **Claim this issue closed.** Your acceptance is an audit of all 25 tools plus a generic invariant test, and one violation fixed is not that. I will reference this issue and leave it open. I will also report if the fix makes an existing test red rather than adjusting the test — `price_recipe`'s ordering is presumably asserted somewhere, and if a test currently encodes the wrong ranking that is worth seeing rather than quietly rewriting. Same defect class as https://forgejo.coilysiren.me/coilyco-gaming/eco-app/issues/269, which I fixed a few minutes ago: a number that looks measured and is not.
Author
Member

Violation 3 built. PR #272. Claim released. This issue stays open — it is a slice, not the sweep.

"perUnitCost": self.per_unit_cost,
"totalCost": self.total_cost if self.complete else None,
"ingredientCost": self.ingredient_cost if self.complete else None,

One payload now agrees with itself. perUnitCost already did this; the two summed fields did not.

The frontend already knew the right answer

This is the part worth recording, because it shipped for free and I did not have to design it:

// UsesPrice.tsx:388
const ap = ac.perUnitCost ?? Number.POSITIVE_INFINITY
const bp = bc.perUnitCost ?? Number.POSITIVE_INFINITY

The recipe sort already treats an unknown cost as infinitely expensive, so an unpriced recipe sorts last rather than winning. That is exactly the behaviour your IronBarItem table wanted, and it was already correct for perUnitCost — it just never applied to the field the tool actually reported. Widening the two types to number | null makes the existing sort do the right thing for them too.

The ingredient-cost cell takes the same null guard the per-unit cell already had.

What is not lost

ingredients[] still carries every resolved leaf and unpricedInputs still names what did not, so a caller can see how far the rollup got. Your note that unpricedInputs resolves recursively and is correct still holds — I did not touch it. Only the summary fields that looked comparable and were not.

Validation

ward exec test            827 passed
ward exec lint            121 files, no issues
ward exec frontend-test   179 passed, 34 files
ward exec frontend-build  typecheck + build clean
ward exec frontend-lint   clean

No existing test changed. test_cost.py:248's ingredientCost == 26.0 assertion passes untouched, because that rollup is complete — which is the distinction the fix turns on.

Mutation: restoring the partial sum fails test_an_incomplete_rollup_reports_no_total_rather_than_zero and nothing else.

Still open here, and I am not claiming otherwise

  • get_economy.govt_funds returning 0 against a treasury of 87,912.99, with datasets_unavailable: [] asserting nothing was missing.
  • get_currency turning a 401 into "no currencies have been created or traded yet", with admin_ok: true on the same payload.
  • The audit of all 25 tools and the generic invariant test, which is what your acceptance actually asks for.

Your framing is right that fixing one demonstrated instance at a time has not converged across three rounds. This is a fourth instance and I am not pretending it is the rule.

**Violation 3 built. PR https://forgejo.coilysiren.me/coilyco-gaming/eco-app/pulls/272. Claim released. This issue stays open — it is a slice, not the sweep.** ```python "perUnitCost": self.per_unit_cost, "totalCost": self.total_cost if self.complete else None, "ingredientCost": self.ingredient_cost if self.complete else None, ``` One payload now agrees with itself. `perUnitCost` already did this; the two summed fields did not. ## The frontend already knew the right answer This is the part worth recording, because it shipped for free and I did not have to design it: ```ts // UsesPrice.tsx:388 const ap = ac.perUnitCost ?? Number.POSITIVE_INFINITY const bp = bc.perUnitCost ?? Number.POSITIVE_INFINITY ``` **The recipe sort already treats an unknown cost as infinitely expensive**, so an unpriced recipe sorts last rather than winning. That is exactly the behaviour your `IronBarItem` table wanted, and it was already correct for `perUnitCost` — it just never applied to the field the tool actually reported. Widening the two types to `number | null` makes the existing sort do the right thing for them too. The ingredient-cost cell takes the same null guard the per-unit cell already had. ## What is not lost `ingredients[]` still carries every resolved leaf and `unpricedInputs` still names what did not, so a caller can see how far the rollup got. **Your note that `unpricedInputs` resolves recursively and is correct still holds** — I did not touch it. Only the summary fields that looked comparable and were not. ## Validation ``` ward exec test 827 passed ward exec lint 121 files, no issues ward exec frontend-test 179 passed, 34 files ward exec frontend-build typecheck + build clean ward exec frontend-lint clean ``` **No existing test changed.** `test_cost.py:248`'s `ingredientCost == 26.0` assertion passes untouched, because that rollup is complete — which is the distinction the fix turns on. Mutation: restoring the partial sum fails `test_an_incomplete_rollup_reports_no_total_rather_than_zero` and nothing else. ## Still open here, and I am not claiming otherwise - **`get_economy.govt_funds`** returning 0 against a treasury of 87,912.99, with `datasets_unavailable: []` asserting nothing was missing. - **`get_currency`** turning a 401 into *"no currencies have been created or traded yet"*, with `admin_ok: true` on the same payload. - **The audit of all 25 tools** and the generic invariant test, which is what your acceptance actually asks for. Your framing is right that fixing one demonstrated instance at a time has not converged across three rounds. This is a fourth instance and I am not pretending it is the rule.
Author
Member

Claiming violation 1, get_economy.govt_funds. Angie (ENG, claude seat), 20 minutes from this comment after the one minute buffer.

I deferred this earlier saying it needed the dataset wiring understood first. That was the right instinct and the wrong conclusion — understanding it is the work, and I have since done the same kind of tracing on four other tools in this repository.

Why this one ahead of anything left on #267

Everything I have fixed today was a bound or a caveat: a payload too big, a truncation unannounced, a check reporting success it never ran. This one returns a wrong number and builds a verdict on it.

get_economy   govt_funds: 0,  datasets_unavailable: [],  health: "healthy"
get_currency  money.governmentHoldings: 87912.99

Same dataset, same server, same minute, two answers. And get_economy's own govt_funds_note says the value comes from "the same dataset get_currency reports as money.governmentHoldings", so the tool documents the contradiction it is producing.

wages_total, taxes_paid, govt_funds_received and net_tax_flow are also 0, and health: "healthy" plus the narrative are derived from them — so a reader is not merely given a wrong figure, they are given a conclusion drawn from it.

datasets_unavailable: [] is the part that makes it undetectable: the tool actively asserts nothing was missing.

What I will establish before changing anything

Whether the treasury read fails or returns zero. Those need opposite fixes — a failed read must become null with the dataset named, per your rule 1 and 4; a genuine zero must stay 0, per rule 2. Getting that backwards would replace a wrong number with a wrong null, and dd09786 already did this work once for other fields in this tool, so there is a pattern to follow rather than invent.

Whether health and the narrative can be computed at all when an input is null. Your rule 3 says a narrative must not assert a fact resting on an unread dataset. If the answer is that health becomes unavailable rather than "healthy", that is a behaviour change worth naming rather than sliding in.

If it turns out the read succeeds and the value is genuinely lost in translation, I will say so — that is a different bug from the one this issue describes.

Not taking

get_currency's 401-as-a-claim and admin_ok: true, and price_recipe which #272 already fixed. The 25-tool audit and generic invariant test stay open too.

**Claiming violation 1, `get_economy.govt_funds`. Angie (ENG, `claude` seat), 20 minutes from this comment after the one minute buffer.** I deferred this earlier saying it needed the dataset wiring understood first. That was the right instinct and the wrong conclusion — understanding it *is* the work, and I have since done the same kind of tracing on four other tools in this repository. ## Why this one ahead of anything left on #267 Everything I have fixed today was a bound or a caveat: a payload too big, a truncation unannounced, a check reporting success it never ran. **This one returns a wrong number and builds a verdict on it.** ``` get_economy govt_funds: 0, datasets_unavailable: [], health: "healthy" get_currency money.governmentHoldings: 87912.99 ``` Same dataset, same server, same minute, two answers. And `get_economy`'s own `govt_funds_note` says the value comes from *"the same dataset get_currency reports as `money.governmentHoldings`"*, so the tool documents the contradiction it is producing. `wages_total`, `taxes_paid`, `govt_funds_received` and `net_tax_flow` are also 0, and **`health: "healthy"` plus the narrative are derived from them** — so a reader is not merely given a wrong figure, they are given a conclusion drawn from it. `datasets_unavailable: []` is the part that makes it undetectable: the tool actively asserts nothing was missing. ## What I will establish before changing anything **Whether the treasury read fails or returns zero.** Those need opposite fixes — a failed read must become `null` with the dataset named, per your rule 1 and 4; a genuine zero must stay `0`, per rule 2. Getting that backwards would replace a wrong number with a wrong null, and `dd09786` already did this work once for other fields in this tool, so there is a pattern to follow rather than invent. **Whether `health` and the narrative can be computed at all when an input is null.** Your rule 3 says a narrative must not assert a fact resting on an unread dataset. If the answer is that `health` becomes unavailable rather than "healthy", that is a behaviour change worth naming rather than sliding in. If it turns out the read succeeds and the value is genuinely lost in translation, I will say so — that is a different bug from the one this issue describes. ## Not taking `get_currency`'s 401-as-a-claim and `admin_ok: true`, and `price_recipe` which https://forgejo.coilysiren.me/coilyco-gaming/eco-app/pulls/272 already fixed. The 25-tool audit and generic invariant test stay open too.
Author
Member

Claiming violation 2, get_currency. Angie (ENG, claude seat), 20 minutes from this comment after the one minute buffer.

#279 fixed violation 1 by finding that _fetch_dataset returned an empty list for any 200, so an unparseable body became a confident zero. This is the same family and the last violation on your list.

What makes it the sharpest of the three

The payload contradicts itself:

narrative:              "no currencies have been created or traded yet"
counts.total:            0
admin_ok:                true
money.activeCurrencies:  40        <- same payload
economy_desc:            "1773 trades, 5 contracts"   <- same payload

A reader does not need a second tool to catch this one. Violation 1 needed get_currency to disprove get_economy; here the disproof is three lines down in the same response.

And get_world against the same host returns HTTP 401 for all nine datasets it tries, so the access failure is unambiguous and a sibling tool already reports it correctly — which is your rule 4's reference behaviour.

What I will establish first

Whether admin_ok: true is wrong or is answering a different question. It may mean "a token was configured" rather than "the admin surface answered", in which case it is technically true and badly named, and the fix is different from making it false. fetch_currency sets it near the token load, which is what makes me suspect the former.

Where the narrative is generated, and whether it can see the failure. Your rule 3 says a narrative must not assert a fact resting on an unread dataset. If the narrative is built from counts.total alone it cannot know, and the fix belongs where that count is derived rather than in the wording.

What I am not doing

Touching the holder cap. That is #267's rule 3, it sits behind a cached fetch, and I documented the trap there rather than walking into it. Same tool, different defect, and I am not bundling them.

I will report if the narrative turns out to be correct for a genuinely empty server and only wrong under 401 — that would make this a smaller fix than the issue implies, and worth saying.

**Claiming violation 2, `get_currency`. Angie (ENG, `claude` seat), 20 minutes from this comment after the one minute buffer.** https://forgejo.coilysiren.me/coilyco-gaming/eco-app/pulls/279 fixed violation 1 by finding that `_fetch_dataset` returned an empty list for any 200, so an unparseable body became a confident zero. This is the same family and the last violation on your list. ## What makes it the sharpest of the three The payload contradicts itself: ``` narrative: "no currencies have been created or traded yet" counts.total: 0 admin_ok: true money.activeCurrencies: 40 <- same payload economy_desc: "1773 trades, 5 contracts" <- same payload ``` **A reader does not need a second tool to catch this one.** Violation 1 needed `get_currency` to disprove `get_economy`; here the disproof is three lines down in the same response. And `get_world` against the same host returns `HTTP 401` for all nine datasets it tries, so the access failure is unambiguous and a sibling tool already reports it correctly — which is your rule 4's reference behaviour. ## What I will establish first **Whether `admin_ok: true` is wrong or is answering a different question.** It may mean "a token was configured" rather than "the admin surface answered", in which case it is technically true and badly named, and the fix is different from making it false. `fetch_currency` sets it near the token load, which is what makes me suspect the former. **Where the narrative is generated**, and whether it can see the failure. Your rule 3 says a narrative must not assert a fact resting on an unread dataset. If the narrative is built from `counts.total` alone it cannot know, and the fix belongs where that count is derived rather than in the wording. ## What I am not doing **Touching the holder cap.** That is #267's rule 3, it sits behind a cached fetch, and I documented the trap there rather than walking into it. Same tool, different defect, and I am not bundling them. I will report if the narrative turns out to be correct for a genuinely empty server and only wrong under 401 — that would make this a smaller fix than the issue implies, and worth saying.
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/eco-app#266
No description provided.