Ion Pull Request Narrative Standard

Use this before opening a draft PR and again before marking it ready for review. A diff and test list are necessary, but not sufficient: the description must preserve the causal history that explains why the patch has its particular shape.

Defect PR admission gate

No reproduction, no PR. For work prompted by a reported defect, do not open or send any pull request—including a draft—until the reported observable failure has been reproduced on a clean baseline and durable evidence has been recorded. A failing mocked/unit test, a suspected causal call, an injected intermediate state, or source inspection does not qualify unless it also produces the reporter’s actual observable failure.

Before creating the PR:

Record the exact baseline SHA and the reporter-level sequence/assertion.

Capture the failing user-visible or externally observable result with an issue-appropriate artifact. For GUI bugs, before-video of the real web or packaged desktop app is the default; screenshots qualify only for static defects. Traces/logs accompany rather than replace visible proof.

Write down assumptions and a falsification attempt; confirm the failure is behavioral rather than boot, fixture, dependency, harness failure, or an instrumentation-created state.

Link public baseline evidence in the PR description so an unaffiliated reviewer can inspect it without access to Ion's private memory.

After the fix, repeat the same user-level sequence unchanged at the exact fix SHA and attach after-video (or the justified non-GUI equivalent) to the PR.

If the issue cannot be reproduced, do not patch it. Record the exact attempts and environment, say when the behavior appears absent, and comment only to explain the blocker or request genuinely missing conditions. Then defer it until new evidence arrives. Do not use a draft PR as the investigation log, and do not promise follow-up reproduction after opening it.

Mandatory self-review and performance evaluation

Before requesting review on every PR:

Re-read the complete diff and trace each changed hot path, including SQL, RPC fanout, cache/invalidation behavior, concurrency limits, allocations, and worst-case complexity.

Use the applicable performance tools already in the repository (Go benchmarks and profiles, benchstat, frontend performance scenarios and traces, browser/network instrumentation, RPC/query counters). Add a focused benchmark when existing scenarios do not exercise the change.

Compare the exact PR base and exact head in the same isolated environment. Use repeated runs and report representative medians plus tail behavior where meaningful; preserve the raw command, environment, and results.

Measure both the ordinary case and a realistic stress case. Timing alone is insufficient when query/RPC count, memory, database statements, payload size, or concurrency can regress.

Put the result in the PR description or review comment. If a change has no runtime execution path (for example documentation-only), state that and explain why performance is not applicable.

Treat unexplained regressions as a design problem: simplify, optimize, or explicitly obtain reviewer acceptance before marking the PR ready.

Context-free reviewer test

A reviewer with no prior context must be able to answer:

What changed upstream? Name the behavior, guard, regression, product decision, or operational event that made this work necessary.

Where did it come from? Link the originating commit, PR, issue, discussion, or Seed document. Include the relevant date or short commit when useful.

Why this design? Explain the constraint chain and why the chosen approach is better than concrete alternatives. Do not merely say that another path “doesn’t work.”

What happens later? State follow-up work and any condition under which a workaround should be removed or reverted.

Recommended description structure

Why now

User/system failure or maintenance need.

Originating change and link.

Causal chain from that change to the observed failure.

What changed

Behavioral summary, not a file list.

Important safety or compatibility boundary preserved.

Why this approach

Alternatives considered.

Why each was rejected or deferred.

Tradeoffs and residual risk.

Validation

Exact tests/checks and their results.

Current-main reproduction or before/after evidence when relevant.

Follow-up / removal condition

Work still owed.

Explicit trigger for deleting a temporary workaround.

Example lesson: PR #1069

The missing backstory was that commit 0f897c776 (2026-09-07, “Disable private document creation to contain exposure access risk”) added the production FailedPrecondition guard. It exposed a test escape hatch only as a package-private field in v3alpha, while daemon e2e tests exercise the real RPC and cannot reach that field. That constraint chain explains why the fixture workaround first prepares public content through the guarded RPC and then signs/stores its own private visibility-bearing Ref instead of bypassing the guard. The description also should say to remove the workaround if supported private creation is safely re-enabled.

Anti-patterns

“This hits the intentional guard” without saying who introduced it, why, or where.

A change list plus green checks with no causal narrative.

“Alternative X was impossible” without explaining the boundary.

Temporary compatibility logic with no deletion condition.

Requiring the reviewer to reconstruct the rationale from git history.

Approval and independent review

Codebase changes require a pull request for human approval. Do not merge on Ion’s own authority. Independent agent review is useful but is not currently a universal required gate; never claim that it replaces the mandatory self-review or human approval. Review must concern the exact head: after substantive changes rerun affected checks and update the evidence.