diff --git a/spec/DSPX-4372-t1.md b/spec/DSPX-4372-t1.md new file mode 100644 index 00000000..c49ccd79 --- /dev/null +++ b/spec/DSPX-4372-t1.md @@ -0,0 +1,469 @@ +--- +ticket: DSPX-4372 +title: Six-stage delivery of multi-arm SDK benchmarks +status: draft +branches: + - opentdf/tests:DSPX-4372-s2-sdkmgr-fixes + - opentdf/tests:DSPX-4372-s2-manifest + - opentdf/tests:DSPX-4372-s2-local-prep + - opentdf/tests:DSPX-4372-s2-bench-env + - opentdf/tests:DSPX-4372-s3-arm-tuple + - opentdf/tests:DSPX-4372-s3-multi-arm + - opentdf/tests:DSPX-4372-s3-read-manifest + - opentdf/tests:DSPX-4372-s4-pairwise +prs: [621, 625, 626, 642, 658, 659, 660, 661, 667, 668, 669, 670] +created: 2026-09-28 +updated: 2026-10-21 +--- + +# DSPX-4372-t1 — Six-stage delivery of multi-arm SDK benchmarks + +## Objective + +Split [PR #621](https://github.com/opentdf/tests/pull/621) into independently +reviewable changes that extend the existing two-arm benchmark to two through +four named SDK builds. Developers must be able to prepare and run the same +experiment locally and in CI through shared Python implementations. + +This plan extends the [original benchmark specification](DSPX-4372.md). +Preserve its measurement isolation, paired rounds, comparability checks, +noise controls, and raw evidence. Deliver a simple result table first; report +presentation can follow after the measurement and conclusions are validated. + +## Starting point + +The review covered commit `7bd7506988c7ba912393c90959838c429268f598`. +[PR #620](https://github.com/opentdf/tests/pull/620), the branch-name flattening +prerequisite, merged on 2026-09-28. Rebase onto current main before extracting +the stages, retaining fixes already present there. + +At review time, all 192 benchmark unit tests passed. The PR benchmark job was +skipped, so those checks did not establish that the new workflow path worked. +Three additional simulated cases exposed gaps: + +| Finding | Observed behavior | Required correction | +| --- | --- | --- | +| Control-only run with censored RSS | `nothing_measured=False` and a PASS headline without any candidate measurement | Register every control comparison before applying censoring; preserve this invariant in the multi-arm conversion | +| Four arms with an uncertain candidate | A winner is declared after checking only the first two candidates in point-estimate order | Require supporting comparisons against every other candidate, or report pairwise conclusions without an overall winner | +| Biased A/A control with a narrow interval | `trustworthy=False` still permits a named winner | A failed applicable control must suppress winner claims | + +The first issue reverses existing classification behavior. Restore the +control-only tests removed by the PR and extend them with the censored case. +Carry these review cases into the stage acceptance tests below. + +## Ownership and contracts + +| Component | Owns | +| --- | --- | +| `xtest/perf` | Benchmark options, reference and arm ordering, budget defaults, measurement, statistical analysis, structured outcomes, and reports | +| `otdf-sdk-mgr` | Artifact resolution, installation, build settings, and a manifest recording exactly what was installed | +| `otdf-local` | Service configuration, startup, provisioning, readiness, environment export, logs, and shutdown | +| GitHub workflow | Dispatch inputs, runner matrix, toolchain setup, caches, calls to the local commands, artifact upload, and cleanup | + +Extract benchmark option validation from pytest fixtures into a pure module +under `xtest/perf`. Both the local preparation entry point and pytest must use +it. `otdf-sdk-mgr` should accept generic ordered artifact requests; it should +not own benchmark thresholds or statistical policy. + +Use a versioned resolved installation manifest to connect the components. +For each SDK request, preserve its requested alias, repository, immutable +commit, installed tag/path, installation method, and relevant build settings. +Record the platform and provisioning CLI pins separately from measured arms. +Preserve request order and the mapping from multiple aliases to one artifact. +Installed paths may differ between machines; artifact identities must not. + +The benchmark preparation result should combine that installation information +with validated benchmark settings and an explicit preparation status. The +renderer displays statuses; it must not decide whether pytest succeeds. +Do not use `report.same_commit()` as an exit-policy dependency. + +The existing scenario schema and `install scenario` manifest are useful +starting points. They are not already a complete implementation: the SDK +scenario install loop currently calls `install_release`, and arbitrary source +refs need explicit support. Extend or reuse that machinery without making a +general scenario-system rewrite a prerequisite for this work. Final CLI +spellings are to be selected in stage 2; the interfaces below describe +required behavior, not commands that already exist. + +## Delivery sequence + +| Stage | Deliverable | Dependencies | +| --- | --- | --- | +| 1 | Independent two-arm statistical fixes and regression coverage | Current main | +| 2 | Shared local preparation and installation contract | Stage 1 baseline | +| 3 | Multi-arm measurement and reference gates with simple output | Stage 1; 3c reads the stage 2 manifest contract | +| 4 | Candidate-to-candidate conclusions | Stage 3 | +| 5 | Thin CI integration using the local path | Stages 2–4 | +| 6 | Report presentation and publishing improvements | Stages 3–4; stage 5 for CI artifact links (6c only) | + +Stages are milestones, not pull requests. Each stage lands as a short stack of +pull requests that can each be reviewed on its own; see the size budget under +"Validation and merge discipline". A stage is done when its last pull request +merges and its acceptance list holds. + +Stages 2 and 3 run as parallel tracks from main. Stage 2 changes only +`otdf-sdk-mgr` and `otdf-local`, and stage 3 changes only `xtest`. The packages +share no imports: the installation manifest is a written JSON convention, so +stage 3 can be reviewed and merged while stage 2 is still in review. The tracks +meet in stage 5. Stage 6 can be developed independently once the result schema +is stable and must not block the core release. Keep the existing nightly +two-arm path operational between merges. + +### Pull requests + +| Part | Branch | Stacked on | Scope | PR | +| --- | --- | --- | --- | --- | +| 1 | `DSPX-4372-s1-stat-fixes` | main | Stage 1 | #625 (merged) | +| 2a | `DSPX-4372-s2-sdkmgr-fixes` | main | Refresh branches in the bare repos; never leave a half-installed js release | #667 | +| 2b | `DSPX-4372-s2-manifest` | 2a | Manifest contract: artifact identities, preparation statuses, collision and neutral checks, `dist_slug` | #668 | +| 2c | `DSPX-4372-s2-local-prep` | 2b | `otdf-sdk-mgr install benchmark` | #626 | +| 2d | `DSPX-4372-s2-bench-env` | 2c | `otdf-local env` exports the manifest and the otdfctl pin | #642 | +| 3a | `DSPX-4372-s3-arm-tuple` | main | Runner, statistics and report over a tuple of arms; JSON schema 2; still two arms | #669 | +| 3b | `DSPX-4372-s3-multi-arm` | 3a | `--bench-refs`, two to four arms, `K / 2` budget, K-copy control | #658 | +| 3c | `DSPX-4372-s3-read-manifest` | 3b | pytest reads the installation manifest: arms, neutral status, provenance | #670 | +| 4 | `DSPX-4372-s4-pairwise` | 3c | Stage 4 | #659 | +| 5 | `DSPX-4372-s5-base` | 2d, 4 | Merge point for stage 5; not a pull request | — | +| 5a | `DSPX-4372-s5-ci-validate` | 5 | `perf/ci.py` validates dispatch input before jobs start; job path unchanged | — | +| 5b | `DSPX-4372-s5-ci` | 5a | The bench job runs the local preparation path | #660 | +| 6a | `DSPX-4372-s6-report` | 4 | Statuses, provenance and decision-first tables | #661 | +| 6b | `DSPX-4372-s6-visuals` | 6a | Effect strip and Braille round traces | — | +| 6c | `DSPX-4372-s6-publish` | 6b, 5b | Artifact links in the CI step summary | — | + +#660 and #661 predate this split: #660 still holds 5a and 5b, #661 all of +stage 6, and both stack on the pre-split stages. They move onto the new stacks, +and split, once `DSPX-4372-s5-base` is rebuilt. + +Work that this plan needed, but which landed as separate fixes: #640 (shared +option validation, stage 2 item 1), #638 and #641 (otdf-local startup +diagnostics, KAS root key and preview flags), and #647 (otdf-local golden +keys). #622 added payload sizes up to 1 GiB, a per-dispatch budget and scratch +pruning; stage 5 consumes its workflow inputs. + +## Stage 1 — Independent two-arm statistical fixes + +### Scope + +Extract fixes that are useful without additional arms: + +- Carry explicit faster-tail p-values through analysis and artifacts, with + their appropriate multiplicity adjustment. Do not infer improvement by + reversing an adjusted slower-tail probability. +- Preserve current-main deadline handling, complete-round recording, and + configured minimum-round enforcement. Include changes here only where the + rebased branch still requires them. +- Establish control-only regression coverage, including censored RSS. + Classify controls before metric-specific censoring. +- Keep the current two-arm CLI and report shape, apart from additive evidence + needed for the statistical fix. + +Primary files: `xtest/perf/stats.py`, `xtest/perf/runner.py`, their tests, and +minimal serializer changes. Exclude multi-arm selection, new workflow inputs, +winner logic, and visual report changes. + +### Acceptance + +- [ ] A planted meaningful slowdown gates; a trivial change does not. +- [ ] A planted improvement uses the faster-tail test and never fails the job. +- [ ] Controls and censored metrics do not enter candidate correction families. +- [ ] A control-only run counts as nothing measured, including when RSS is at + the measurement floor; the session fails unless explicitly run without gating. +- [ ] Deadline expiry never commits a partial round or grants a verdict below + the configured minimum. +- [ ] Existing two-arm invocations and functional tests retain their behavior. + +## Stage 2 — Shared local preparation + +### Scope + +Provide a locally executable preparation path for the existing two-arm case. +Keep CI on its existing path until stage 5. + +1. Create the shared benchmark configuration parser and validator. Normalize + the existing pytest baseline/candidate aliases into one ordered request. + Validate SDK names, conflicting options, duplicate requests, finite positive + budgets, thresholds, and `max_rounds >= min_rounds` before setup begins. +2. Extend `otdf-sdk-mgr` to resolve ordered artifact requests, install the + resolved identities, and emit the installation manifest. Reuse existing + release and source-build implementations. Resolve mutable refs once per + plan and install those commits without resolving the names again. +3. Reject conflicting installed tags/paths before one build can overwrite + another. Preserve aliases when multiple requests identify one commit. +4. Represent two distinct aliases for the same commit as an explicit neutral + preparation outcome, produced before builds or service startup. Unknown + refs and failed resolutions remain errors. This must work identically + locally and in CI. +5. Use `otdf-local` to prepare only the benchmark's required backend services. + Start from `up --services docker,platform` and add any missing configuration + parity through its settings and CLI. Include required features, provisioning, + readiness, environment export, log paths, and cleanup. +6. Pin the provisioning `otdfctl` independently of the benchmark arms. A + release-only Go comparison must not fall back implicitly to another CLI. +7. Feed installed provenance and preparation status to pytest directly. Local + users must not need to construct `BENCH_VERSION_INFO` or + `BENCH_REQUESTED_REFS` themselves. + +Limit the service work to what the benchmark needs. Do not migrate the entire +functional matrix or expand the scenario runtime as part of this stage. + +### Pull requests + +- Item 1 landed as #640; item 5 as #638, #641, #647 and 2d. +- **2a** holds the `otdf-sdk-mgr` fixes the benchmark needs but every install + benefits from: refreshing branches in the bare repositories and removing a + failed js release. +- **2b** is the contract: the manifest's data model and its validation, plus + `dist_slug`, which the manifest and `install tip` must share. +- **2c** resolves, installs and records, covering items 2–4 and 6. +- **2d** exports the manifest and the provisioning pin through `otdf-local env`. +- Item 7's xtest side lands in **3c**, because it reads the ordered arm specs + that 3b introduces. The stage 2 acceptance item on provenance completes there. + +### Acceptance + +- [ ] A documented local sequence resolves, installs, starts services, exports + the environment, runs a two-arm comparison, and shuts down the environment. +- [ ] Local preparation requires no copied shell from workflow files and no + GitHub-specific environment variables. +- [ ] Branches containing slashes, releases, SHAs, and supported PR refs retain + their requested identity and map to the expected installed build. +- [ ] Invalid options and ref collisions fail before installation or startup. +- [ ] Same-commit preparation is neutral; missing or failed builds are errors. +- [ ] A plan can be reused without a moving branch silently changing its SHA. +- [ ] Release-only Go arms use the explicitly selected provisioning CLI. +- [ ] Unit/CLI tests cover validation, manifest round trips, resolution failures, + and installation identity; a live local smoke run verifies service parity. + +## Stage 3 — Multi-arm measurement and reference gates + +### Scope + +Generalize the local measurement path to two through four named builds of one +SDK. The first arm is the reference, and every candidate is compared with it. + +- Introduce explicit arm identities, labels, reference identity, and per-arm + sample vectors in `runner.py` and the benchmark fixtures. +- Preserve randomized within-round execution. Record a round only after all + arms complete successfully. +- Run K copies of the reference for the A/A control and assess the worst + applicable control contrast. Preserve stage 1's control classification. +- Apply comparability checks across every arm and share reference-produced + ciphertexts for decrypt measurements. +- Scale the default budget by `K / 2` in the shared configuration module. + Honor explicit budgets unchanged. Treat the four-arm ceiling as a deliberate + supported benchmark limit, independent of the Action's installation slots. +- Support `--bench-refs`; preserve existing pytest baseline/candidate aliases + as adapters to the same validated configuration. +- Reject a partially duplicated multi-arm request before installation. If all + distinct requested aliases resolve to one commit, use the explicit neutral + preparation status from stage 2. +- Version the JSON schema for per-arm samples and reference contrasts. Provide + a simple table containing arm identities, metrics, intervals, and verdicts. + +Exclude candidate ranking, new report visualizations, and CI orchestration. +Retain convenience baseline/candidate labels where useful, but document that +changed sample and contrast structures require schema-aware consumers. + +### Pull requests + +- **3a** converts the runner, statistics and report from a baseline/candidate + pair to a tuple of arms with a reference, and bumps the JSON schema. Option + handling and fixtures still produce two arms, so behavior is unchanged apart + from the artifact shape. +- **3b** adds `--bench-refs`, raises the arm ceiling to four, scales the default + budget, runs the control as K copies of the reference, and checks + comparability across every arm. +- **3c** has pytest read the installation manifest. A prepared run measures the + installed arms in request order, reports a neutral preparation as such, and + carries the manifest's provenance into the JSON artifact. Fixtures that need a + platform are requested only once a cell is known to be measurable. + +### Acceptance + +- [ ] Two-, three-, and four-arm tests exercise each arm once per completed + round and preserve seeded execution order. +- [ ] A regression in any candidate's gated metric is detected against the + configured reference; CPU remains informational. +- [ ] All arms read the same ciphertext and use compatible operation settings. +- [ ] Censored control-only runs still count as nothing measured. +- [ ] Budget expiry, warm-up failures, partial rounds, and cleanup are tested + with more than two arms. +- [ ] JSON identifies every arm and preserves every completed sample with an + explicit schema version and reference. +- [ ] Existing two-arm commands continue to work. +- [ ] A live local multi-arm smoke run produces actual comparison cells and + usable artifacts; an all-skipped run does not satisfy acceptance. +- [ ] `xtest/perf/README.md` documents the new local invocation and contracts. + +## Stage 4 — Candidate-to-candidate conclusions + +### Scope + +Add every non-reference pairwise comparison to the analysis and simple report. +Keep candidate comparisons informational: they cannot fail the regression gate. + +- Define FASTER, SLOWER, TIED, and INCONCLUSIVE for the practical effect band. + Review the directional tests and multiplicity treatment explicitly. +- Keep gated reference comparisons, candidate comparisons, and other ungated + metrics in their intended separate correction families. +- Report individual pairwise conclusions first. Point-estimate order is not + evidence that every adjacent candidate is distinguishable. +- If an overall winner is emitted, require trustworthy supporting comparisons + against every other candidate. A tie, an unusable comparison, or an + unresolved competitor prevents a unique winner claim. +- Suppress winner claims when the applicable A/A control trips, even if its + interval is narrow. Preserve raw measurements and explain the limitation. +- Keep winner eligibility in the analysis/result model so JSON, terminal, and + Markdown cannot disagree about the conclusion. + +### Acceptance + +- [ ] Reversing comparison direction preserves the underlying conclusion. +- [ ] Adding informational comparisons does not alter the reference gate's + correction family. +- [ ] A clear two-candidate difference is reported without failing the job + solely because one candidate loses the comparison. +- [ ] A tied pair has no unique winner. +- [ ] With three candidates, a resolved first pair and an inconclusive third + competitor do not produce an overall winner. +- [ ] A tripped control suppresses winners even when `underpowered=False`. +- [ ] No control, censored data, and insufficient precision produce honest + unresolved conclusions. +- [ ] The README describes the pairwise verdicts and winner requirements. + +## Stage 5 — Thin CI integration + +### Scope + +Replace benchmark-specific workflow implementation with calls to the local +preparation, installation, service, and measurement paths delivered above. + +- Retain dispatch inputs, focus-SDK matrix selection, toolchain setup, caches, + upload actions, and cleanup in YAML. +- Run shared validation before creating expensive benchmark jobs. Emit the + matrix from validated input through a small adapter; do not reimplement ref + parsing, arm limits, defaults, or same-commit handling in shell. +- Resolve benchmark arms during preparation before platform startup. Consume + the same installed manifest and configuration that local pytest consumes. +- Replace the benchmark job's use of the fixed-slot setup action with the + manager's installation path. Other jobs can retain the action until a + separate migration. +- Remove the newly proposed deprecated workflow inputs `bench-baseline-ref` + and `bench-candidate-ref`; they did not exist before PR #621. Expose one + `bench-refs` form and retain the already-supported pytest aliases. +- Export service environment through `otdf-local`; ensure logs and completed + results remain available when measurement fails. Run cleanup on success and + failure. +- Keep measurements serial, with one SDK per runner and no ordinary PR perf + gate. Preserve nightly two-arm defaults. +- Choose and document a job timeout that includes installation, service setup, + measurement, and artifact upload. Validate any advertised budget ceiling + against that timeout; do not imply arbitrary budgets fit a fixed backstop. + +Known gap: when an SDK's newest release is the same commit as `main`, the +default nightly pair is a neutral preparation and measures nothing. The run +exits successfully and reports NEUTRAL, which is accurate but means the nightly +tests nothing until the next commit. DSPX-5260 tracks choosing nightly roles so +that the default pair is never one commit. Until then, hand-dispatched runs +should pin a distinct baseline release. + +### Pull requests + +- The merge point **5** also documents the prepared run in the local sequence + of `xtest/perf/README.md`: the arms come from the manifest, up to four refs + can be prepared at once, and pytest reports a neutral preparation. That + section arrives with 2d and the behavior with 3c, so neither track can + document it alone. +- **5a** adds `perf/ci.py` and runs it in `resolve-versions`, so invalid + dispatch input fails before any runner starts. The bench job keeps its + current installation path. +- **5b** switches the bench job to `otdf-sdk-mgr install benchmark`, + `otdf-local up` and `otdf-local env`. It is the risky change, so it stays + small and carries the dispatch smoke-run evidence. + +### Acceptance + +- [ ] The workflow contains no duplicate benchmark parsing or statistical policy. +- [ ] The same prepared experiment selects the same artifacts and settings + locally and in CI. Timings need not match across machines. +- [ ] Invalid requests fail before expensive setup; same-commit requests report + the same neutral outcome as local preparation. +- [ ] Dispatch smoke runs cover two, three, and four arms, with comparison + cells confirmed to execute and artifacts available. +- [ ] The default nightly path is verified for Go, Java, and JS. +- [ ] Failed installation/startup and a benchmark failure exercise cleanup and + retain the available diagnostic evidence. +- [ ] Existing functional jobs retain their behavior. + +## Stage 6 — Report presentation and publishing + +### Scope + +Extract the large presentation expansion from PR #621 into this final stage: +provenance links, effect charts, Braille traces, round diagnostics, improved +summaries, budget guidance, and links to uploaded artifacts. + +Keep serialization, conclusions, and rendering distinct. The renderer consumes +the structured outcome from earlier stages and never changes gate status, +winner eligibility, or statistical families. Provenance comes from the installed +manifest in both environments. + +Keep the simple table and JSON usable independently of visual enhancements. +Generate the Markdown artifact locally as well as in CI. If artifact links +require publication after upload, implement that rendering/substitution in a +locally testable function; YAML supplies only the returned artifact URL. + +### Pull requests + +- **6a** renders the statuses, the compared builds with their requested refs + and installed commits, the attention table, candidate comparisons, skipped + cells and run facts. It stacks on stage 4, not stage 5. +- **6b** adds the effect strip and Braille round traces. +- **6c** fills uploaded-artifact links into the step summary. It is the only + part of stage 6 that needs stage 5. + +### Acceptance + +- [ ] Local and CI reports identify requested refs and installed commits. +- [ ] PASS, regression, no measurement, same commit, untrustworthy control, + partial coverage, ties, and unresolved competitors have distinct wording. +- [ ] A partial run lists skipped cells and does not imply complete coverage. +- [ ] JSON, terminal, and Markdown agree on outcomes and winner eligibility. +- [ ] Large effects, long arm names, Markdown-sensitive characters, missing + provenance, and unavailable intervals render legibly. +- [ ] The report works without GitHub environment variables or an artifact URL; + CI publication never leaves an unresolved URL placeholder in the summary. +- [ ] Existing analysis fixtures produce identical gate decisions before and + after presentation changes. + +## Validation and merge discipline + +For every pull request, include its tests and documentation in the same PR. +Run lint, format, and type checks from each touched Python package using +`uv run`, plus the relevant package tests. Follow the package `AGENTS.md` +requirements. + +Keep each pull request to one concern and about 400 changed lines of non-test +code. Tests may run longer, but a test file that dwarfs the change it covers is +a sign the PR holds more than one concern. A PR over budget says why in its +description. Land refactors that preserve behavior before the behavior changes +they enable, as 3a does for 3b, so each diff shows one kind of change. + +Use offline simulated measurements for statistical and deadline invariants. +Use CLI tests with controlled resolution/installation dependencies for the +preparation contract. Use live smoke runs to establish that installed builds, +service setup, and measurement work together. Record completed rounds, +comparison counts, and skip reasons when assessing smoke-run success. + +Each PR description should state its independent user-visible result, +dependencies, compatibility effects, and validation evidence. Keep extraction +work based on current main so already-fixed two-arm behavior is not lost. +Do not carry report embellishments or unrelated workflow refactoring into the +core stages merely because they were present in the original branch. + +## Out of scope + +Historical performance storage, cross-SDK speed rankings, further +payload-size experiments, a general workflow migration, a general +scenario-runtime rewrite, smarter scratch-disk scheduling (DSPX-5253), and +performance gating on ordinary pull requests remain separate work. Payload sizes +up to 1 GiB landed separately in #622.