[Flue] Add review validation step to suppress false-positive findings

[mvvmm]

Summary

Inserts a validation step between reconcile and publish in the ReviewOrchestrator pipeline. The validator agent (GLM-5.2) reads the actual repo file content at the PR head SHA and checks each active finding for accuracy, applicability, and false positives. Invalid findings are suppressed before publishing and before R2 persistence, so rejected findings are not carried forward in incremental reviews.

Design

• Suppress-only: the validator can remove false positives but never add findings
• Fail-open: on timeout/error/missing output, all findings are kept
• Degraded streams (specialist failed) skip validation — carried-forward findings may reference files that have since changed
• Streams as data: represented as an array for extensibility — future specialists append another stream before validation
• R2 persistence moved from reconcile to after validation, so only validated findings are carried forward

New files

• agents/review-validator.ts — Flue agent using cloudflare/@cf/zai-org/glm-5.2 with read_repo_file + search_repo tools and submit_review_validation structured output
• .agents/skills/review-validation/SKILL.md — validation skill with rules for checking evidence, rule applicability, suggestion correctness, and false positives
• lib/run-review-validation.ts — driver with applyValidationDecisions (pure, unit-tested), runReviewValidation (agent round-trip), validateStream (fail-open wrapper)
• lib/run-review-validation.test.ts — 8 unit tests covering fail-open, duplicates, unknown IDs, empty arrays
• evals/review-validation.eval.ts — 3 eval cases: valid finding kept, false positive suppressed (proper error handling), style false positive suppressed (img in code block)

Modified

• cloudflare.ts — new validate-findings workflow step; R2 persistence moved after validation; publish/mark-auto-review/return use validated instead of reconciled
• app.ts — ReviewValidator added to EVAL_AGENTS
• wrangler.jsonc — v11 migration adding FlueReviewValidatorAgent
• vite.config.ts — eval mock redirect extended to review-validator.ts
• evals/mocks/github-repo-tools.ts — added makeSearchRepoTool mock + validator eval fixtures
• AGENTS.md — updated agent table, pipeline description, request flow, models, migrations, eval coverage

Validation

• tsc --noEmit — pass
• vitest run — 161/161 pass (10 test files)
• vite build — pass
• wrangler deploy --dry-run — pass, FlueReviewValidatorAgent DO binding confirmed

Scope

Only affects ReviewOrchestrator (the generic MR review pipeline). IngestWorkflow (spam/off-topic), DependabotReviewWorkflow, and RebaseWorkflow are untouched.

[cloudflare-docs-bot]

Review

⚠️ 2 warnings, 💡 3 suggestions found in commit 345ce1d.

👉 Fix in your agent 👈

Fix the following review findings in PR #32614 (https://github.com/cloudflare/cloudflare-docs/pull/32614).

Before making changes, review each finding and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g. the fix requires disproportionate effort for minimal benefit,
  or the finding is factually incorrect), explain why
- If you need clarification before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

After triaging, follow this order:
1. Post a comment on this PR for any findings you are skipping, with the finding ID and your reasoning.
2. Then commit the fixes for the legitimate findings.

The comment must come before the commit — the bot reads PR comments when a new
push triggers a review, so skip comments posted after the push will be missed.

---

## Code Review

### Warnings (2)

#### CR-1cb42b5f774f · Unwrapped GitHub API call in optional validation step
- **File:** `.flue/cloudflare.ts` line 559
- **Issue:** The validation step awaits `getPullRequestFiles(token, number)` inside `step.do("validate-findings", ...)` with no try/catch. A network or GitHub API failure here will fail the whole workflow step, even though validation is a fail-open filter that could otherwise continue with reduced context.
- **Fix:** Wrap the context fetch in a try/catch so a transient failure falls back to `changedFiles = []` and `prTemplate = ""` rather than failing the orchestrator step.

#### CR-4baaf8fd7e60 · Conventions rule validation logic
- **File:** `.flue/.agents/skills/review-validation/SKILL.md` line 52
- **Issue:** The skill invalidates conventions findings whose rule doesn't match a rule defined in conventions-check, but conventions-check defines rules only as prose headings ('Product or area identified (warning)', 'Description explains the work (warning)', 'Scope accuracy (warning)') with no stable machine-readable rule IDs. Its own example uses the rule label 'PR title format', which doesn't match any heading, so the validator would mark that example invalid.
- **Fix:** Add stable rule names/IDs to the conventions-check skill and specify the matching scheme exactly, or remove the exact-match invalidation and rely on semantic rule-applicability checks.

### Suggestions (3)

#### CR-57ddbf6d6c36 · Array-index result extraction is order-dependent
- **File:** `.flue/cloudflare.ts` line 624
- **Issue:** After `Promise.all(streams.map(...))`, the validated streams are referenced by index: `validatedResults[0]` for code, `[1]` for conventions, and `[2]` for style. The matching order is defined only a few lines earlier, so any future reordering of the `streams` array will silently swap categories.
- **Fix:** Return an object keyed by stream label (e.g., `{ code, conventions, style }`) instead of destructuring by index.

#### CR-3fe3228ac4ef · Missing diff line data for pre-existing-code check
- **File:** `.flue/.agents/skills/review-validation/SKILL.md` line 60
- **Issue:** Step 5 instructs validators to mark findings invalid when they are about 'pre-existing code the PR did not change', but args.changedFiles only provides filename, status, additions, and deletions — not changed line ranges or the base-ref content. The validator cannot reliably tell whether a specific cited line within a changed file is pre-existing.
- **Fix:** Add per-file changed-line ranges or the base ref to the validation inputs, or narrow the instruction to only suppress findings whose path is not present in args.changedFiles.

#### CR-f272b59c8456 · Weak assertion for duplicate-decision preference
- **File:** `.flue/lib/run-review-validation.test.ts` line 58
- **Issue:** The test titled "prefers valid over invalid for duplicate decisions" only asserts `toHaveLength(1)`. With a single input finding, this only confirms the finding was not removed entirely; it does not verify that the valid verdict actually takes precedence over the invalid one.
- **Fix:** Strengthen the assertion to verify the specific finding is returned, e.g. `expect(result.map((f) => f.id)).toEqual(["CR-1"])`.

Code Review

This code review is in beta and may not always be helpful — use your judgment.

Warnings (2)

File	Issue
.flue/cloudflare.ts line 559	Unwrapped GitHub API call in optional validation step — The validation step awaits getPullRequestFiles(token, number) inside step.do("validate-findings", ...) with no try/catch. A network or GitHub API failure here will fail the whole workflow step, even though validation is a fail-open filter that could otherwise continue with reduced context. Fix: Wrap the context fetch in a try/catch so a transient failure falls back to changedFiles = [] and prTemplate = "" rather than failing the orchestrator step.
.flue/.agents/skills/review-validation/SKILL.md line 52	Conventions rule validation logic — The skill invalidates conventions findings whose rule doesn't match a rule defined in conventions-check, but conventions-check defines rules only as prose headings ('Product or area identified (warning)', 'Description explains the work (warning)', 'Scope accuracy (warning)') with no stable machine-readable rule IDs. Its own example uses the rule label 'PR title format', which doesn't match any heading, so the validator would mark that example invalid. Fix: Add stable rule names/IDs to the conventions-check skill and specify the matching scheme exactly, or remove the exact-match invalidation and rely on semantic rule-applicability checks.

Suggestions (3)

File	Issue
.flue/cloudflare.ts line 624	Array-index result extraction is order-dependent — After Promise.all(streams.map(...)), the validated streams are referenced by index: validatedResults[0] for code, [1] for conventions, and [2] for style. The matching order is defined only a few lines earlier, so any future reordering of the streams array will silently swap categories. Fix: Return an object keyed by stream label (e.g., { code, conventions, style }) instead of destructuring by index.
.flue/.agents/skills/review-validation/SKILL.md line 60	Missing diff line data for pre-existing-code check — Step 5 instructs validators to mark findings invalid when they are about 'pre-existing code the PR did not change', but args.changedFiles only provides filename, status, additions, and deletions — not changed line ranges or the base-ref content. The validator cannot reliably tell whether a specific cited line within a changed file is pre-existing. Fix: Add per-file changed-line ranges or the base ref to the validation inputs, or narrow the instruction to only suppress findings whose path is not present in args.changedFiles.
.flue/lib/run-review-validation.test.ts line 58	Weak assertion for duplicate-decision preference — The test titled "prefers valid over invalid for duplicate decisions" only asserts toHaveLength(1). With a single input finding, this only confirms the finding was not removed entirely; it does not verify that the valid verdict actually takes precedence over the invalid one. Fix: Strengthen the assertion to verify the specific finding is returned, e.g. expect(result.map((f) => f.id)).toEqual(["CR-1"]).

Conventions

_{No convention issues found.}

Style Guide Review

_{No style-guide issues found.}

---

Acknowledged by author (1)

Review	File	Issue	Note
Code	src/components/OneTrust.astro line 27	OneTrust callback ordering	The author states this file was accidentally committed from another branch and will be removed via rebase, so they are not fixing it in this PR.

Commands

Only codeowners can run commands. Post a comment with the command to trigger it.

Command	Description
/review	Runs a review now. Incremental if a prior review exists, full if not.
/full-review	Re-reviews the entire PR diff from scratch, ignoring incremental history. Useful after a rebase, when you want a fresh review, or if the bot gets out of sync and reports issues that no longer exist.
/ignore-review-limit	Permanently lifts the 2-review automatic limit for this PR. Future pushes will trigger reviews as normal.
/disable-auto-review	Stops automatic reviews from triggering on future pushes to this PR. Codeowners can still run /review or /full-review manually.
/rebase	Rebases the PR branch against production. On conflict, attempts to resolve automatically using AI. Stops with an explanation if confidence is not high enough.

[mvvmm]

Review Finding Triage

Skipping

Finding	Reason
CR-0afe15ef7d0f · Test file not matched by Vitest workspace patterns	Disagree — .flue/vitest.config.ts uses include: ["**/*.test.ts"], which picks up all .test.ts files regardless of suffix. All existing .flue test files (run-reconcile.test.ts, code-review-files.test.ts, webhook-classify.test.ts, etc.) use the same pattern without .node.test.ts suffixes. The test suite confirms: 161/161 tests pass across 10 files including run-review-validation.test.ts.

[mvvmm]

Review Finding Triage (Round 2)

Skipping

Finding	Reason
CR-0afe15ef7d0f · Test file not matched by Vitest workspace patterns	Disagree — same finding as round 1. .flue/vitest.config.ts uses include: ["**/*.test.ts"], which picks up all .test.ts files. The root repo's vitest.config.ts with workspace projects (*.node.test.ts, *.worker.test.ts, etc.) does not apply to the .flue subdirectory — it has its own vitest config. All 161 unit tests pass across 10 files, including run-review-validation.test.ts.

[github-actions]

This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:

Pattern	Owners
*	@cloudflare/product-owners
*.ts	@cloudflare/content-engineering, @kodster28
*.astro	@cloudflare/content-engineering, @kodster28
/src/components/ui/search/	@cloudflare/content-engineering, @cloudflare/ai-search

[mvvmm]

Review Finding Triage (Round 3)

Skipping

Finding	Reason
CR-de51d8b56c59 · OneTrust callback ordering	Disagree — src/components/OneTrust.astro is not part of this PR's intended changes. It was accidentally committed in a stray commit (b5cd854d4b) from another branch. That commit will be rebased out.
CV-4e2ea484ef86 · Scope accuracy	Disagree — the PR description accurately accounts for all .flue/ changes. The 5 src/ files are from the same stray commit and will be removed via rebase.

[github-actions]

Preview URL: https://cc4c3a6b.preview.developers.cloudflare.com
Preview Branch URL: https://flue-validation-step.preview.developers.cloudflare.com