docs(agents): replace the Review record with a Reviewer Brief

.agents/skills/pre-pr-review/SKILL.md

@@ -1,83 +1,94 @@
 ---
 name: pre-pr-review
-description: Review a RedisVL branch before its PR is marked ready, fix what the review finds, and write a Review record into the PR description for the human approver. Use before opening or marking a PR ready, after a substantive push, or when responding to Bugbot or reviewer comments on a PR.
-compatibility: Works with any coding agent. Needs git, make and pytest. Optional - gh, to read and update the PR; the public Redis Docs MCP at https://redis.io/mcp, for the Redis behaviour lens.
+description: Review a RedisVL branch before a human sees it, fix what holds up, and write a Reviewer Brief into the PR description so the approver knows where to look. Use it whenever you're about to open a PR, mark one ready for review, or push substantive changes to one, and when you're handling Bugbot or reviewer comments, even if nobody asked for a review by name.
+compatibility: Works with any coding agent. Needs git, make, pytest and Python 3. Optional - gh, to read and update the PR; the public Redis Docs MCP at https://redis.io/mcp, for the Redis behaviour lens.
 ---
 
 # Pre-PR review
 
-This review runs before a human sees the PR. Its output is a short Review record in the PR description, telling the approver what was reviewed and what wasn't, so they can check the evidence instead of re-reviewing the change.
+A human approves every merge to `main`. This review runs before they see the PR, so the problems it catches never reach them. Its output, the Reviewer Brief, gives them a head start: where to begin reading, what they need to decide, where the change could still fail, and what nobody has checked.
 
-The review is only useful if the approver can trust it. Report only what you can tie to the code, fix only what survives challenge, and never claim a check you didn't run.
+The approver can only rely on the brief if it's accurate, so report only what you can tie to the code, and never claim a check you didn't run.
 
 ## 1. Scope the change
 
-- Diff the branch against its merge base with `main`: `git diff $(git merge-base origin/main HEAD)...HEAD`.
-- Read each changed file in full, not just the hunks.
-- For every changed public function, method or class, read its callers and its sync or async twin (`SearchIndex` and `AsyncSearchIndex`, and so on).
-- Note the size (lines added outside `tests/` and `docs/`), the type (fix, feat, refactor, docs, test, chore or CI) and the paths touched. Step 2 uses all three.
+- Diff the work against its merge base with `main`: `git diff $(git merge-base origin/main HEAD)` covers commits and uncommitted changes, and `git status` shows untracked files to read as well.
+- Read each changed file in full, not just the hunks. Bugs often sit in the unchanged lines a hunk depends on.
+- For every changed public function, method or class, read its callers and its sync or async twin (`SearchIndex` and `AsyncSearchIndex`, and so on). Drift between twins is one of this project's most common defects.
+- In a stack of PRs you may review the whole stack at once, but keep track of which PR introduced each line. Each PR gets its own brief.
 
 ## 2. Choose lenses
 
-Pick the lenses this diff needs, and write down the reason for each one you run and each one you skip. Most PRs need two or three.
+Run all six lenses by default. Each catches problems the others miss, and running all of them is the most thorough option.
 
-| Lens | File | Run it when |
+A lens that finds nothing relevant in the diff has still done its job; say nothing about it beyond the footer. If you need to save tokens, the "Most useful when" column says where each lens pays off. Correctness always runs. The footer of the brief lists the lenses that ran, so the reviewer can see what was covered.
+
+| Lens | File | Most useful when |
 |---|---|---|
-| Correctness | `references/correctness.md` | Always. A single quick pass for diffs under 100 lines and for chore, CI or refactor changes |
+| Correctness | `references/correctness.md` | Always |
 | Security | `references/security.md` | The diff touches `redisvl/mcp/`, `redisvl/query/filter.py`, `redisvl/utils/token_escaper.py`, code that builds query strings or `SCAN` patterns, destructive methods (`delete`, `clear`, drop), `redisvl/redis/connection.py`, `.github/`, or dependencies |
-| Design | `references/design.md` | Feature PRs over about 300 lines; new or changed public API or MCP surface; work on deprecated APIs; new vectorizer, reranker or provider modules |
+| Design | `references/design.md` | Feature work, new or changed public API or MCP surface, work on deprecated APIs, new vectorizer, reranker or provider modules |
 | Tests | `references/tests.md` | Tests are added or changed, or the change sits in an area CI can't exercise (cluster, live provider APIs) |
 | Docs | `references/docs.md` | `docs/`, notebooks, docstrings or comments change, or public behaviour changes |
-| Redis behaviour | `references/redis.md` | The diff changes how Redis commands or queries are built, relies on a server default, limit or reply shape, or states how Redis behaves. Checks claims against the official docs, ideally through the Redis Docs MCP |
-
-If you're unsure whether a lens applies, run it. A lens you skip is a gap the approver should know about, so the record lists it.
+| Redis behaviour | `references/redis.md` | The diff changes how Redis commands or queries are built, relies on a server default, limit or reply shape, or states how Redis behaves |
 
 ## 3. Review, one lens at a time
 
-Run one pass per chosen lens. Start each pass from the diff and that lens's file, not from an earlier pass's conclusions.
+Run one pass per lens. Start each pass from the diff and that lens's file, not from an earlier pass's conclusions, so that one pass's blind spots don't become the next one's.
 
 - If your agent can start isolated sub-tasks (subagents, sub-sessions, parallel workers), give each lens its own, and run them in parallel.
 - If it can't, run the lenses one after another, and write each pass's findings to a scratch file before starting the next.
 
-Every finding takes the same shape:
+Write each finding in this shape, so the challenger can check it quickly:
 
 ```
 [lens] path/to/file.py:LINE: claim, in plain words. Failure: the input or state that goes wrong, and what happens. Fix: the smallest change that resolves it.
 ```
 
-Drop formatting, import order and typing nits. CI and `make lint` already handle them.
+Skip formatting, import order and typing nits, which CI and `make lint` already catch.
 
 ## 4. Challenge the findings
 
-Run a separate challenger pass, as described in `references/challenger.md`. It starts from the code and the list of findings, not from the reasoning that produced them, and scores each finding from 1 to 10. Keep findings that score 7 or more and have a file, a line and a concrete failure or cost. Record the rest as rejected, each with its one-line reason.
+Run a separate challenger pass, as described in `references/challenger.md`. Review passes produce false positives, and every one that survives costs someone time. Keep findings that score 7 or more out of 10 and have a file, a line and a concrete failure or cost.
 
 Run a second challenge round only if the first round's fixes changed the code materially.
 
 ## 5. Fix and re-check
 
-- Fix only the kept findings, and only within the PR's scope. A real problem outside scope goes into the record as an open item, not into the diff.
-- Fix the root cause. Add the narrowest regression test that fails without the fix, or say why none applies.
-- Re-run whatever the fixes touched: `make format`, `make check-types`, and the affected tests (`make test ARGS="tests/unit/test_x.py"` or `pytest` on the specific file).
+- Fix the kept findings that are within the PR's scope, at the root cause. A real problem outside scope becomes a "Decisions for you" item rather than an unrequested change.
+- Add the narrowest regression test that fails without the fix, or note why none applies. A test you've seen fail without the fix is strong evidence for the brief's "Beyond CI" line.
+- Re-run whatever the fixes touched: `make format`, `make check-types`, and the affected tests (`make test ARGS="tests/unit/test_x.py"`, or `pytest` on the specific file).
+- Commit the fixes locally. The author pushes.
+
+## 6. Write the Reviewer Brief
 
-## 6. Write the Review record
+Write the brief following `references/reviewer-brief.md`. The reviewer sees the change as it is now, so the brief points at the current code, not at the problems you fixed.
 
-Fill in the template in `references/review-record.md`. It replaces everything under the `## Review record` heading of the PR description, including the template's placeholder comment or any earlier record, and leaves the rest of the description untouched.
+To put it in the PR description, save the brief to a file and run:
+
+```
+python3 .agents/skills/pre-pr-review/scripts/update_brief.py <pr-number> <brief-file>
+```
 
-With `gh`: fetch the current description (`gh pr view <number> --json body -q .body`), replace only that section, write the whole description to a file and run `gh pr edit <number> --body-file <file>`. `--body-file` replaces the entire description, so never pass it the record alone. Without `gh`, print the record for the author to paste.
+The script replaces only the `## Reviewer Brief` section and keeps the rest of the description, including Bugbot's summary. Add `--dry-run` to print the result without changing the PR. If the PR doesn't exist yet, put the brief under `## Reviewer Brief` in the description you open it with. Without `gh`, give the brief to the author to paste.
 
 ## Responding to review
 
-When Bugbot or a person comments on the PR, handle each point like this:
+When Bugbot or a person comments on the PR:
 
-1. **Gather everything open.** That means review threads that are neither resolved nor outdated, review summaries and top-level PR comments. With `gh`, `gh pr view <number> --comments` shows reviews and top-level comments, and `gh api --paginate repos/{owner}/{repo}/pulls/<number>/comments` lists inline comments. Resolution state is only exposed through the GraphQL `reviewThreads` field, so check it there or on GitHub.
-2. **Verify each point against the current code.** The reviewer's wording is a claim to check, not a verdict.
-3. **Give it exactly one disposition:**
+1. **List what's still open:**
+   ```
+   python3 .agents/skills/pre-pr-review/scripts/open_feedback.py <pr-number>
+   ```
+   It prints unresolved, non-outdated review threads, review summaries and top-level comments, each with its author and location.
+2. **Verify each point against the current code.** A reviewer's wording, human or bot, is a claim to check, not a verdict.
+3. **Decide its outcome:**
    - `fixed in <sha>`
    - `already fixed`
    - `invalid`, with the evidence
    - `duplicate of <item>`
    - `needs a decision`, for the author or a maintainer to settle
-4. **Before you reply "fixed", check the fix.** Re-read the exact file and line in the fixed tree, and search for other instances of the same pattern.
-5. **Reply and record it.** Answer each thread with its disposition, then add one line per point to the Review record's "Review comments" list.
+4. **Check the fix before saying so.** Re-read the exact location in the fixed tree, and search for other instances of the same pattern, before you reply "fixed".
+5. **Reply on each thread with its outcome, and bring the brief up to date.** Fixes that change behaviour need a fresh review of the new commits and a rewritten brief, because the reviewer will rely on it. Trivial fixes need only a "Since the last brief" line. Anything that needs a decision goes under "Decisions for you".
 
-Commit fixes locally; the author pushes. Ask the author before you post replies on the PR, unless they've already told you to.
+Ask the author before posting replies on the PR, unless they've already said you can.

.agents/skills/pre-pr-review/evals/evals.json

@@ -0,0 +1,41 @@
+{
+  "skill_name": "pre-pr-review",
+  "notes": "Fill in any <placeholder> with a current PR before running, since PR state changes. Run each prompt with and without the skill, from a checkout of the PR's head commit. Compare the Reviewer Briefs against the expectations. Never let an eval run push, edit a PR description or post comments: use update_brief.py --dry-run.",
+  "evals": [
+    {
+      "id": 1,
+      "prompt": "I'm about to mark PR 777 (the OpenTelemetry spans one) ready for review. Run the pre-PR review on it and give me the Reviewer Brief, but don't touch the PR.",
+      "expected_output": "A brief that a reviewer who never saw the review can act on.",
+      "expectations": [
+        "\"Start here\" names one file or function from the diff and says why it matters.",
+        "No section lists bugs that were fixed during the review, finding counts or lens names outside the footer.",
+        "\"Where it could fail\" has at most three items, each with a file and line, or a clearly named behaviour, and a concrete way it could go wrong.",
+        "\"Not verified\" names live provider APIs or cluster mode.",
+        "\"Beyond CI\" contains only checks CI does not run, or is omitted.",
+        "The footer names the reviewed commit, the agent and model, and the lenses that ran.",
+        "The visible brief is under about 200 words."
+      ]
+    },
+    {
+      "id": 2,
+      "prompt": "Quick one: I fixed a typo in a docstring in redisvl/query/filter.py and a broken link in docs/user_guide/how_to_guides/observability.md. Do the pre-PR review thing and write the brief.",
+      "expected_output": "A very short brief that scales to the change.",
+      "expectations": [
+        "The brief is under about 60 words.",
+        "\"Decisions for you\" says None, and nothing is invented to fill \"Where it could fail\".",
+        "The review does not raise findings about code the change did not touch."
+      ]
+    },
+    {
+      "id": 3,
+      "prompt": "Bugbot left comments on PR <number of an open PR with unresolved Bugbot threads>. Go through them using the repo's review process and tell me what you'd reply to each. Don't post anything.",
+      "expected_output": "Each open point verified against the code and given exactly one outcome.",
+      "expectations": [
+        "The agent lists the open feedback with scripts/open_feedback.py or an equivalent that respects resolved and outdated threads.",
+        "Each point gets exactly one of: fixed in <sha>, already fixed, invalid with evidence, duplicate, needs a decision.",
+        "Points that are already resolved on the PR are not re-litigated.",
+        "Nothing is posted to the PR."
+      ]
+    }
+  ]
+}

.agents/skills/pre-pr-review/references/challenger.md

@@ -18,6 +18,6 @@ For each finding:
 | 4–6 | Plausible but unproven: it depends on inputs or states you couldn't confirm can occur |
 | 1–3 | Wrong, already handled, out of scope, or a style preference |
 
-Keep findings that score 7 or more and have a file, a line and a concrete failure or cost. Reject the rest. Each rejection gets a one-line reason, because the reasons go into the Review record and show the approver that the review discriminated.
+Keep findings that score 7 or more and have a file, a line and a concrete failure or cost. Reject the rest, each with a one-line reason in your scratch notes. A rejection only reaches the Reviewer Brief if a reasonable reviewer might side with the finding; then it goes under "Decisions for you".
 
 The challenger may also add a finding it noticed while checking. A finding it adds gets the same scoring, done in a fresh pass.

.agents/skills/pre-pr-review/references/correctness.md

@@ -16,7 +16,7 @@ Find changes that make the code behave wrongly for some input, state or environm
 - **Dependency floors.** Check every call into an optional dependency against the oldest version `pyproject.toml` allows. A new attribute or keyword that only exists in a later release crashes users on the floor version. This is the most common high-severity finding in this repository.
 - **Redis replies.** The code has to handle RESP2 and RESP3 reply shapes, and both hash and JSON storage.
 - **Result limits.** Default result counts (`num_results`, `LIMIT`, `MAXSEARCHRESULTS`) silently truncate. Check any read that is meant to return everything.
-- **Cluster mode.** Multi-key operations, `SCAN` and pipelines behave differently on a cluster, and partial failures are possible. CI never runs cluster tests, so reason about these cases from the code. If you can't settle one that way, put it under "Not checked" in the record.
+- **Cluster mode.** Multi-key operations, `SCAN` and pipelines behave differently on a cluster, and partial failures are possible. CI never runs cluster tests, so reason about these cases from the code. If you can't settle one that way, put it under "Not verified" in the Reviewer Brief.
 - **Client ownership.** Who creates, owns and closes a Redis client, and what happens on reconnect, especially for async clients reused across event loops.
 
 ## Depth

.agents/skills/pre-pr-review/references/redis.md

@@ -12,7 +12,7 @@ Redis hosts a public, read-only MCP server for its documentation at `https://red
 
 Send it questions about Redis, not code from the diff, credentials or anything unreleased. The corpus is the redis.io documentation, which includes RedisVL's own docs. Check each page's version metadata against the Redis versions RedisVL supports before you rely on it.
 
-If your agent can't use MCP, read the same pages on `https://redis.io/docs/latest/`. If you can reach neither, skip the lens and put it under "Not checked" in the record.
+If your agent can't use MCP, read the same pages on `https://redis.io/docs/latest/`. If you can reach neither, skip the lens and say so under "Not verified" in the Reviewer Brief.
 
 ## Run it when
 
@@ -31,4 +31,4 @@ If your agent can't use MCP, read the same pages on `https://redis.io/docs/lates
 
 ## How to report
 
-Each finding cites the page it rests on: `... Per: <redis.io URL>`. A claim you couldn't confirm in the docs is not a finding. List it under "Not checked" instead, so the reviewer knows it was raised but not settled. Record each page you relied on in the record's "Docs consulted" line.
+Each finding cites the page it rests on: `... Per: <redis.io URL>`. A claim you couldn't confirm in the docs is not a finding. If it matters, list it under "Not verified" in the Reviewer Brief, so the reviewer knows it was raised but not settled. When a point in the brief rests on a docs page, link the page in that line.