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..c66545be95 --- /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 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 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 + +**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.