From 41bb7c6807f71ddbcfd3073d57332a24773b704e Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 28 Sep 2026 13:52:32 -0400 Subject: [PATCH] docs(spec): six-stage delivery plan for multi-arm SDK benchmarks Records the plan that replaced #621 (DSPX-4372-t1): independently reviewable stages for statistical fixes, shared local preparation, multi-arm measurement, pairwise conclusions, thin CI integration, and report presentation, with the review findings each stage must carry. Signed-off-by: Dave Mihalcik --- spec/DSPX-4372-t1.md | 356 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 356 insertions(+) create mode 100644 spec/DSPX-4372-t1.md diff --git a/spec/DSPX-4372-t1.md b/spec/DSPX-4372-t1.md new file mode 100644 index 00000000..0a509353 --- /dev/null +++ b/spec/DSPX-4372-t1.md @@ -0,0 +1,356 @@ +--- +ticket: DSPX-4372 +title: Six-stage delivery of multi-arm SDK benchmarks +status: draft +branches: [opentdf/tests:DSPX-4372-02-karm-core] +prs: [621] +created: 2026-09-28 +updated: 2026-09-28 +--- + +# 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 | Stages 1–2 | +| 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 | + +Prepare one focused PR per stage. 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. + +## 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. + +### 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. + +### 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. + +### 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. + +### 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 stage, 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. + +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, new payload-size +experiments, a general workflow migration, a general scenario-runtime rewrite, +and performance gating on ordinary pull requests remain separate work.