From 0c40fb7e19df72a41d45e1790078428983bb2ca2 Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Thu, 11 Jun 2026 16:10:23 +0100 Subject: [PATCH 1/2] DOC-6741 Codex-only skill to review Claude's work --- .codex/skills/claude-review/SKILL.md | 57 +++++++++++++ .../references/claude-review-patterns.md | 79 +++++++++++++++++++ .../claude-review/references/review-style.md | 50 ++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 .codex/skills/claude-review/SKILL.md create mode 100644 .codex/skills/claude-review/references/claude-review-patterns.md create mode 100644 .codex/skills/claude-review/references/review-style.md diff --git a/.codex/skills/claude-review/SKILL.md b/.codex/skills/claude-review/SKILL.md new file mode 100644 index 0000000000..1d0c995c19 --- /dev/null +++ b/.codex/skills/claude-review/SKILL.md @@ -0,0 +1,57 @@ +--- +name: claude-review +description: Use when asked to independently review Claude-authored docs or code changes, Claude-generated diffs, or Claude PR work in the Redis docs repo. +--- + +# Claude Review + +Use this skill for independent reviews of Claude-authored work in this repo. The goal is to find real correctness, docs, consistency, test, and maintainability issues without becoming biased by past Claude behavior. + +## Required References + +Read these before reviewing: + +- [`references/review-style.md`](references/review-style.md) for output format and severity. +- [`references/claude-review-patterns.md`](references/claude-review-patterns.md) for recurring review patterns. + +Treat the pattern file as a map of places to inspect, not as evidence. The current diff and current workspace always win. + +## Workflow + +1. Identify the review target: + - Prefer the user-supplied diff, PR, branch, or file list. + - If the target is local, inspect `git diff`, `git status --short`, and the touched files before judging. + - If the target is a PR, inspect the PR diff and any relevant review comments. +2. Read the nearby source of truth: + - Adjacent docs pages, examples, tests, config, data files, shortcodes, generated mappings, or skill instructions touched by the change. + - Existing repo conventions before claiming a convention was violated. +3. Use `claude-review-patterns.md` as a targeted checklist: + - Pick only patterns relevant to the touched files and change type. + - For each suspected issue, verify the exact current file and line before reporting it. + - Do not report a pattern match unless it causes an actionable problem in this change. +4. Produce findings first: + - Order by severity. + - Include precise file/line references. + - Explain the user-visible or maintainer-visible impact. + - Keep summaries secondary and brief. +5. If no issues are found: + - Say that clearly. + - Mention residual risk, unrun tests, or review limits. + +## Independence Rules + +- Historical patterns suggest what to inspect; they never prove a current bug. +- Never assume Claude made a mistake because Claude made a similar mistake before. +- Verify against the current workspace to avoid stale-snapshot findings. +- Prefer fewer, stronger findings over broad speculative concerns. +- Do not modify this skill or its references during a review unless the user explicitly asks. + +## Skill Memory Updates + +At the end of a review, add a short `Suggested skill-memory updates` section only if the review revealed a reusable pattern that is: + +- concrete enough to scan for in future reviews; +- backed by a current finding or a clearly described false positive; +- not already covered by [`references/claude-review-patterns.md`](references/claude-review-patterns.md). + +Suggest the update text, but do not apply it unless explicitly asked. New entries should follow the schema in `claude-review-patterns.md`. diff --git a/.codex/skills/claude-review/references/claude-review-patterns.md b/.codex/skills/claude-review/references/claude-review-patterns.md new file mode 100644 index 0000000000..f1e976e252 --- /dev/null +++ b/.codex/skills/claude-review/references/claude-review-patterns.md @@ -0,0 +1,79 @@ +# Claude Review Patterns + +This is the living memory file for recurring review patterns in Claude-authored work. These entries are heuristics for where Codex should look during a review. They are not evidence by themselves. + +When adding a new entry, use this schema: + +```markdown +## Pattern Name + +**Status:** candidate | recurring | established +**Source:** file, PR, review date, or pasted review excerpt +**What to check:** concrete search/read strategy +**Pass criterion:** what correct behavior looks like +**False-positive guard:** when this pattern does not apply +**Suggested review prompt:** optional focused prompt for future reviewers +``` + +## Stale Snapshot Findings + +**Status:** established +**Source:** `.agents/skills/redis-use-case-ports/SKILL.md`, Phase 4b independent review notes. +**What to check:** Before reporting or fixing any issue found by a prior reviewer, grep or read the current file for the described pattern and verify the exact line still exists. +**Pass criterion:** Every finding cites the current file state, not a stale diff, parallel-agent snapshot, or earlier review comment. +**False-positive guard:** If the finding is about a removed line or an already-fixed block, do not report it; at most mention that the prior concern appears resolved. +**Suggested review prompt:** Verify each candidate finding against the current workspace before reporting it. Flag stale findings separately from real current issues. + +## Cross-Client Drift + +**Status:** established +**Source:** `.agents/skills/redis-use-case-ports/SKILL.md` synthesis, targeted audit, and cross-client diff workflow. +**What to check:** For changes under `content/develop/use-cases/` or client examples, compare equivalent behavior across all touched client implementations and their guides. +**Pass criterion:** Shared helper APIs, demo behavior, constants, error handling, return shapes, and prose claims are consistent unless a per-client deviation is explicit and justified. +**False-positive guard:** Do not require byte-for-byte code similarity; language idioms, library APIs, and runtime constraints can justify different implementations. +**Suggested review prompt:** Compare the touched client implementation against its siblings and the reference implementation. Report only divergences that change behavior, docs truth, or user expectations. + +## Generated Docs Or Data Drift + +**Status:** recurring +**Source:** Redis docs skill workflows that require examples, mappings, shortcodes, and generated data to stay in sync. +**What to check:** When docs pages reference examples, shortcodes, command metadata, JSON mappings, navigation, or generated artifacts, verify the backing files changed too, or that no backing change is needed. +**Pass criterion:** User-facing docs, example IDs, shortcode paths, data entries, and generated outputs agree with each other. +**False-positive guard:** Some generated files are intentionally not committed; check repo convention before requiring an artifact update. +**Suggested review prompt:** Trace every new or changed docs reference to its backing shortcode, data file, example file, or generated artifact, and flag broken links or stale mappings. + +## Timeout And Race Assumptions + +**Status:** established +**Source:** `.agents/skills/redis-use-case-ports/assets/audit-checklist.md` entries on deadline overflow, subscribe acknowledgement races, background worker stop behavior, and polling loops. +**What to check:** Inspect asynchronous helpers, worker threads, subscription setup, polling loops, lock TTLs, timeout arithmetic, and shutdown paths. +**Pass criterion:** A helper should not return before required background setup is complete; timeout math should use safe clocks; stop paths should be bounded and leave state coherent. +**False-positive guard:** Some synchronous client calls close the race window by completing the protocol write before returning; verify the actual library behavior before reporting. +**Suggested review prompt:** Audit every async, polling, subscription, and worker-lifecycle path touched by the diff for return-before-ready, silent timeout fallthrough, unbounded waits, and unsafe deadline arithmetic. + +## Inert Or Imaginary Configuration + +**Status:** established +**Source:** `.agents/skills/redis-use-case-ports/assets/audit-checklist.md` semantic-cache notes about config keys that looked valid but did not take effect. +**What to check:** For new framework, server, client, build, or package configuration, confirm the option name exists in the real API or in nearby working code. +**Pass criterion:** The configuration is accepted by the runtime/tool and actually enforces the behavior claimed in docs or code comments. +**False-positive guard:** Do not require external documentation for obvious existing repo patterns; nearby tested usage can be sufficient evidence. +**Suggested review prompt:** Verify that each new config option is a real option for the selected framework version and that the code does not rely on a setting the runtime ignores. + +## Redis Search TAG Escaping Drift + +**Status:** established +**Source:** `.agents/skills/redis-use-case-ports/assets/audit-checklist.md` semantic-cache notes about escaped TAG values in docs wire-form snippets. +**What to check:** Inspect Redis Search TAG query examples and prose snippets that include punctuation such as hyphens, dots, braces, spaces, or backslashes. +**Pass criterion:** TAG values shown in query strings escape special characters correctly, including the escape character itself where required. +**False-positive guard:** Do not apply query-string escaping rules to stored raw values, non-TAG fields, or examples intentionally showing unescaped user input before escaping. +**Suggested review prompt:** Check every changed Redis Search TAG filter and docs snippet for correct escaping of punctuation in the query wire form. + +## Shared Demo UI Text Drift + +**Status:** recurring +**Source:** `.agents/skills/redis-use-case-ports/assets/audit-checklist.md` semantic-cache notes about per-language strings in shared HTML. +**What to check:** When multiple demos share HTML, JavaScript, screenshots, or prose templates, look for language-specific labels, default values, or comments that should be populated dynamically. +**Pass criterion:** Shared UI assets do not hardcode one client language's values unless the asset is used only for that client. +**False-positive guard:** Per-language guide prose and per-client server files may intentionally include client-specific naming. +**Suggested review prompt:** Inspect shared demo assets for hardcoded per-client labels, thresholds, ports, or copy that should come from `/state`, config, or each implementation. diff --git a/.codex/skills/claude-review/references/review-style.md b/.codex/skills/claude-review/references/review-style.md new file mode 100644 index 0000000000..08eadd62df --- /dev/null +++ b/.codex/skills/claude-review/references/review-style.md @@ -0,0 +1,50 @@ +# Claude Review Style + +Use a code-review stance. Prioritize bugs, behavioral regressions, docs inaccuracies, missing tests, and maintainability risks. Findings come first. + +## Output Shape + +If there are findings: + +```markdown +Findings + +- [severity] `path:line` Short title. Explain the concrete problem, impact, and why the current change introduces or preserves it. + +Open Questions + +- Only include questions that affect whether a finding is valid or how it should be fixed. + +Notes + +- Briefly mention tests not run, review limits, or residual risk. +``` + +If there are no findings: + +```markdown +No findings. + +Notes: Mention tests not run, files not reviewed, or residual risk. +``` + +## Severity + +- `blocker`: very likely to break builds, publishing, production behavior, or core examples. +- `major`: real correctness, data, security, or user-facing docs issue that should be fixed before merge. +- `minor`: localized bug, confusing docs, missing edge-case handling, or test gap with limited blast radius. +- `nit`: small clarity or maintainability issue; use sparingly. + +## Review Discipline + +- Do not include praise-padding. +- Do not speculate. If the evidence is incomplete, say what would need to be checked. +- Do not bury findings under a summary. +- Prefer one precise finding over several variants of the same root cause. +- Cite current file paths and lines whenever possible. +- Use exact commands or tests only when they help the author reproduce or verify the issue. +- Keep the final summary short and secondary. + +## No-Findings Discipline + +When no issues are found, say so plainly. Still mention any meaningful review limits, such as tests not run, unavailable PR context, or areas that were outside the supplied diff. From f0ac03a91a31f591796636b2349e1e3684556261 Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Thu, 11 Jun 2026 16:23:07 +0100 Subject: [PATCH 2/2] DOC-6741 Bugbot fixes --- .../claude-review/references/claude-review-patterns.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.codex/skills/claude-review/references/claude-review-patterns.md b/.codex/skills/claude-review/references/claude-review-patterns.md index f1e976e252..c66545be95 100644 --- a/.codex/skills/claude-review/references/claude-review-patterns.md +++ b/.codex/skills/claude-review/references/claude-review-patterns.md @@ -28,10 +28,10 @@ When adding a new entry, use this schema: **Status:** established **Source:** `.agents/skills/redis-use-case-ports/SKILL.md` synthesis, targeted audit, and cross-client diff workflow. -**What to check:** For changes under `content/develop/use-cases/` or client examples, compare equivalent behavior across all touched client implementations and their guides. -**Pass criterion:** Shared helper APIs, demo behavior, constants, error handling, return shapes, and prose claims are consistent unless a per-client deviation is explicit and justified. +**What to check:** For changes under `content/develop/use-cases/` or client examples, compare equivalent behavior across touched client implementations, shared use-case prose such as `_index.md`, and the relevant sibling guides or reference implementation even when no client directory changed. +**Pass criterion:** Shared helper APIs, demo behavior, constants, error handling, return shapes, and prose claims are consistent across shared pages and client-specific guides unless a per-client deviation is explicit and justified. **False-positive guard:** Do not require byte-for-byte code similarity; language idioms, library APIs, and runtime constraints can justify different implementations. -**Suggested review prompt:** Compare the touched client implementation against its siblings and the reference implementation. Report only divergences that change behavior, docs truth, or user expectations. +**Suggested review prompt:** Compare the touched use-case page, client implementation, or guide against its siblings and the reference implementation. Report only divergences that change behavior, docs truth, or user expectations. ## Generated Docs Or Data Drift