Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!--
Keep this proportional to the change. The goal is to hand a reviewer a model
they can reconstruct, not to produce a longer changelog.
See ../UNDERSTANDING-REVIEW.md for the review contract.
-->

## What changed

<!-- Summarize the user-visible or operator-visible change. -->

## Why

<!-- Explain the problem or opportunity and why this boundary is appropriate. -->

## Understanding check

- **Intended model:**
- **Prediction:** What should a user, operator, or dependent system observe?
- **Likely misconception or failure mode:**
- **Evidence:** Which test, invariant, trace, dry-run, or live check supports it?
- **Remaining uncertainty:** Write “none” only when the relevant scope is genuinely covered.

## Validation

<!-- List the checks that ran and any skipped checks or residual risk. -->
59 changes: 59 additions & 0 deletions .github/workflows/understanding-review.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
name: Prompt understanding-first review

on:
pull_request_target:
types: [opened, synchronize, reopened]

permissions:
pull-requests: write

jobs:
prompt:
if: github.repository == 'oddharsh/site'
runs-on: ubuntu-latest
steps:
- name: Add or refresh the understanding prompt
uses: actions/github-script@v7
with:
github-token: ${{ github.token }}
script: |
const marker = "<!-- understanding-review -->";
const issue_number = context.payload.pull_request.number;
const comments = await github.paginate(github.rest.issues.listComments, {
owner: context.repo.owner,
repo: context.repo.repo,
issue_number,
per_page: 100,
});
const body = [
marker,
"## Understanding-first review",
"",
"This is a prompt, not a score or merge gate. Before approving, reconstruct the change:",
"",
"- **My model:** What do I think this PR does?",
"- **Mechanism:** What path or invariant makes that behavior true?",
"- **Falsifier:** What would prove my model wrong?",
"- **Evidence:** What test, trace, dry-run, or live check did I inspect?",
"- **Residual uncertainty:** What remains unproven?",
"",
"For docs and dependency changes, a compact answer is enough. For behavior or production changes, include the causal path and one concrete counterexample. The author’s claim card belongs in the PR description; this prompt is for the reviewer’s independent reconstruction.",
"",
"See [UNDERSTANDING-REVIEW.md](https://github.com/oddharsh/site/blob/main/UNDERSTANDING-REVIEW.md).",
].join(String.fromCharCode(10));
const prior = comments.find((comment) => comment.body?.includes(marker));
if (prior) {
await github.rest.issues.updateComment({
owner: context.repo.owner,
repo: context.repo.repo,
comment_id: prior.id,
body,
});
} else {
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number,
body,
});
}
9 changes: 9 additions & 0 deletions MAINTENANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ Before merging the first revision that uses this path, change each Workers
Build project's production branch from `main` to `production`. Otherwise the
merge push can still trigger the old direct production build.

## Understanding-first review

Every pull request uses the author claim card in
`.github/pull_request_template.md` and receives an idempotent reviewer prompt
from `.github/workflows/understanding-review.yml`. The canonical practice is
documented in [UNDERSTANDING-REVIEW.md](UNDERSTANDING-REVIEW.md): reconstruct
the model, name a falsifier, inspect evidence, and leave residual uncertainty
visible. The prompt is advisory; it is not a prose-quality or AI-scoring gate.

Key facts (don't hardcode these elsewhere, they drift):
- RN_KV namespace id: `3cb8a107c58e47dc9244e75b33401f36`
- R2 bucket: `aadhar-photos` (SOOC originals + full-res JPGs)
Expand Down
90 changes: 90 additions & 0 deletions UNDERSTANDING-REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Understanding-first PR review

This repository treats review as an understanding exercise, not only a diff
inspection exercise. A PR is ready to approve when a second person can explain
the intended behavior, trace the mechanism that should make it true, name a
way it could be wrong, point to evidence, and state what remains uncertain.

The practice is deliberately small. It should make the model of a change
clearer, not add a second project-management ritual.

## The default loop

### 1. The author writes the claim card

The PR description should answer five questions:

- **Intended model:** What is changing, and why is this the right boundary?
- **Prediction:** What should a user, operator, or dependent system observe?
- **Likely misconception or failure mode:** What is easy to misunderstand or
what would be the first plausible way this could be wrong?
- **Evidence:** Which test, invariant, trace, dry-run, or live check supports
the prediction?
- **Remaining uncertainty:** What did this PR not prove?

The claim card is a compact handoff from the person who built the change to
the person who will review it. For a documentation or dependency PR, a short
answer or an explicit “none” is enough.

### 2. The reviewer reconstructs the model

Before approving, the reviewer should be able to answer independently:

1. What do I think this change does?
2. What path or invariant makes that behavior true?
3. What would falsify my model?
4. What evidence did I inspect?
5. What remains unproven?

If the reviewer cannot answer those questions, the right response is to ask
for a clearer explanation or better evidence—not to infer understanding from
the size of the diff or the green checkmarks.

### 3. Evidence closes the loop

Tests are evidence about specific claims, not proof that the whole change is
understood. The useful review comment connects the claim to the check:

> I expected X because Y. The check at Z supports X. It does not cover W,
> which remains the residual risk.

That format keeps “passed CI” from becoming a substitute for a causal model.

## Scale it, do not waive it

| Change type | Minimum useful review card |
|---|---|
| Docs or copy | Intended reader change, one prediction, and how wording was checked |
| Dependency or tooling | Update surface, compatibility risk, checks, and a concrete leverage point or “none” |
| UI or behavior | Causal path, falsifiable prediction, likely user misconception, and behavioral evidence |
| Infrastructure or production path | Boundaries, failure/rollback behavior, local and remote evidence, and residual risk |

The questions stay constant; the amount of detail changes with the blast
radius. A five-question quiz is appropriate for a learning page. A PR usually
needs a short claim card and a short reconstruction, not a full exam.

## What the automation does

`.github/workflows/understanding-review.yml` adds or refreshes one maintained
comment on every PR. The comment contains the reviewer prompts and links back
to this document. It is a prompt, not a score and not a merge gate.

The workflow uses `pull_request_target` only to call the GitHub API. It does
not check out the PR branch, run PR code, parse untrusted files, or use an AI
grader. That keeps the default prompt safe for forked PRs and makes the
comment idempotent when a PR is synchronized.

The PR template carries the author claim card. The reviewer prompt remains a
separate comment so the author’s explanation does not become a substitute for
the reviewer’s own reconstruction.

## Anti-patterns

- Do not require polished prose from a small docs or dependency change.
- Do not turn the five prompts into mandatory checkbox theater.
- Do not use an LLM-generated “understanding score” as a merge gate.
- Do not claim that green CI proves behavior the checks do not exercise.
- Do not hide uncertainty behind a complete-sounding summary.

The operating rule is simple: make the model legible, test the important
prediction, and leave the unknowns visible.