fix: one source of truth for what counts as rate limited #10

Merged
jlxq0 merged 1 commit from fix/one-source-of-truth-for-rate-limited into main 2026-08-28 03:46:12 +00:00
Owner

A rate limit and an empty read were already distinguishable at this boundary, and the incident still happened: the 21:00 slot that reports whether a gym day went unlogged read a rate-limited count_workouts and said "no workout logged" on a day Julian trained. That caller had an error object in its hands and discarded it, and it is fixed in its own repository at de0cd24d. Two things on this side made the error harder to use than it should have been.

What the boundary returns, measured through /mcp

All four are HTTP 200, because that is how JSON-RPC over streamable HTTP works. The discriminator is the error key against the result key.

condition envelope
Hevy answers 429 error, -32012, data.class = "rate_limited"
our per-bearer limiter error, -32029, data.class = "rate_limited"
empty workout list result, isError: false, workouts: []
count of zero result, isError: false, workout_count: 0

The two fields that disagreed about one event

emit_tool_audit counted both -32029 and -32012 as rate_limited. audit::error_class knew only -32029 and filed -32012 under "other". So a Hevy 429 was rate-limited in one field of an audit event and unclassified in another, and the metric undercounted exactly the case that started this: "how often is a Hevy read rate-limited" was not a series that existed.

audit::class_for_code is now the only place a JSON-RPC code becomes a class. The audit outcome, the error_class field and the data.class on the wire all derive from it, so they cannot disagree by construction rather than by being kept in step.

The two rate limits that had different shapes

Hevy's 429 carried data.code. Our own limiter carried no data at all, so a caller matching the field that exists to be matched saw one rate limit and missed the other, and its fallback was a numeric code or substring-matching an English sentence. A message rewording then becomes an outage.

Both now carry data.class, which is the one predicate a caller needs, with data.code still distinguishing ours from Hevy's underneath it.

No retry. A retry that succeeds hides how often the first read fails, and that rate is the measurement worth having.

The test was wrong first, and that is the part worth reading

The first version of the caller test built both errors by calling structured_error directly. It passed. It then stayed green when the local limiter was reverted to carrying no data at all, which is precisely the defect it existed to catch. A test that pins a constructor pins nothing about the call site.

It now drives a real count_workouts through /mcp with a one-per-minute limiter. Both mutations are red against the version being merged:

error_class forgets -32012        2 tests red
local limiter carries no data     1 test red:
  local rate limit is invisible to a caller matching data.class:
  {"error":{"code":-32029,"message":"rate limit exceeded"},"id":2,"jsonrpc":"2.0"}

The second test asserts the agreement between the wire class, the audit class and the outcome for six codes, rather than any one of their values, because the defect was the disagreement.

Verification

Gates on 1.98.0 through the build slot: fmt, clippy, test (48 passed), audit, deny. cargo +1.93.0 check --all-features --locked passes, so the build floor is unmoved.

Production behaviour was measured before any of this, and the method carries to the release check: ninety concurrent count_workouts on a probe bearer returned -32029 with a shape identical to the local test, so the deployed binary matches the tree rather than the tree being read as a proxy for it.

A rate limit and an empty read were **already** distinguishable at this boundary, and the incident still happened: the 21:00 slot that reports whether a gym day went unlogged read a rate-limited `count_workouts` and said "no workout logged" on a day Julian trained. That caller had an error object in its hands and discarded it, and it is fixed in its own repository at `de0cd24d`. Two things on this side made the error harder to use than it should have been. ## What the boundary returns, measured through `/mcp` All four are `HTTP 200`, because that is how JSON-RPC over streamable HTTP works. The discriminator is the `error` key against the `result` key. | condition | envelope | |---|---| | Hevy answers 429 | `error`, `-32012`, `data.class = "rate_limited"` | | our per-bearer limiter | `error`, `-32029`, `data.class = "rate_limited"` | | empty workout list | `result`, `isError: false`, `workouts: []` | | count of zero | `result`, `isError: false`, `workout_count: 0` | ## The two fields that disagreed about one event `emit_tool_audit` counted both `-32029` and `-32012` as `rate_limited`. `audit::error_class` knew only `-32029` and filed `-32012` under `"other"`. So a Hevy 429 was rate-limited in one field of an audit event and unclassified in another, and **the metric undercounted exactly the case that started this**: "how often is a Hevy read rate-limited" was not a series that existed. `audit::class_for_code` is now the only place a JSON-RPC code becomes a class. The audit `outcome`, the `error_class` field and the `data.class` on the wire all derive from it, so they cannot disagree by construction rather than by being kept in step. ## The two rate limits that had different shapes Hevy's 429 carried `data.code`. Our own limiter carried no `data` at all, so a caller matching the field that exists to be matched saw one rate limit and missed the other, and its fallback was a numeric code or substring-matching an English sentence. A message rewording then becomes an outage. Both now carry `data.class`, which is the one predicate a caller needs, with `data.code` still distinguishing ours from Hevy's underneath it. **No retry.** A retry that succeeds hides how often the first read fails, and that rate is the measurement worth having. ## The test was wrong first, and that is the part worth reading The first version of the caller test built both errors by calling `structured_error` directly. It passed. It then **stayed green when the local limiter was reverted to carrying no `data` at all**, which is precisely the defect it existed to catch. A test that pins a constructor pins nothing about the call site. It now drives a real `count_workouts` through `/mcp` with a one-per-minute limiter. Both mutations are red against the version being merged: ``` error_class forgets -32012 2 tests red local limiter carries no data 1 test red: local rate limit is invisible to a caller matching data.class: {"error":{"code":-32029,"message":"rate limit exceeded"},"id":2,"jsonrpc":"2.0"} ``` The second test asserts the **agreement** between the wire class, the audit class and the outcome for six codes, rather than any one of their values, because the defect was the disagreement. ## Verification Gates on 1.98.0 through the build slot: `fmt`, `clippy`, `test` (48 passed), `audit`, `deny`. `cargo +1.93.0 check --all-features --locked` passes, so the build floor is unmoved. Production behaviour was measured before any of this, and the method carries to the release check: ninety concurrent `count_workouts` on a probe bearer returned `-32029` with a shape identical to the local test, so the deployed binary matches the tree rather than the tree being read as a proxy for it.
fix: one source of truth for what counts as rate limited
All checks were successful
CI / cargo (pull_request) Successful in 38s
CI / docker (pull_request) Successful in 31s
63061ea223
A rate limit and an empty read were already distinguishable at the tool
boundary and still cost a wrong answer: the 21:00 slot that reports
whether a gym day went unlogged read a rate-limited count_workouts and
said "no workout logged" on a day Julian trained. That caller discarded
an error object and is fixed in its own repository. Two things on this
side made the error harder to use than it should have been.

`emit_tool_audit` counted both -32029 and -32012 as rate_limited while
`error_class` knew only -32029 and filed -32012 under "other". One event,
two fields, disagreeing, and no series that answers "how often is a Hevy
read rate-limited" — the metric undercounted exactly the case that
started this. `audit::class_for_code` is now the only place a code
becomes a class, and the outcome, the error_class field and the wire all
derive from it, so they cannot disagree by construction rather than by
being kept in step.

And the two rate limits had different shapes on the wire. Hevy's 429
carried data.code; our own limiter carried no data at all, so a caller
matching the field that exists to be matched saw one and missed the
other, leaving it a numeric code or a substring match on an English
sentence. Both now carry data.class, one predicate for a caller, with
data.code still distinguishing ours from Hevy's underneath.

No retry. A retry that succeeds hides how often the first read fails,
and that rate is the measurement worth having.

The first version of the caller test built both errors by calling
structured_error directly. It passed, and it stayed green when the local
limiter was reverted to carrying no data at all, which is the defect it
existed to catch. It now drives a real count_workouts through /mcp with
a one-per-minute limiter. Both mutations are red against the version
that shipped:

  error_class forgets -32012      2 tests red
  local limiter carries no data   1 test red, quoting the envelope

Gates on 1.98.0: fmt, clippy, 48 tests, audit, deny. cargo +1.93.0 check
passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NCiHcHN6LrhiuLXbG9SBYG
jlxq0 merged commit 8d850d5548 into main 2026-08-28 03:46:12 +00:00
jlxq0 deleted branch fix/one-source-of-truth-for-rate-limited 2026-08-28 03:46:12 +00:00
Sign in to join this conversation.
No reviewers
No labels
waiting-on-julian
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
jlxq0/hevy-mcp!10
No description provided.