fix(loadtesting): back every capability's returns with a typed response schema

[sourabhd-cbu]

Context

External contract review of capability/loadtesting.capability-index.json (build 43eba70) found that every capability declared response fields no schema backs — 21 of 21, 75 fields. Root cause: all 21 resolve to the shared SuccessEnvelope, whose data is a typeless object with no properties. So returns carried the whole payload contract as a flat list of names — no types, no nesting.

Two knock-on effects the reviewer called out, both real:

• describeCapability returns an empty schema for all 21 capabilities.
• Drift is undetectable — scripts/check-contract.py walks returns → schema, and with the schema empty it can never catch a field the API stopped returning.

Fix (headline)

Per-capability response schemas composed with allOf, as tm did in its index — a shared field like runId is defined once and the capabilities carrying it can't drift apart:

"200": { "schema": { "allOf": [
  { "$schema": "SuccessEnvelope" },
  { "properties": { "data": { "$schema": "GetLoadTestRunReportData" } } }
] } }

• Added a typed <Cap>Data schema for all 21 capabilities; each returns field is now a backed property.
• scripts/check-contract.py capability/loadtesting.capability-index.json → 0 unbacked (was 21 / 75).
• Types asserted only where certain (ids, *Sec, booleans, arrays, date-time/uuid formats). Three genuinely ambiguous fields (metrics for trends, kpiDeltas, estimatedVuHoursRange) are backed with a description but no asserted type — deliberately not guessing types.

Also (derivable contract fixes from the same review)

• One-line description on all 9 entities — the field listProducts routes on (was 0/9).
• format: date-time on the 11 ISO timestamp params + ifVersion; format: uuid on the compare run-ids.
• minimum/maximum on all 6 limit params, from their declared caps.
• example on 5 high-leverage create/search/compare params.
• updateLoadTest guidance: ifVersion is opt-in optimistic concurrency; omitting it is last-write-wins.
• searchLoadTests guidance: hasMore has no cursor (top-N — narrow the query), so it isn't a dead-end page-2 signal.
• Reset the stale scripts/contract-baseline/loadtesting.json to its now-clean state.

Deliberately not changed

• "3 writes declare no required body fields" — false positive for us: stopLoadTestRun is path-only, startLoadTestRun is an all-optional re-run, updateLoadTest is a partial update. createLoadTest/estimate/compare already declare required fields.
• listActiveLoadTestRuns truncation signal — negligible; active runs are concurrency-capped.

Validation

• tsc --noEmit: clean · eslint: clean · 726/726 tests pass · check-contract.py (loadtesting): exit 0.
• Updated the two tests that asserted the old "loadtesting ships no entity descriptions" gap; they now assert descriptions are present and meet the shared ≤140-char / not-a-name-restatement quality bar.

Note (out of scope)

npm run check:contract runs tm && loadtesting; tm still fails (2 pre-existing empty-schema caps), so the chain short-circuits before loadtesting runs. Our index passes when checked directly — making the CI step gate loadtesting needs tm fixed too, or the && split so both run independently.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited), Workspace UI (inherited)

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: d3058ad6-eae7-462e-a167-576b19106abd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[sarve-shreyas]

Review

Traced how the index is actually consumed (bind.ts, index-loader.ts, register.ts, check-contract.py) and ran the suite plus the contract gate in a throwaway worktree: 737/737 tests pass, and check-contract.py on the loadtesting index reports 0 unbacked. The PR does what it says.

Six inline comments below. Two things that don't anchor to a changed line:

The fix lands one level below the root cause

The PR's stated problem is "data is a typeless object with no properties" — but 20 of the 79 new <Cap>Data properties are themselves typeless containers, including every list endpoint's row array (tests, runs, projects) plus kpis, transactions, slaVerdicts, errorsByCategory, meta, config, plan, vuHours, concurrency, report, metrics, aliases.

So both stated benefits are only partly delivered: describeCapability still says nothing about a row's fields, and check-contract.py still cannot detect drift inside them. searchLoadTests guidance promises "each row carries testId, projectId, name, testType and framework" and listProjectLoadTestRuns promises "each run carries a tags array" — no schema backs either, so the API can stop returning them silently. At minimum, giving the four row arrays an items schema would close most of it.

The reset baseline currently verifies nothing

scripts/contract-baseline/loadtesting.json is inert on this branch:

• this branch's check-contract.py has no --baseline handling at all — grep -c against_baseline → 0, the argument is silently ignored and the strict main() path runs
• npm run check:contract short-circuits on tm's failure before reaching the loadtesting invocation anyway

Both confirmed by running them. So nothing in CI verifies the 0-unbacked state this PR establishes, and a future edit that re-breaks it lands unnoticed. The PR body names the short-circuit but not the unimplemented flag. (--baseline has since been implemented on main, so this may resolve itself on rebase.)

One stale comment outside the diff

src/tools/capability-registry/types.ts:198 — the EntityDoc.description doc comment still reads "loadtesting has 0 of 9 today". This PR makes that 9 of 9 and updates the two tests that documented the same gap, but not the comment.

[sarve-shreyas]

This is enforced, not documentation — and it rejects values the guidance asks for.

format: "date-time" is run through FORMATS["date-time"] in bind.ts:139, whose regex requires a full T/space plus HH:MM:SS. The param is described only as an "ISO-8601 lower bound", and date-only is valid ISO-8601. listLoadTestRuns guidance goes further and says "To find a run by date, pass sinceIso/untilIso".

Concrete failure: an agent asked "which runs ran on Sept 12?" sends sinceIso=2026-09-12 (or 2026-09-12T10:30Z, no seconds) and invokeCapability refuses locally with 'sinceIso' must be a date-time — before any request goes out. That call previously reached the API.

bind.ts's own docstring names this as the failure mode to avoid: "a validator that rejects a correct value is worse than no validator."

Affects 11 params (also 349, 355, 454, 460, 805, 811, 1436, 1442, 1558, 1564), 8 of which carry no example to hint at the required shape. Either widen to accept date-only, or add an example to all 11 plus a guidance line.

[sourabhd-cbu]

Fixed in 3ccd5a6: dropped the enforced date-time format from all 11 sinceIso/untilIso/updatedSince params — the BE parses them with new Date(), so date-only and no-seconds values are valid there. Each now documents both shapes ("2026-09-12" or "2026-09-12T10:30:00Z") and has an example.

[sarve-shreyas]

Question rather than a defect — what does version actually look like?

ifVersion gets an enforced format: "date-time", but the version token a caller must round-trip is declared as a bare {"type": "string"} in CreateLoadTestData (2237), UpdateLoadTestData (2250) and GetLoadTestData (2266) — this PR deliberately declines to assert a format there.

If that token is ever anything other than strict RFC3339 (an epoch, a hash, v3), the optimistic-concurrency path becomes unreachable: the agent reads version from getLoadTest, passes it as ifVersion, and bind rejects it client-side.

Asserting on the consumer side of a value you don't constrain on the producer side is the risk. Either assert the format on version too, or drop it from ifVersion.

[sourabhd-cbu]

Asserted it on the producer side instead (3ccd5a6): version is always the BE's updated_at via toISOString() (util/agentVersionToken.js), so CreateLoadTestData / UpdateLoadTestData / GetLoadTestData now declare version as format date-time, matching ifVersion.

[sarve-shreyas]

These declare a non-null type while their own description says the value is null.

11 fields in CompareLoadTestRunsData: kpiDeltas.items.baseline/candidate/absChange/pctChange, transactionDeltas.items.* plus regressed, and slaVerdictChanges.items.from/to.

type: "number" excludes null, so the schema contradicts its own description and any consumer validating a real comparison response fails on the first null delta.

The repo already has the right encoding — nullable: true appears 676 times in tm.capability-index.json, and index-loader.ts:564 calls it out as exactly the absent-vs-explicitly-null distinction a caller cannot otherwise make.

[sourabhd-cbu]

Fixed in 3ccd5a6: the 11 CompareLoadTestRunsData fields that can be null are now nullable: true (tm's encoding).

[sarve-shreyas]

Typeless container — see the summary comment.

This is one of 20 <Cap>Data properties that declare no shape, which is the PR's own root cause relocated one level down. The row arrays (tests, runs, projects) are the highest-value ones to fix, since guidance already promises specific fields inside them that nothing backs.

[sourabhd-cbu]

Addressed the row arrays in 3ccd5a6: tests (list + search), projects and runs (per-test, per-project, active) now have item schemas taken from the BE shapers, so the fields the guidance promises are backed. The rest stay open by design for now — kpis / aliases are metric-keyed maps, and report / meta / config / plan are pass-through blocks; happy to type those in a follow-up.

[sarve-shreyas]

Worth confirming against the backend before shipping — this is a hard enum, not guidance.

values is enforced as a closed allowlist by bind.ts:305. Any axis the backend accepts but these four omit is now refused client-side, with no override.

Two things suggest the set is being narrowed by inference rather than from the API: commit 3d1833a removed url from this list after the fact, and the sibling getLoadTestRunReport.groupBy names a completely different set in prose (url/browser/location/transaction/step) while deliberately declaring no values at all.

If the backend's accepted set isn't known precisely, prose guidance is the safer encoding than an enum.

[sourabhd-cbu]

Confirmed against the backend: load-testing-backend#3135 accepts exactly transaction / label / threadGroup / scenario for compare (AGENT_TXN_GROUP_BY; anything else is a 400) — url was dropped there because url rows are keyed by request name. The report endpoint's groupBy is a different parameter with its own set (ALLOWED_GROUPBY), hence the prose there. Keeping the enum so the client rejects what the BE would.

[sarve-shreyas]

Inlining the schema drops the response description, and orphans the named Success.

Replacing {"$response": "Success"} with an inline schema loses "Standard success envelope; operation data under data" across all 21 capabilities — so describeCapability's 200 is now the only status with no description, while 400/401/404/500 keep theirs. That's the one place the envelope gets explained.

It also leaves the named Success response at line 2362 referenced by nothing — the dangling-definition class that check-contract.py's own docstring flags as something a live probe had to find.

tm's inline 200s use {description, schema}. Matching that keeps the prose; then either delete Success or keep referencing it.

[sourabhd-cbu]

Fixed in 3ccd5a6: all 21 inline 200s carry the success-envelope description again, and the unreferenced named Success response is removed.

[sourabhd-cbu]

Thanks for the thorough review — addressed in 3ccd5a6 (replies inline). On the two summary items: the EntityDoc "0 of 9" comment is updated. On the inert baseline: agreed that nothing in CI on this branch checks the 0-unbacked state — the --baseline handling exists on main, so it resolves once this branch is rebased/merged; I've kept the reset baseline file so it's correct when that lands.

[sarve-shreyas]

Approving. Re-verified against 3ccd5a6 rather than the head I originally reviewed.

Verified on this head: 737/737 tests pass, tsc clean, and check-contract.py on the loadtesting index reports 0 capabilities / 0 fields unbacked.

Fixed from the earlier review:

finding	status
Enforced date-time rejecting valid date-only inputs	fixed — enforced params 12 → 1
ifVersion asserting a format version never declared	fixed — format: date-time now asserted on version in all three schemas, with a description naming it as updated_at
Non-null types on fields documented as null	fixed — 11 nullable added to CompareLoadTestRunsData
200 losing its description, Success orphaned	fixed — 200 is {description, schema}, and Success is deleted rather than left dangling
Stale types.ts comment	fixed

Asserting the format on version as well as ifVersion was the stronger of the two available fixes — producer and consumer now agree, which also makes the one remaining enforced date-time correct rather than a leftover.

Typed row schemas: shapeless containers went 23 → 17, and the half that mattered landed — the list endpoints' row arrays (tests, runs, projects) now carry items, so the fields the guidance promises are finally backed. What remains is concentrated in GetLoadTestRunReportData (kpis, transactions, slaVerdicts, errorsByCategory, meta), GetLoadTestQuotaData and the two metrics trend objects. Worth a follow-up, not a blocker.

Two things deliberately not blocking:

• The groupBy value set is the Load Testing team's call to make, not the registry's. Withdrawn.
• scripts/contract-baseline/loadtesting.json is inert on this branch — --baseline has no handling in check-contract.py here, so nothing in CI yet verifies the 0-unbacked state this PR establishes. It is implemented on main, so a rebase picks it up.