fix(benchmarks): bound the k6 measurement window and gate on it - #156
Merged
Conversation
…stant-vus scenario
Every aspect script declared its load with the k6 vus/duration shorthand, which silently inherits the 30s default gracefulStop. requests_per_second is a counter rate over state.testRunDurationMs, and that duration spans the graceful-stop tail, so a single VU stalling at cutoff stretches the denominator without adding requests and deflates the reported rate by up to 1.5x on a 60s run.
Add scenario() and DEFAULT_GRACEFUL_STOP ('5s') to lib/summary.js and convert all 12 aspect scripts to scenarios.default, bounding worst-case window inflation to 5/60 = 8.3%. buildSummary behaviour is unchanged; the BENCHMARK_NAME constants and the passthrough_relay mode switch that benchmark-manifest.py regex-parses are untouched.
…omparable window pair The comparator read only requests_per_second and the two latency percentiles, never start_time or end_time, so it compared a 90.001s window against a 60.007s one and reported the resulting 1.5x rate deflation as a 35.8% throughput regression that never happened. Add windowMillis(), windowVerdict(), the WINDOW_MISMATCH verdict and a 10% DEFAULT_WINDOW_TOLERANCE (passthrough.baseline.window.tolerance), lead the rendered table with a measurement-window row, and extend regressed() to fail on the mismatch. Operator ruling recorded in the class javadoc: refuse and fail, never rescale and never downgrade to a warning; an absent window pair stays n/a and never fails. Additive - the existing PassthroughBaselineComparatorTest passes untouched (134 tests green). Adds K6Benchmark-211 and documents 5, 6 and 208-211 in doc/LogMessages.adoc.
…y arm `passthrough_sni` is a property of the whole gateway process: declaring it non-empty is what starts the accept-time SNI front listener. The primary instance declares two entries for its whole lifetime, so the benchmark's `passthroughRelayEmpty` arm — whose entire claim is that it measures the zero-overhead default where that listener is never created — was pointed at a gateway that always had one. Both arms of the no-regression gate issued the identical request to the identical listener stack on one instance: the gate was comparing a configuration against itself, and could neither pass nor fail for the reason it existed. Adds a seventh gateway instance, api-sheriff-passthrough-empty (10449 / 19006), reusing the same native image and overlaying a gateway.yaml that is the base descriptor with the `tls.passthrough_sni` block — and only that block — removed. It sets no QUARKUS_HTTP_SSL_PORT: with no front listener there is nothing to free the public port for. The benchmark overlay repoints it at nginx-static so both arms share the primary's upstream (ADR-0012 identical-upstreams parity). Makes the arm's emptiness a structural fact rather than a review convention. TlsEdgeActivationWiringTest gains four guards — no passthrough_sni declared, no internal-port split, parsed-structure equality with the base once passthrough_sni is removed, and upstream parity asserted as an equality between the two declared values rather than a hard-coded literal. Each was shown red against a deliberately broken descriptor before being accepted. Because the overlay is a deliberate base copy, it inherits the base's `allow_get_with_content_length_body: true`. GetWithBodyActivationWiringTest's strict-default drift guard is widened rather than weakened: base-derived descriptors are exempted by literal path enumeration — never by prefix, glob or substring — so a sibling nobody enumerated still fails the guard. A negative control pins that fail-closed property, and the enumeration is asserted to name only committed files so a stale entry cannot become a silent hole. Membership is earned by the equality guard above, not merely asserted. Re-derives the count claims the seventh instance invalidates across the compose comments, the declared-limit and topology notes, the topology diagram, the benchmark workflow's startup-budget comment, and the four COMMITTED_DESCRIPTOR_COUNT floors. Co-Authored-By: Claude <noreply@anthropic.com>
…tance D4 added the dedicated `api-sheriff-passthrough-empty` gateway instance, but the benchmark's empty-mode arm still resolved its URL through `targetUrl()` — so it kept issuing its request to the primary instance, which declares two `passthrough_sni` entries for its whole lifetime. The instance existed and nothing pointed at it; the no-regression gate would have gone on comparing a configuration against itself. `lib/target.js` gains `passthroughEmptyUrl(path)` over an exported `PASSTHROUGH_EMPTY_BASE_URL` constant, overridable via `__ENV.PASSTHROUGH_EMPTY_BASE_URL` in the same shape as the existing `TARGET_BASE_URL` override. It deliberately does NOT route through `gatewayTarget()`: the empty arm compares one API Sheriff configuration against another, so `GATEWAY_TARGET=apisix` would otherwise aim it at a gateway that has no such instance and no such configuration — producing a summary that is nonsense rather than a run that fails. The exclusion is documented in the module's `@fileoverview`. The repeated trailing-slash trim is factored into one helper now that two resolvers need it. `passthrough_relay.js` switches the empty branch to `passthroughEmptyUrl()` and drops the `targetUrl` import outright (clean break; eight surviving call sites across seven other scripts are unaffected). Its `@fileoverview` now states the real topology — a second gateway instance whose `gateway.yaml` declares no `passthrough_sni`, where the front listener is never created and Quarkus terminates on the public port directly — rather than describing emptiness as a mode of one process, which it cannot be. `BENCHMARK_NAME` and the mode switch are untouched; `benchmark-manifest.py generate` still resolves a goal name for all twelve wired executions and still reports `sessionMediated` as unwired. Documentation is corrected to the post-D4 topology rather than left describing the arm's former target: the POM execution comment, the README's aspect-table row and its two-modes methodology block — which now names the parity conditions the comparison rests on (same image, same route, same nginx-static upstream, same TLS material, same 512M/4.0-CPU limits, ADR-0012) and the four `TlsEdgeActivationWiringTest` assertions that enforce the single-variable and same-upstream properties structurally. `PassthroughBaselineComparator`'s class javadoc is corrected the same way, and states what the gate's asymmetry means: the baseline instance's front listener adds an accept-time ClientHello peek plus an L4 relay hop to every connection, so the empty arm is expected to measure at or above the baseline and `candidate >= baseline * (1 - tolerance)` is a one-sided bound. Javadoc only — the 109 existing comparator tests pass unmodified, which is the evidence no behaviour moved. Co-Authored-By: Claude <noreply@anthropic.com>
…erdict shape D3 added the window-comparability precondition; nothing yet asserted it against the shape that motivated it. Extends PassthroughBaselineComparatorTest with a summary-builder overload emitting the `start_time` / `end_time` pair, and covers the measurement window on the same terms as the existing bands. The regression case reproduces CI run 30872335137: both arms served the same request count, but the candidate's measured window ran ~1.56x the baseline's, so its counter-derived rate came out proportionally lower with nothing having slowed down. That pair must classify as WINDOW_MISMATCH and must fail the run — the second assertion pins the operator's ruling (refuse, never rescale, never downgrade to a warning) as executable behaviour rather than prose. Fail-without-fix is asserted explicitly rather than described: a third case feeds the same two rates to `throughputVerdict` alone and asserts it still says REGRESSION — the ~35.8% drop no request ever experienced, which is exactly the false verdict the lane shipped. The fix changes none of that arithmetic; it adds a row that runs first, so the pair is now refused as incomparable instead of blamed on throughput. The window-row assertion itself would not even resolve against the pre-D3 comparator, which had no such row. The matched negative control keeps the guard honest in the other direction: an equal-window half-throughput collapse must still read REGRESSION, attributed to throughput and explicitly NOT to the window. An absent window pair stays NOT_MEASURED and never fails, so summaries predating the fields are not retroactively broken, and one-sided absence is covered in both directions. Two property-style @RepeatedTest cases drive drift in BOTH directions — a window that ran short is as incomparable as one that ran long, the rate being a quotient either way — and one boundary case pins the band between the two magnitudes it was chosen to separate: the bounded 5s gracefulStop's worst case on a 60s run must pass, the observed 1.56x inflation must not. `windowMillis` is covered for the measured, absent and unparseable pair. The window-tolerance property gets the same three-case treatment as the existing tolerance property (default, valid override, rejection of out-of-range and malformed values), and the two suites now share one save/restore helper rather than a near-duplicate. Every band under test reads the comparator's own constants; no hand-copied literals. Co-Authored-By: Claude <noreply@anthropic.com>
… gate The pre-commit quality gate rewrote two files this plan authored: - PassthroughBaselineComparator: the multi-line concatenated report preamble becomes a text block. Semantically identical - the closing delimiter on its own line preserves the trailing "\n\n". - TlsEdgeActivationWiringTest: the two instanceof-else chains in environmentEntries() and dependsOnNames() become pattern-matching switches with `case null, default ->`, which covers the same else branch. Both files are inside this plan's footprint, so the churn is committed rather than reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MbbsDwgfoibRGPam1wrQxM
The collapsed variant group grew from 4 to 6 members, but the visible encl-sub caption still read "overlaid gateway.yaml" -- true of the original four, false of api-sheriff-plain-mgmt, which overlays a single environment variable instead. The file's own <desc> narrative and integration-test-topology.adoc already carried the carve-out, so the sighted reader got a story the screen-reader user did not. Text-only; no geometry change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MbbsDwgfoibRGPam1wrQxM
…h-baseline-gate Three post-implementation simplifications over this plan's own diff: - lib/summary.js: drop scenario()'s unused third parameter. All 12 aspect scripts call scenario(vus(N), duration()); the gracefulStop override had no caller. One lane-wide tail is also the stronger contract, since it is what makes the two arms' windows commensurable. - PassthroughBaselineComparator: remove the two single-caller private wrappers this changeset left on top of the detailFor(result, verdict) helper it had just extracted. - TlsEdgeActivationWiringTest: delete environment() in favour of the form-agnostic environmentEntries() this changeset added, and tighten the assertion from a list .contains() to an equality on the parsed value, which stops being vacuously green if compose switches to the mapping form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MbbsDwgfoibRGPam1wrQxM
The prober-measurement paragraph read "ran against all six gateway instances", which against today's seven-instance tree reads as a stale count rather than as evidence with a date. Reworded to "the six gateway instances that existed when it was taken", with a short paragraph recording that the table is dated evidence, that api-sheriff-passthrough-empty is absent by design, and that no row may be invented for it. The six data rows and the headroom argument are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MbbsDwgfoibRGPam1wrQxM
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 15 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe benchmark suite now uses dedicated k6 scenarios with graceful-stop timing, compares passthrough results only across comparable measurement windows, and runs empty passthrough mode against a separate gateway instance. Compose topology, descriptors, wiring tests, logs, and documentation now cover seven gateway instances. ChangesBenchmark comparison and execution
Gateway integration topology
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍(Review updated until commit 22f7320)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
integration-tests/scripts/dump-keycloak-logs.sh (1)
50-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the container list instead of hardcoding it.
The loop hardcodes a list that must mirror the
api-sheriff*services inintegration-tests/docker-compose.yml. The comment asks a contributor to keep the two in lockstep. That drift already happened once: this change records thatapi-sheriff-plain-mgmtwas missing until PLAN-46. The same class of miss returns with the next instance.The list also hardcodes the
integration-tests-project prefix. IfCOMPOSE_PROJECT_NAMEdiffers, thedocker psguard skips every container silently and the dump produces no log.Derive the names from the running containers instead:
♻️ Proposed refactor to derive the instance list
-for app in integration-tests-api-sheriff-1 \ - integration-tests-api-sheriff-mtls-1 \ - integration-tests-api-sheriff-cookie-1 \ - integration-tests-api-sheriff-cookie-2-1 \ - integration-tests-api-sheriff-ws-admission-1 \ - integration-tests-api-sheriff-plain-mgmt-1 \ - integration-tests-api-sheriff-passthrough-empty-1; do - if docker ps -a --format "{{.Names}}" | grep -q "^${app}$"; then - echo "📥 Dumping app logs: ${app} -> ${FAILSAFE_DIR}/${app}.log" - docker logs "$app" > "${FAILSAFE_DIR}/${app}.log" 2>&1 || true - fi -done +# Derived from the running containers rather than restated here, so a new api-sheriff* +# instance is dumped without a script edit and a renamed compose project still matches. +while IFS= read -r app; do + [ -n "$app" ] || continue + echo "📥 Dumping app logs: ${app} -> ${FAILSAFE_DIR}/${app}.log" + docker logs "$app" > "${FAILSAFE_DIR}/${app}.log" 2>&1 || true +done < <(docker ps -a --format "{{.Names}}" | grep -E '(^|-)api-sheriff(-|$)' || true)Based on learnings: treat a hardcoded list that must mirror a set defined elsewhere as a defect unless it is derived from that source at build or run time.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0eac9cd4-49b9-4aeb-b728-26b127945bfb
⛔ Files ignored due to path filters (1)
doc/resources/diagrams/integration-test-topology.svgis excluded by!**/*.svg
📒 Files selected for processing (32)
.github/workflows/benchmark.ymlbenchmarks/README.adocbenchmarks/pom.xmlbenchmarks/src/main/java/de/cuioss/sheriff/gateway/k6/benchmark/K6BenchmarkLogMessages.javabenchmarks/src/main/java/de/cuioss/sheriff/gateway/k6/benchmark/PassthroughBaselineComparator.javabenchmarks/src/main/resources/k6-scripts/bearer_proxied.jsbenchmarks/src/main/resources/k6-scripts/gateway_health.jsbenchmarks/src/main/resources/k6-scripts/graphql.jsbenchmarks/src/main/resources/k6-scripts/grpc_unary.jsbenchmarks/src/main/resources/k6-scripts/health_live.jsbenchmarks/src/main/resources/k6-scripts/http2.jsbenchmarks/src/main/resources/k6-scripts/lib/summary.jsbenchmarks/src/main/resources/k6-scripts/lib/target.jsbenchmarks/src/main/resources/k6-scripts/passthrough_relay.jsbenchmarks/src/main/resources/k6-scripts/proxied_static.jsbenchmarks/src/main/resources/k6-scripts/session_mediated.jsbenchmarks/src/main/resources/k6-scripts/upload_large.jsbenchmarks/src/main/resources/k6-scripts/upload_small.jsbenchmarks/src/main/resources/k6-scripts/websocket_echo.jsbenchmarks/src/test/java/de/cuioss/sheriff/gateway/k6/benchmark/PassthroughBaselineComparatorTest.javadoc/LogMessages.adocdoc/development/declared-limit-assertion-coverage.adocdoc/development/integration-test-topology.adocintegration-tests/docker-compose.benchmark.ymlintegration-tests/docker-compose.ymlintegration-tests/scripts/dump-keycloak-logs.shintegration-tests/src/main/docker/sheriff-config-passthrough-empty/gateway.yamlintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/AssetContentTypeActivationWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BodyLimitActivationWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/EgressAllowlistActivationWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/GetWithBodyActivationWiringTest.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/TlsEdgeActivationWiringTest.java
…tion Addresses the CodeRabbit review on #156. The window-comparability band was a fixed 10%, blind to k6.duration. The `quick` profile sets it to 30s, where the 5s graceful tail is 16.7% -- so a perfectly healthy quick run was rejected as WINDOW_MISMATCH. The band is now a fixed 6s absolute allowance (5s bounded gracefulStop + 1s start/stop skew) expressed as a fraction of the effective load phase, so 60s still yields exactly 10% and 30s yields 20%. A reverted or unbounded tail still fails at any configured duration. The explicit override property still wins outright. windowVerdict() compared only absolute drift, so two windows that were both zero -- or both reversed -- compared equal and passed, letting the comparator compare rates over an impossible measurement interval. Non-positive windows are now refused, placed after the absence check so an unmeasured pair stays NOT_MEASURED. dump-keycloak-logs.sh derived its container list from a hardcoded seven-name list that had to mirror docker-compose.yml -- the drift this plan had just fixed by hand. The list is now derived from `docker ps` anchored to the compose project prefix, resolved the same way start-integration-container.sh resolves it. Anchoring to the project (rather than a bare api-sheriff match) keeps a foreign stack's containers out; the keycloak container name carried the same hardcoded-prefix defect and is derived too. The declared-limit-assertion-coverage derivation rationale claimed "twelve rows". Counting the matrix: seven x4 rows plus seven bare sheriff-config/gateway.yaml rows, of which the tls.passthrough_sni row would gain no citation since that is the key the overlay drops -- thirteen rows would gain one, and "a fifth line number" binds to the seven x4 rows. The passage now names the rows and states both figures instead of a bare number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MbbsDwgfoibRGPam1wrQxM
Collaborator
Author
|
All CodeRabbit feedback addressed in 22f7320 — the three inline threads are answered and resolved individually. Two of them were real defects, and both are worth naming because they were false-negative/false-positive paths in a release-gating check:
On the
|
Collaborator
Author
|
/review |
Collaborator
Author
|
@coderabbitai review The previous run hit the OSS review limit before it could review |
|
Your plan includes PR reviews subject to rate limits. Reviews are available now. |
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.
Intent
The post-merge benchmark gate failed on
mainatb39b271(run30872335137):PassthroughBaselineComparatorreported the empty-passthrough_sniarm at 3221.18 RPS against a 5021.41 RPS in-run baseline — a 35.8% gap past the 15% noise band. This PR establishes what actually happened, fixes the cause at both layers, and makes the gate measure what it claims to measure.D1 — the verdict, settled against the run's own artifacts
The raw k6 summary JSONs are never uploaded as CI artifacts, so the evidence came from the run's own emitted
buildSummaryblocks in the attempt-1 job log:proxiedStatic(baseline)passthroughRelayMappedpassthroughRelayEmpty(candidate)Normalising both arms to the 60s load phase gives 5020.5 vs 4830.0 — a 3.79% gap, not 35.8%. The regression was a measurement artifact.
api-sheriff/src/main/**never entered scope; theClientHelloSniParserattribution stays refuted.Three further findings that shaped the fix:
01/50 VUs, 289,908; the count then sat frozen at 289,908 with one VU alive for the remaining 29 seconds, ending1 interrupted. The window was inflated by a stall, not by work — which is why the fix does not simply normalise the tail away.passthroughRelayEmpty,grpcUnary(2661.47 RPS),uploadLarge(144.29 RPS). All twelve used the shorthandvus/durationform and the sharedsummary.js.sheriff-config/gateway.yamldeclares a non-emptytls.passthrough_snifor the whole lane, andPASSTHROUGH_SNIonly re-points the k6 URL — it never reconfigures the gateway. Both arms were issuing a byte-identical request to the same instance.What changed
D2 — script layer. All 12 aspect scripts converted from the shorthand
vus/durationform to explicitscenarios.defaultvia a newscenario()helper inlib/summary.js, withgracefulStop: '5s'.gracefulStopis a k6 scenario-level option, so the shorthand form silently ignores it — the conversion is what makes the setting take effect at all. 5s bounds worst-case window inflation to 8.3% while avoiding the'0s'hazard, where interrupted iterations register as failures against the 0.01BENCHMARK_MAX_ERROR_RATEthreshold.D3 — comparator layer.
PassthroughBaselineComparatorgains a window term (windowMillis,windowVerdict,WINDOW_MISMATCH,DEFAULT_WINDOW_TOLERANCE = 0.10), derived from the existingstart_time/end_timepair — no summary schema change. On an incomparable pair the run fails with a diagnostic naming both windows rather than being rescaled: normalising would restore the gate to green in exactly the case where a stalled VU inflated the window, masking the defect.K6Benchmark-211added, plus the previously-undocumented 5, 6, 208–210.D4/D5 — the gate now measures what it claims. New
api-sheriff-passthrough-emptyinstance (10449/19006) with asheriff-config-passthrough-empty/gateway.yamlthat is the base descriptor with thetls.passthrough_sniblock — and only that block — removed, so it is the one instance that creates no SNI front listener. The empty benchmark arm is re-pointed at it viapassthroughEmptyUrl(). Four newTlsEdgeActivationWiringTestguards pin the single-variable property, the absentQUARKUS_HTTP_SSL_PORT, and same-upstream parity.The byte-identical base copy trips
GetWithBodyActivationWiringTest.siblingDescriptorsKeepTheStrictDefault. Settled by widening the guard rather than weakening the descriptor: an enumerated carve-out naming the exempt descriptors by literal path (no glob, no prefix match), plus a negative-control test asserting that a non-enumerated sibling declaring the opt-in still fails and that every enumerated entry must name a committed file.D6 — regression test.
PassthroughBaselineComparatorTestgrows 109 → 170 tests, including the run-30872335137 field shape classified asWINDOW_MISMATCH, the fail-without-fix contrast (throughputVerdictalone still saysREGRESSIONon that same pair), the equal-window collapse control, and the band-separation boundary case.Verification
verify -Ppre-commitand fullverifyboth green at2de2cff, run after the rebase onto #154. Each new wiring assertion was shown red against a deliberate break before being accepted.Not in this PR
D7 — re-verifying the gate green on
mainand recording the run id — is by definition post-merge. It is tracked and will be carried out against the post-merge benchmark run once this lands; this PR is not the completion of that deliverable.🤖 Generated with Claude Code
https://claude.ai/code/session_01MbbsDwgfoibRGPam1wrQxM
Summary by CodeRabbit
Benchmarking
Documentation
Tests