[ss-o]

Closes #629

Proposes ADR-0024 (status PROPOSED, deciders TBD): benchmark results are observed, not gated, extending ADR-0009 to performance; a comparison flags a case for review at a median regression over 10% or a p95 regression over 15%, never fails on timing; baseline and candidate are measured in one runner with balanced order, warmups, median, p95, minimum, sample count, and an A/A control, and a functional failure invalidates the case; workloads are network-free; per-release results are committed by a maintainer under benchmarks/results/<tag>/, never by automation; the shared report shape is admitted to PATTERNS.md once z-shell/zi#556 lands.

Context: zpmod and z-a-meta-plugins already ship benchmarks with different shapes, and zi is adding a third after a promotion review found a +20% ice-tokenizing regression only through an ad hoc A/B (z-shell/zi#554). The thresholds come from that A/B, where unrelated cases sat within one percent.

Draft only; acceptance and the deciders field are yours.

Instruction impact review (runbooks/instruction-update.md)

1. Shared policy: an organization decision record, delivered as a required decision surface like ADR-0023, not runtime-only behaviour or enforcement.
2. Every runtime the manifest lists for decision surfaces (codex, claude-code, copilot, gemini-cli, human), in every repository context, because it governs how any repository's benchmark relates to merging; the consumer list mirrors decision-0023.
3. Canonical owner is z-shell/.github (decisions/); repository benchmark READMEs (zpmod, z-a-meta-plugins, zi#556) describe their local implementation and link here rather than restating the policy.
4. No duplication: ADR-0009 covers coverage and is referenced, not repeated; the zi README states the thresholds as the ADR's values. z-a-meta-plugins' README says "no timing thresholds", which the ADR reconciles by requiring flag thresholds without gating; that README needs a one-line update after acceptance, tracked in the ADR's decision 6.
5. Manifest: one added route, decision-0024 in .github/instruction-surfaces.json (same shape as decision-0023); the private control workspace's manifest and composite are unaffected because they consume this repository's decisions through the existing public-baseline route, and no Plugin Standard text changes.
6. Yes: the surface is a plain Markdown decision file discovered by the manifest; no hook or skill is needed for any runtime to receive it.
7. Yes: scripts/validate-agent-policy.py and scripts/decision-records.py --check pass on this branch; the decision index is regenerated by the validator, and no size limit applies to decision surfaces.

[Copilot]

🟡 Changes recommended

ADR-0024 lacks its required manifest entry, and the proposed shared schema conflicts with existing benchmark output.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Proposes ADR-0024 to standardize non-gating benchmark reporting and publication.

Changes:

• Defines review thresholds, comparison methodology, and publication rules.
• Adds ADR-0024 to the generated decision index.

File summaries

File	Description
decisions/README.md	Indexes ADR-0024.
decisions/0024-benchmarks-observed-not-gated.md	Documents the proposed benchmark policy.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The canonical schema omits required control data, and several policy claims conflict with their referenced sources.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

Previously missed (1) — in code that hasn't changed since the last review.

decisions/0024-benchmarks-observed-not-gated.md:11

• The z-a-meta-plugins workflow is path-filtered, so it does not run on every pull request. Describe it as running on pull requests that affect benchmarked code to keep the ADR's evidence accurate.

This issue also appears in the following locations of the same file:

• line 21
• line 22
• line 34
• line 40
• line 47

decisions/0024-benchmarks-observed-not-gated.md:21

• The final sentence includes source_revision among the identity fields that must match. Useful baseline/candidate and release-to-release comparisons necessarily use different revisions, so this wording makes every real comparison invalid. Exclude the revision from equality and match only environment and workload identity.

5. **Publication.** Per-release results are committed by a maintainer under `benchmarks/results/<tag>-<os>-<arch>/`, the naming zpmod already uses, after a release, together with the exact source revision, Zsh version, architecture, and runner or host identity. Automation uploads artifacts and job summaries; it never commits results. Results are compared only when those identity fields match.

decisions/0024-benchmarks-observed-not-gated.md:34

• runbooks/org-review.md defines the “Needs review” section but does not currently instruct reviewers to collect benchmark flags. This sentence presents a mitigation as already operational even though the non-gated policy still has no durable review path. Either update that runbook in this PR or state this as an acceptance prerequisite.

- A flag that nobody reads is a regression that ships. The weekly organization review ([`runbooks/org-review.md`](../runbooks/org-review.md)) lists flagged benchmark runs under "Needs review".

decisions/0024-benchmarks-observed-not-gated.md:40

• ADR-0013 does not discourage bypass paths; its costs explicitly preserve an escape route for deadlocked required checks and omit enforce_admins (decisions/0013-repository-settings-baseline.md:224-238). Remove the reversed attribution and base this alternative on the actual concern: noisy timing would make a required check unreliable.

1. **Blocking above a threshold.** Rejected for now: without an A/A history, hosted-runner noise produces false reds and forces a bypass path, which ADR-0013 discourages. Revisit once the control column has a track record.

decisions/0024-benchmarks-observed-not-gated.md:47

• Issue #555 documents a high baseline cost to optimize, not a regression. Calling both #554 and #555 regressions misstates the linked evidence; describe them as performance issues instead.

- [z-shell/zi#553](https://github.com/z-shell/zi/issues/553), [z-shell/zi#554](https://github.com/z-shell/zi/issues/554), [z-shell/zi#555](https://github.com/z-shell/zi/issues/555): the zi suite and the two regressions it exists to track.

decisions/0024-benchmarks-observed-not-gated.md:22

• Version 1 omits any control field even though decision 3 requires an A/A control and the consequences discuss control-column flags. Consumers cannot tell where control results or their flags belong; the linked Zi implementation currently emits a separate top-level control. Add an explicit control shape and flag representation to version 1, then align consumers before claiming schema compatibility.

6. **Canonical comparison schema, version 1.** A comparison report carries `schema_version: 1`, `baseline` and `candidate` identity (`label`, `source_revision`, `environment{zsh_version,architecture,cpu,runner_image}`, `workload{warmups,samples}`), a boolean `comparable` derived from those fields, and `cases[case]` where each case is either `{failure: {baseline, candidate}}` or `{results: {baseline, candidate}, change, flag}` with `results[variant] = {median, p95, min, count, samples}` and `change = {median_delta_ms, median_delta_percent, p95_delta_ms, p95_delta_percent}`; `flagged` and `failed` list the affected case names. `z-shell/zi` implements this schema. `z-a-meta-plugins` today emits `results[variant][metric]{median,p95,samples}` and `change[metric]{median_delta_ms,median_delta_percent}` per metric; it adapts to version 1 (adding `min`, `count`, and the p95 deltas, and lifting its per-metric nesting to one case per metric or an explicit `metrics` map in a schema revision) before the shape is admitted to `PATTERNS.md` as an observed two-repository pattern. Until then the pattern is proposed, not observed.

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The ADR contains conflicting comparison criteria, an incomplete control schema, and inaccurate references.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The ADR is prematurely accepted, and its schema and operational review contract remain incomplete.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

decisions/0024-benchmarks-observed-not-gated.md:22

• Severity: IMPORTANT. The comparability rule is self-contradictory: source_revision and label are declared identity fields, but baseline and candidate normally differ in exactly those fields. If comparable is derived from all listed fields—or publication comparisons require all identity fields to match—ordinary A/B reports become incomparable. Specify the exact equality subset (for example environment and workload), while retaining labels and revisions only as provenance.

This issue also appears in the following locations of the same file:

• line 22
• line 22
• line 22
  decisions/0024-benchmarks-observed-not-gated.md:34

• Severity: IMPORTANT. The linked runbook does not currently collect benchmark flags under “Needs review”; its inputs mention recent workflow failures, while timing flags intentionally do not fail jobs (runbooks/org-review.md:18-40). Without adding an explicit discovery step, this claimed review path will not run. Update the runbook in this PR or describe this as unresolved follow-up work.

decisions/0024-benchmarks-observed-not-gated.md:22

• Severity: IMPORTANT. Version 1 does not define where the required A/A control from line 19 is represented: the listed identities and per-case results contain only baseline and candidate. A conforming producer could therefore omit the control entirely. Add the control's identity, per-case results/deltas, comparability, and failure semantics to the canonical schema.

6. **Canonical comparison schema, version 1.** A comparison report carries `schema_version: 1`, `baseline` and `candidate` identity (`label`, `source_revision`, `environment{zsh_version,architecture,cpu,runner_image}`, `workload{warmups,samples}`), a boolean `comparable` derived from those fields, and `cases[case]` where each case is either `{failure: {baseline, candidate}}` or `{results: {baseline, candidate}, change, flag}` with `results[variant] = {median, p95, min, count, samples}` and `change = {median_delta_ms, median_delta_percent, p95_delta_ms, p95_delta_percent}`; `control[case]`, present whenever the A/A control from decision 3 was measured, carries the same row shape with the control measurement in the `candidate` position, so the noise floor is exchangeable and validatable alongside the comparison; `flagged` and `failed` list the affected case names, `failed` covering control rows as well. `z-shell/zi` implements this schema. `z-a-meta-plugins` today emits `results[variant][metric]{median,p95,samples}` and `change[metric]{median_delta_ms,median_delta_percent}` per metric; it adapts to version 1 (adding `min`, `count`, and the p95 deltas, and lifting its per-metric nesting to one case per metric or an explicit `metrics` map in a schema revision) before the shape is admitted to `PATTERNS.md` as an observed two-repository pattern. Until then the pattern is proposed, not observed.

decisions/0024-benchmarks-observed-not-gated.md:22

• Severity: IMPORTANT. z-shell/zi does not yet implement this schema: the linked implementation is open PR z-shell/zi#556 and is currently unmerged, matching this PR body's statement that the shared shape waits for #556 to land. Refer to the open PR as proposed implementation, or land it before making this present-tense claim authoritative.

6. **Canonical comparison schema, version 1.** A comparison report carries `schema_version: 1`, `baseline` and `candidate` identity (`label`, `source_revision`, `environment{zsh_version,architecture,cpu,runner_image}`, `workload{warmups,samples}`), a boolean `comparable` derived from those fields, and `cases[case]` where each case is either `{failure: {baseline, candidate}}` or `{results: {baseline, candidate}, change, flag}` with `results[variant] = {median, p95, min, count, samples}` and `change = {median_delta_ms, median_delta_percent, p95_delta_ms, p95_delta_percent}`; `control[case]`, present whenever the A/A control from decision 3 was measured, carries the same row shape with the control measurement in the `candidate` position, so the noise floor is exchangeable and validatable alongside the comparison; `flagged` and `failed` list the affected case names, `failed` covering control rows as well. `z-shell/zi` implements this schema. `z-a-meta-plugins` today emits `results[variant][metric]{median,p95,samples}` and `change[metric]{median_delta_ms,median_delta_percent}` per metric; it adapts to version 1 (adding `min`, `count`, and the p95 deltas, and lifting its per-metric nesting to one case per metric or an explicit `metrics` map in a schema revision) before the shape is admitted to `PATTERNS.md` as an observed two-repository pattern. Until then the pattern is proposed, not observed.

decisions/0024-benchmarks-observed-not-gated.md:22

• Severity: IMPORTANT. The canonical report omits the effective threshold values even though decision 2 permits repositories to tighten them. A stored flag: false is therefore not reproducible or comparable across repositories because consumers cannot tell whether 10/15 or stricter values were applied. Include the median and p95 thresholds, plus the non-gating policy, in version 1.

6. **Canonical comparison schema, version 1.** A comparison report carries `schema_version: 1`, `baseline` and `candidate` identity (`label`, `source_revision`, `environment{zsh_version,architecture,cpu,runner_image}`, `workload{warmups,samples}`), a boolean `comparable` derived from those fields, and `cases[case]` where each case is either `{failure: {baseline, candidate}}` or `{results: {baseline, candidate}, change, flag}` with `results[variant] = {median, p95, min, count, samples}` and `change = {median_delta_ms, median_delta_percent, p95_delta_ms, p95_delta_percent}`; `control[case]`, present whenever the A/A control from decision 3 was measured, carries the same row shape with the control measurement in the `candidate` position, so the noise floor is exchangeable and validatable alongside the comparison; `flagged` and `failed` list the affected case names, `failed` covering control rows as well. `z-shell/zi` implements this schema. `z-a-meta-plugins` today emits `results[variant][metric]{median,p95,samples}` and `change[metric]{median_delta_ms,median_delta_percent}` per metric; it adapts to version 1 (adding `min`, `count`, and the p95 deltas, and lifting its per-metric nesting to one case per metric or an explicit `metrics` map in a schema revision) before the shape is admitted to `PATTERNS.md` as an observed two-repository pattern. Until then the pattern is proposed, not observed.

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Policy routing, operational review coverage, and schema consistency remain incomplete.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

.github/instruction-surfaces.json:930

• The impact review says this mandatory benchmark policy must reach every runtime/repository context that evaluates benchmarks, but this route is selected only for architecture-decision. Because AGENTS.md:14 requires both task and path dimensions to match, ordinary benchmark testing will not load ADR-0024; the existing testing surface is routed under testing (.github/instruction-surfaces.json:257-266). Add that task so the policy is actually delivered where it governs merge decisions.
  decisions/0024-benchmarks-observed-not-gated.md:22
• The canonical v1 schema omits the effective threshold values even though repositories may tighten them. Consequently, a standalone report cannot explain or reproduce its flag value, and two schema-v1 reports may apply different thresholds without exposing that difference. Make median and p95 thresholds required report metadata (the zi draft already emits them) and include them in the schema definition.
  decisions/0024-benchmarks-observed-not-gated.md:34
• This mitigation is not implemented by the linked runbook. runbooks/org-review.md:18-22,35-40 considers recent workflow failures and generic regressions, but timing flags intentionally leave benchmark jobs successful, and the runbook contains no benchmark/flag inspection step. Update that runbook to inspect successful benchmark summaries or artifacts; otherwise these flags can be omitted from the weekly review.

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The updated ADR, index, manifest routing, and documented impact review are consistent, and prior substantive findings have been addressed.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Balanced