Skip to content

fix(#202): remove broken 001-clean-approve review eval case - #205

Merged
rh-hemartin merged 3 commits into
mainfrom
agent/202-remove-broken-review-eval
Jul 16, 2026
Merged

fix(#202): remove broken 001-clean-approve review eval case#205
rh-hemartin merged 3 commits into
mainfrom
agent/202-remove-broken-review-eval

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

The 001-clean-approve review eval case has been broken since PR #148 merged. The post-review script returns 422 errors from inline review comments citing invalid line numbers, which prevents the ready-for-merge label from being applied. This causes the required_labels judge to fail (pass_rate=0.0%), blocking functional-tests-complete and the entire merge queue.

Remove the case directory to unblock the merge queue. The review eval infrastructure (eval.yaml, hooks, judges, thresholds) is preserved. The test case should be reintroduced after the line number accuracy issue is fixed (see PR #197).


Closes #202

Post-script verification

  • Branch is not main/master (agent/202-remove-broken-review-eval)
  • Secret scan passed (gitleaks — 39a2485de2655110196cccd2fa0e4a414d7096d4..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 16, 2026 14:52
@rh-hemartin

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:00 PM UTC · Completed 3:09 PM UTC
Commit: 12bd957 · View workflow run →

@rh-hemartin

Copy link
Copy Markdown
Member

/fs-fix Add a .gitkeep to the folder: touch eval/review/cases/.gitkeep

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 3:03 PM UTC · Completed 3:08 PM UTC
Commit: 12bd957 · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Added a .gitkeep file to eval/review/cases/ as requested by the human instruction. The directory was missing because the previous commit removed the only test case (001-clean-approve) it contained. The .gitkeep ensures git tracks the empty directory for future eval cases.

Fixed (1):

  1. Add .gitkeep to eval/review/cases/ directory (eval/review/cases/.gitkeep): Created eval/review/cases/.gitkeep to preserve the empty directory in git after the 001-clean-approve case was removed. The directory did not exist on the branch, so it was created along with the .gitkeep file.

Tests: passed

Updated by fullsend fix agent

@fullsend-ai-review

Copy link
Copy Markdown

Review

Reason: stale-head

The review agent reviewed commit 0ffafa8b5d9e920c6e237c79e5c2bf4c8710765b but the PR HEAD is now 3101b9ae36f6ab39ff7e3ed95453d8fe58afb93b. This review was discarded to avoid approving unreviewed code.

@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

fullsend-ai-coder Bot and others added 3 commits July 16, 2026 17:15
The 001-clean-approve review eval case has been broken
since PR #148 merged. The post-review script returns 422
errors from inline review comments citing invalid line
numbers, which prevents the ready-for-merge label from
being applied. This causes the required_labels judge to
fail (pass_rate=0.0%), blocking functional-tests-complete
and the entire merge queue.

Remove the case directory to unblock the merge queue. The
review eval infrastructure (eval.yaml, hooks, judges,
thresholds) is preserved. The test case should be
reintroduced after the line number accuracy issue is
fixed (see PR #197).

Closes #202
The functional test runner now exits 0 with a warning when an agent's
cases directory is empty, instead of letting workspace.py error out.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
@rh-hemartin
rh-hemartin force-pushed the agent/202-remove-broken-review-eval branch from 45ca372 to 18a2cb7 Compare July 16, 2026 15:15

@ben-alkov ben-alkov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHIP IT

@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 16, 2026
@rh-hemartin

Copy link
Copy Markdown
Member

🚢

Merged via the queue into main with commit fbd32c8 Jul 16, 2026
16 checks passed
@rh-hemartin
rh-hemartin deleted the agent/202-remove-broken-review-eval branch July 16, 2026 15:21
@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #205 — Remove broken 001-clean-approve review eval case

Timeline

  1. 14:39Z — Human (rh-hemartin) filed issue #202: the 001-clean-approve review eval case has been broken since PR ci: parallel functional tests with dynamic matrix strategy #148 merged on 2026-07-13, blocking the merge queue.
  2. 14:40Z — Triage agent ran (run 29507579704), classified as critical CI-blocking bug, recommended deleting the case directory. Completed in ~3 min.
  3. 14:44Z — Code agent ran (run 29507921017), removed eval/review/cases/001-clean-approve/ in commit c4c86fc. Completed in ~8 min. PR fix(#202): remove broken 001-clean-approve review eval case #205 created.
  4. 14:59Z — Human requested /fs-review.
  5. 15:01Z — Human requested /fs-fix Add a .gitkeep to the folder — the code agent had removed the only case directory without preserving the empty parent directory.
  6. 15:08Z — Fix agent completed, added .gitkeep in commit b218c4e.
  7. 15:09Z — Review agent completed its analysis (7.5 min, 455s) but was discarded as stale-head: it reviewed commit 0ffafa8 but HEAD was now 3101b9a (fix agent had pushed). The review agent's analysis was substantive — it correctly identified that removing the case without handling empty directories would still fail.
  8. 15:09Z — Review bot re-dispatched /fs-review for current HEAD.
  9. ~15:15Z — Human pushed commit 18a2cb7 directly: added an early exit in run-functional.sh for empty case directories (lines 96–103).
  10. 15:16–15:19Z — Three humans approved the PR.
  11. 15:21Z — PR merged. The re-dispatched review never completed before merge.

What went well

  • Triage agent performed excellently: correctly identified severity as critical, traced root cause to PR ci: parallel functional tests with dynamic matrix strategy #148's merge-queue timing gap, and recommended the right fix approach in ~3 minutes.
  • Code agent produced a correct initial commit with well-formatted conventional commit message.
  • End-to-end speed: issue filed to PR merged in ~42 minutes for a merge-queue-blocking issue.
  • Review agent analysis quality was high — it identified the same empty-directory issue the human later fixed. Unfortunately this analysis was discarded due to stale-head.

What could go better

  1. Code agent missed two follow-up concerns: (a) preserving the empty cases/ directory with .gitkeep — required a human /fs-fix intervention; (b) handling empty case directories in run-functional.sh — required a human direct commit. The code-implementation skill already has guidance on assessing downstream consumer impact (step 7), but the agent didn't apply it to this deletion.
  2. Wasted review cycle: The review agent spent ~7.5 minutes on a review that was discarded due to stale-head. This is a known issue (Reduce wasted review runs from stale-head races fullsend#1331, Remove stale-head re-dispatch: redundant with concurrency group, causes self-triggering fullsend#2399). The review contained the exact insight the human later acted on.
  3. Humans merged without automated review: Three humans approved and merged before the re-dispatched review completed. The change was simple enough that this was reasonable, but it means zero automated review coverage on this PR.

Existing issues covering stale-head waste

The stale-head review waste pattern is already tracked by fullsend-ai/fullsend#1331 and fullsend-ai/fullsend#2399. This PR provides additional evidence: the discarded review contained a substantive finding (empty-directory handling gap) that the human independently identified and fixed. No new proposal needed.

Proposals filed

@fullsend-ai-retro

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 3:23 PM UTC · Completed 3:33 PM UTC
Commit: 18a2cb7 · View workflow run →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove review eval case 001-clean-approve until post-review 422 errors are resolved

3 participants