docs: put the reviewer in front of what is true on 2026-08-07 - #33
Merged
Conversation
REVIEW-READY.md was written across sessions and had accumulated a third layer of
stale claims, in a document whose whole value is that a reviewer can trust it
without having been in those sessions. Every claim below was re-checked against
the deployment, the metrics or the manifests rather than against the previous
version of this file.
WHAT WAS WRONG, and how each was checked:
- Section 4.8 said the control-plane limiter's production behaviour was
"unexplained". It is explained and SHARK-3592 is closed: the limiter refuses,
and mcp_ankr_refusals_total{reason="bucket_empty"} stood at 150. The competing
"several replicas, several buckets" explanation is ruled out by up{} returning
exactly one series per plane.
- Section 4b's live table said GET /healthz answers 404. It answers 200, and so
does /readyz; that was the readiness gap SHARK-3607 closed. /metrics answering
404 from the public host is now stated as the intended result rather than left
looking like a missing endpoint, since it is served on its own listener and
scraped in-cluster.
- Section 4b said build identity was half solved and the missing half was ours.
Both halves shipped. Both planes answer 0.2.0+<sha> on initialize and carry
the same fact on mcp_ankr_build_info.
- Section 4b said the two planes run different source states. src/ is
byte-identical between the two RC branches and both rolled together in
infrastructure-k8s PR #2093.
- Section 5 listed SHARK-3596 as not in this branch. It is in: mgmt schemas are
strict, and getTokenPrice takes `chain` with `blockchain` as a documented
deprecated alias that refuses both-at-once.
- Section 5 listed the e2e run against a deployed build as missing. It is PR #32
and it scores 26/26 against what is serving.
- Section 6 carried seven questions for Aleksandr Balev. Two remain, and both are
decisions rather than lookups. The rest closed: some by his work, some by the
observability in SHARK-3607, and three by reading a file rather than asking
anyone, which is recorded as such because it is the least flattering way for a
question to close.
WHAT IS NEW AND IS NOT A CORRECTION:
- The Istio routing is now QUOTED in 4b instead of described, because it is
committed YAML in infrastructure-k8s. So the old summary "production matches no
manifest anywhere" is half wrong and is restated: it matches manifests, just
not the ones in this repository.
- Two VirtualServices share one host and one gateway and the management one is a
catch-all, so the data plane answers only because the specific /rpc prefix is
evaluated first. Nobody has confirmed that ordering is guaranteed. Recorded as
a live single point of failure with the cheap fix (one VirtualService, or an
explicit match set instead of a catch-all).
- The route timeout on long-lived SSE streams is genuinely unknown: neither
VirtualService sets one. A stream cut by a default looks like the server going
quiet rather than erroring, which is the worst shape this failure can take.
- The charts still carry ingress.enabled: true, a Traefik stripPrefix on /rpc and
a 128Mi request. Production is safe only because the per-cluster values
override two of the three. Restated as armed-and-stepped-around rather than
fixed.
- Production runs a chart version that says -rc.1, which is deliberate but leaves
a release decision pending.
USER-STORIES.md: row 4.5 said DONE while it fails in production on at least one
account (gateway 504 on transactionHistory, SHARK-3593). The row now carries that
where a reader meets it. The legend gained a paragraph on what DONE claims and
what it does not, plus the two facts a reader needs to hold: this branch is what
is serving, and every store is per process on one pod per plane.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-checks every deployment and status claim in
REVIEW-READY.mdagainst the live deployment, the metrics and theinfrastructure-k8smanifests, rather than against the previous version of the file. Full reasoning is in the commit message.Corrections (each verified, not inferred): 4.8 the limiter is explained and SHARK-3592 is closed; 4b
/healthzanswers 200 not 404; build identity shipped on both halves; the two planes are back on one source state; SHARK-3596 has landed; the e2e run against a deployed build is PR #32 at 26/26; section 6 went from seven open questions to two.New, not corrections: the Istio routing is quoted rather than described; two VirtualServices share one host and one gateway with the management one as a catch-all, which is a live single point of failure with no test behind it; the SSE route timeout is genuinely unknown; the charts still carry
ingress.enabled: true, a Traefik stripPrefix on/rpcand a 128Mi request, disarmed only by per-cluster overrides.USER-STORIES.md: row 4.5 said DONE while it fails in production on at least one account (SHARK-3593); the row now says so where a reader meets it, and the legend gained a paragraph on what DONE claims and what it does not.No code changes.
format:checkclean.