fix: one source of truth for what counts as rate limited #10
No reviewers
Labels
No labels
waiting-on-julian
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
jlxq0/hevy-mcp!10
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/one-source-of-truth-for-rate-limited"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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_workoutsand 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 atde0cd24d. Two things on this side made the error harder to use than it should have been.What the boundary returns, measured through
/mcpAll four are
HTTP 200, because that is how JSON-RPC over streamable HTTP works. The discriminator is theerrorkey against theresultkey.error,-32012,data.class = "rate_limited"error,-32029,data.class = "rate_limited"result,isError: false,workouts: []result,isError: false,workout_count: 0The two fields that disagreed about one event
emit_tool_auditcounted both-32029and-32012asrate_limited.audit::error_classknew only-32029and filed-32012under"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_codeis now the only place a JSON-RPC code becomes a class. The auditoutcome, theerror_classfield and thedata.classon 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 nodataat 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, withdata.codestill 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_errordirectly. It passed. It then stayed green when the local limiter was reverted to carrying nodataat 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_workoutsthrough/mcpwith a one-per-minute limiter. Both mutations are red against the version being merged: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 --lockedpasses, 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_workoutson a probe bearer returned-32029with 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.