mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
Ruben Fiszel
11b60e5b95
* ci: share review policy across Claude/Codex/Pi via review-prompt-shared.md All three reviewers now consume a single canonical policy document (.github/review-prompt-shared.md) covering AGENTS.md compliance, severity triage (P0/P1/P2), and a checklist for new public surfaces (auth contract, module placement, half-finished pub fns, input validation). Each tool's own prompt file shrinks to just its output-format quirks, and each workflow concatenates shared + tool-specific at runtime before invoking the model. Drops the suppressive "Prefer at most 10 findings" / "Keep the review high signal. If there is no clear issue, return no findings" wording from Codex and Pi, which was clipping P1 and P2 findings (e.g. half-finished pub fn, blocking I/O, wrong module placement). Replaces it with severity triage so both reviewers report all P0/P1 and surface P2 when the diff invites it. Also makes AGENTS.md authoritative for Codex (was CLAUDE.md, which is just @AGENTS.md in this repo) and adds an explicit "new public function" checklist that covers the missing-auth-check failure mode none of the three reviewers flagged on the test PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: move test-coverage assessment to shared prompt, slim per-tool prompts - Replace per-tool 'Reproduction instructions' with a single shared 'Test coverage assessment' section that asks each reviewer to evaluate automated coverage (sufficient / thin / appropriate) and describe what manual verification remains, if any. - Slim per-tool prompts to the absolute minimum: just where to read context, the comment header, severity tagging, and the Pi-only 'no preamble' constraint. Everything else lives in the shared policy. - Drop the model name from Pi's title ('Pi Review (DeepSeek V4)' → 'Pi Review') — the title's job is to let the bot find its own prior comment when re-reviewing; the model is irrelevant to the reader. The titles ('## Codex Review', '## Pi Review') stay because Codex and Pi both post as github-actions[bot], so the heading is the only discriminator the bot can use to find its own past comment in the prior-discussion context. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: scope test-coverage assessment to layers actually changed Don't ask reviewers about integration tests on a frontend-only diff or about playwright tests on a backend-only diff. The shared 'Test coverage' section now lists categories (backend / frontend / CI-docs) and tells the reviewer to skip the ones the PR does not touch — only ask about Rust integration tests when backend handlers/workers/queues were modified, only ask about frontend tests when components or state machines were touched, and explicitly call out 'no automated tests expected' for CI/docs/config diffs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: don't ask reviewers to flag missing frontend component tests The Windmill frontend codebase doesn't generally test Svelte components — existing tests cover pure-logic utilities only (flowDiff, previousResults, copilot logic, dbtable queries, etc.). Asking reviewers to flag every new component for lacking a test would produce noise inconsistent with the established convention. Limit the frontend test-coverage check to new pure-logic utilities (files that would naturally have a sibling *.test.ts). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: point local-review skill at the shared review policy Codex flagged (and Pi confirmed on its second pass) that slimming .claude/review-prompt.md to output-only broke the local-review skill contract — the skill still told Claude to read only that file for the review criteria, so /local-review would no longer apply severity triage, the public-surface checklist, or AGENTS.md compliance. Update the skill to read .github/review-prompt-shared.md as the policy source and .claude/review-prompt.md only for Claude output preferences. Also align the local output format with the severity-tag convention used by the workflow reviewers, and replace the lingering 'CLAUDE.md compliance' wording with 'AGENTS.md compliance'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>