[Flue] Add review validation step to suppress false-positive findings - #32614
Merged
Conversation
Contributor
Review✅ No issues found in commit Code ReviewThis code review is in beta and may not always be helpful — use your judgment. No code review issues found. ConventionsNo convention issues found. Style Guide ReviewNo style-guide issues found. CommandsOnly codeowners can run commands. Post a comment with the command to trigger it.
|
Collaborator
Author
Review Finding TriageSkipping
|
Collaborator
Author
Review Finding Triage (Round 2)Skipping
|
Contributor
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
Collaborator
Author
Review Finding Triage (Round 3)Skipping
|
mvvmm
force-pushed
the
flue-validation-step
branch
from
August 7, 2026 20:48
cc4c3a6 to
345ce1d
Compare
Contributor
|
Preview URL: https://cc4c3a6b.preview.developers.cloudflare.com |
Collaborator
Author
|
/rebase |
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 represented as data array for extensibility — future specialists append another stream before validation New files: - agents/review-validator.ts — Flue agent using GLM-5.2 with read_repo_file + search_repo tools and submit_review_validation - .agents/skills/review-validation/SKILL.md — validation skill - lib/run-review-validation.ts — driver with applyValidationDecisions (pure), runReviewValidation, validateStream (fail-open) - lib/run-review-validation.test.ts — 8 unit tests - evals/review-validation.eval.ts — 3 eval cases Modified: - cloudflare.ts — validate-findings step, R2 persistence moved - app.ts — ReviewValidator in eval routes - wrangler.jsonc — v11 migration (FlueReviewValidatorAgent) - vite.config.ts — eval mock redirect for review-validator - evals/mocks/github-repo-tools.ts — makeSearchRepoTool mock + fixtures - AGENTS.md — agent table, pipeline description, models, migrations
- CR-03e9920f0b33: Validate submit_review_validation payload — enforce one decision per finding, unique ids, and matching ids before writeResult - CR-1f5937ec6fb7: Add deleted-file handling to skill — if file is removed or not found at head SHA, mark finding invalid - CR-b57ab0753ec4: Define the three conventions rules in the skill instead of naming them without explanation - CR-99186f263b21: Extract shared FENCED_IMG_FIXTURE constant to deduplicate eval fixture content - CR-ad165f9dc29d: Align review-validator row in agents table - CR-0b8f95ae717c: Update vite.config.ts comment to include review-validator in the redirect description Skipped: CR-0afe15ef7d0f (vitest config uses include: ["**/*.test.ts"], all existing .flue test files use the same pattern)
The 'suppresses a false-positive finding on proper error handling' eval case asserts verdict === 'invalid', but the live model (GLM-5.2) may not always correctly identify false positives. Match the pattern used by other live model evals: assert the contract was fulfilled (decision produced + tool called) rather than the specific verdict.
- CR-be49bc18d8f0: Broaden missing-file detection — instruct validator to check for any 'not found' or empty response, not exact string match - CR-21324151c7c1: Reference conventions-check skill file instead of inlining the three allowed rules, so new rules don't require updating the validation skill - CR-9b3970826741: Rename eval case to 'reads a file with proper error handling and produces a decision' — reflects what the assertion actually tests (contract fulfillment), not suppression behavior Skipped: CR-0afe15ef7d0f (same as round 1 — .flue has its own vitest config with include: ["**/*.test.ts"])
- CR-57f52043f9d5: Remove the 'reads a file with proper error handling' eval case — its assertion accepted either verdict, providing no regression protection. The other two cases (valid finding kept, style false positive suppressed) cover the validator adequately. Also remove the now-unused eval-val-proper-handling fixture. - Rebase out stray commit b5cd854 ('fix: resolve remaining Astro check hints') which accidentally included 5 src/ files from another branch. PR now contains only .flue/ changes. Skipped: CR-de51d8b56c59 (OneTrust callback ordering — file not part of this PR), CV-4e2ea484ef86 (scope accuracy — src/ files removed).
cloudflare-docs-bot
Bot
force-pushed
the
flue-validation-step
branch
from
August 12, 2026 16:45
345ce1d to
a7592ee
Compare
- Fix eval finding line: 11 → 8 to match the actual <img> location in the shared FENCED_IMG_FIXTURE (line 8, not 11) - Swap validation procedure steps: rule applicability now comes before evidence check, so code-block suppression takes precedence over the 'wrong line but issue exists elsewhere → valid' rule - Add explicit 'regardless of line number accuracy' to the code-block rule to remove the ambiguity that caused the model to default to valid
CR-57788a8f3cb6: The skill told validators to mark findings invalid for pre-existing code but didn't explain how to determine that. The validator already has read_repo_file (accepts any ref) and pullRequest.base in its inputs — the skill just needs to tell it to use them.
The single-line <img> between fences was too minimal for the validator model to reliably identify as a code block. Expanded to a multi-line HTML block (div + two img tags) so the code fence is unambiguous. Updated shared FENCED_IMG_FIXTURE and both evals that use it: - style-guide.eval.ts: added lines now cover full code block (7-12) - review-validation.eval.ts: finding line 8 → 9, additions 3 → 6
kodster28
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Inserts a validation step between reconcile and publish in the
ReviewOrchestratorpipeline. 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
New files
agents/review-validator.ts— Flue agent usingcloudflare/@cf/zai-org/glm-5.2withread_repo_file+search_repotools andsubmit_review_validationstructured output.agents/skills/review-validation/SKILL.md— validation skill with rules for checking evidence, rule applicability, suggestion correctness, and false positiveslib/run-review-validation.ts— driver withapplyValidationDecisions(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 arraysevals/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— newvalidate-findingsworkflow step; R2 persistence moved after validation; publish/mark-auto-review/return usevalidatedinstead ofreconciledapp.ts—ReviewValidatoradded toEVAL_AGENTSwrangler.jsonc— v11 migration addingFlueReviewValidatorAgentvite.config.ts— eval mock redirect extended toreview-validator.tsevals/mocks/github-repo-tools.ts— addedmakeSearchRepoToolmock + validator eval fixturesAGENTS.md— updated agent table, pipeline description, request flow, models, migrations, eval coverageValidation
tsc --noEmit— passvitest run— 161/161 pass (10 test files)vite build— passwrangler deploy --dry-run— pass,FlueReviewValidatorAgentDO binding confirmedScope
Only affects
ReviewOrchestrator(the generic MR review pipeline).IngestWorkflow(spam/off-topic),DependabotReviewWorkflow, andRebaseWorkfloware untouched.