coilyco-flight-deck/mcp-beaver

Watch

coverage counts no arrays at all on a nested payload, so every GraphQL response arrives with an empty items block #88

New issue

Closed

opened 2026-08-19 09:06:41 +00:00 by coilyco-ops · 1 comment

coilyco-ops commented

2026-08-19 09:06:41 +00:00

Member

Filed by Angie (engineer, claude seat), as the runtime-side slice named in the #70 comment and confirmed by measurement once umbra#306 landed.

The gap, measured

countArrays (internal/mcpserver/coverage.go) walks exactly one level. It counts an array whose value sits directly under the top-level object, and stops. A payload that nests its arrays under objects therefore produces no items entry at all, not a partial one.

Measured live through a served mcp-beaver on 5802a9a, calling a graphql guardfile against graphql.anilist.co:

{"coverage":{"truncated":false,"bytes":201,"over_budget":false},
 "result":{"data":{"Page":{"media":[{"id":1,"title":{"romaji":"Cowboy Bebop"}}, ...]}}}}

Three media rows returned. items is absent. The array is at data.Page.media, two objects down.

This is the #68 failure mode arriving through a different door. That issue's own framing is that "a count in meaning is what changes an answer" and that a view reporting no count reads as though none exists. GraphQL is not an edge case here: data wrapping is the entire protocol, so every GraphQL response has this shape, and every one of them is currently uncounted.

Why the one-level rule exists, and why it is not what needs changing

The current comment states the rationale, and it is a good one:

One level deep on purpose: an array nested inside an array element is a per-row detail rather than a statement about the view, and enumerating those would put the coverage block itself over budget.

That reasoning is about arrays inside array elements. Counting the tags array on each of 200 rows is per-row noise and would bloat the block 200-fold. It says nothing about arrays inside plain objects, where data.Page.media is exactly a statement about the view.

So the fix is not to relax the rule but to apply it where it was aimed: descend through objects, never into array elements.

Shape

Walk nested objects, keying each count by its dotted path (data.Page.media).
Never descend into an array's elements, preserving the per-row rationale verbatim.
Bound the object descent, so a pathological payload cannot make the coverage block the expensive part of the response.
A top-level array stays result, and a top-level field keeps its bare name, so nothing already emitted changes spelling.

Also fixed by this

SQL grants (umbra#308) return {"rows":[...],"truncated":...,"columns":[...]}, which is flat and already counts. But an upstream that wraps its payload in data, result, response, or items is common well beyond GraphQL, and every one of those is silently uncounted today.

Acceptance

A nested array is counted and named by its path.
An array inside an array element is still not counted.
A top-level array still reports as result, and a top-level named array keeps its bare key.
The descent is depth-bounded.
Verified against a real GraphQL response, not only a fixture.

Not in scope

Refusing a GraphQL response carrying errors. That is the guardfile author's fail-when, per the split recorded on #70: umbra owns refusing a call, mcp-beaver owns projecting one.

**Filed by Angie (engineer, `claude` seat)**, as the runtime-side slice named in the `#70` comment and confirmed by measurement once `umbra#306` landed. ## The gap, measured `countArrays` (`internal/mcpserver/coverage.go`) walks exactly one level. It counts an array whose value sits directly under the top-level object, and stops. A payload that nests its arrays under objects therefore produces **no `items` entry at all**, not a partial one. Measured live through a served mcp-beaver on `5802a9a`, calling a `graphql` guardfile against `graphql.anilist.co`: ```json {"coverage":{"truncated":false,"bytes":201,"over_budget":false}, "result":{"data":{"Page":{"media":[{"id":1,"title":{"romaji":"Cowboy Bebop"}}, ...]}}}} ``` Three media rows returned. `items` is **absent**. The array is at `data.Page.media`, two objects down. This is the `#68` failure mode arriving through a different door. That issue's own framing is that "a count in meaning is what changes an answer" and that a view reporting no count reads as though none exists. GraphQL is not an edge case here: `data` wrapping is the entire protocol, so **every** GraphQL response has this shape, and every one of them is currently uncounted. ## Why the one-level rule exists, and why it is not what needs changing The current comment states the rationale, and it is a good one: > One level deep on purpose: an array nested inside an array element is a per-row detail rather than a statement about the view, and enumerating those would put the coverage block itself over budget. That reasoning is about arrays inside **array elements**. Counting the `tags` array on each of 200 rows is per-row noise and would bloat the block 200-fold. It says nothing about arrays inside plain **objects**, where `data.Page.media` is exactly a statement about the view. So the fix is not to relax the rule but to apply it where it was aimed: descend through objects, never into array elements. ## Shape * Walk nested objects, keying each count by its dotted path (`data.Page.media`). * Never descend into an array's elements, preserving the per-row rationale verbatim. * Bound the object descent, so a pathological payload cannot make the coverage block the expensive part of the response. * A top-level array stays `result`, and a top-level field keeps its bare name, so nothing already emitted changes spelling. ## Also fixed by this SQL grants (`umbra#308`) return `{"rows":[...],"truncated":...,"columns":[...]}`, which is flat and already counts. But an upstream that wraps its payload in `data`, `result`, `response`, or `items` is common well beyond GraphQL, and every one of those is silently uncounted today. ## Acceptance * A nested array is counted and named by its path. * An array inside an array element is still not counted. * A top-level array still reports as `result`, and a top-level named array keeps its bare key. * The descent is depth-bounded. * Verified against a real GraphQL response, not only a fixture. ## Not in scope Refusing a GraphQL response carrying `errors`. That is the guardfile author's `fail-when`, per the split recorded on `#70`: umbra owns refusing a call, mcp-beaver owns projecting one.

coilyco-ops referenced this issue from a commit

2026-08-19 09:08:59 +00:00

fix(coverage): count arrays nested under objects, so a GraphQL response stops reporting nothing

coilyco-ops closed this issue

2026-08-19 09:09:00 +00:00

coilyco-ops commented

2026-08-19 09:09:10 +00:00

Author

Member

Landed on main as e8eefc0. Built by Angie (engineer, claude seat).

Acceptance

A nested array is counted and named by its path. data.Page.media.
An array inside an array element is still not counted. {"rows":[{"tags":[...]},...]} reports rows and nothing else, so the per-row rationale the one-level rule was written for is preserved verbatim.
Nothing already emitted changes spelling. A top-level array is still result, a top-level named array keeps its bare key, and the two existing count tests pass unchanged.
The descent is depth-bounded at four object levels.
Verified against a real GraphQL response.

The measurement, same call before and after

Before, on 5802a9a:

"coverage":{"truncated":false,"bytes":201,"over_budget":false}

After, on e8eefc0:

"coverage":{"truncated":false,"bytes":201,"over_budget":false,"items":{"data.Page.media":3}}

Three media rows, now stated. Live through a served mcp-beaver against graphql.anilist.co, not a fixture.

What I did not change

The one-level rule was not relaxed, and that distinction is the whole design. Its stated rationale is about arrays inside array elements and it is a good rule: counting tags on each of 200 rows would multiply the coverage block by the row count. It simply says nothing about arrays inside plain objects, which is where data.Page.media lives and which is exactly a statement about the view.

So the walk descends through objects and refuses to enter an array's elements. Both properties are asserted.

Four levels covers data.<Query>.<field> with a layer to spare. A payload nesting deeper than that reports no count rather than an expensive one, which is the same trade the original bound made.

**Landed on `main` as `e8eefc0`.** Built by Angie (engineer, `claude` seat). ## Acceptance * **A nested array is counted and named by its path.** `data.Page.media`. * **An array inside an array element is still not counted.** `{"rows":[{"tags":[...]},...]}` reports `rows` and nothing else, so the per-row rationale the one-level rule was written for is preserved verbatim. * **Nothing already emitted changes spelling.** A top-level array is still `result`, a top-level named array keeps its bare key, and the two existing count tests pass unchanged. * **The descent is depth-bounded** at four object levels. * **Verified against a real GraphQL response.** ## The measurement, same call before and after Before, on `5802a9a`: ```json "coverage":{"truncated":false,"bytes":201,"over_budget":false} ``` After, on `e8eefc0`: ```json "coverage":{"truncated":false,"bytes":201,"over_budget":false,"items":{"data.Page.media":3}} ``` Three media rows, now stated. Live through a served mcp-beaver against `graphql.anilist.co`, not a fixture. ## What I did not change The one-level rule was not relaxed, and that distinction is the whole design. Its stated rationale is about arrays inside array **elements** and it is a good rule: counting `tags` on each of 200 rows would multiply the coverage block by the row count. It simply says nothing about arrays inside plain **objects**, which is where `data.Page.media` lives and which is exactly a statement about the view. So the walk descends through objects and refuses to enter an array's elements. Both properties are asserted. Four levels covers `data.<Query>.<field>` with a layer to spare. A payload nesting deeper than that reports no count rather than an expensive one, which is the same trade the original bound made.

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-flight-deck/mcp-beaver#88

No description provided.

Rows
Columns