LearningCircuit / LearningCircuit/Friendly-AI-Reviewer
Reviewer approves while deferring checks it identified, and states unverified control-flow claims as verified
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 10
- Forks
- 2
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 3
Description
Summary
On local-deep-research#5187 the reviewer posted ✅ Approved with recommendations while explicitly flagging a check it had not run, and separately asserted a control-flow claim that is wrong. Both are the same shape: a confident verdict resting on an unexecuted check.
1. Approved while deferring a check it identified itself
From the review body:
Inference (not verified): the removed
verify-changed-filesslugoutput appears to have had only one consumer (thebranch:field, updated here). If any other step referencedsteps.verify-changed-files.outputs.slug, it would now silently expand to empty and reintroduce shared branch names — worth a quick grep, though the diff and PR description strongly suggest it was single-purpose.
The reviewer correctly identified the exact command that would settle it, described the failure mode accurately, and then approved without running it. The grep is one command:
$ git grep -n "outputs\.slug" <base>
.github/workflows/update-npm-dependencies.yml:214: branch: ...${{ steps.verify-changed-files.outputs.slug }}
tests/ci/test_npm_update_matrix.py:227: "${{ steps.verify-changed-files.outputs.slug }}" in branch
$ git grep -n "outputs\.slug" <head>
(no output)
Two consumers, not "only one" — and zero dangling references on the head, so the conclusion held. But the count was stated wrong and the verification was pushed onto the human reviewer inside an approval.
2. A control-flow claim asserted as verified, but incorrect
A top-level dir literally named
root(slugroot) now collides loudly with.instead of silently sharing a branch — the guard covers this case too. Nice.
This credits the new slug-collision guard. It isn't reached. A directory named root is neither . nor under tests/, so build_cmd and test_cmd are both empty and the pre-existing unknown-dir guard fires first, exiting before the collision guard runs. Confirmed by executing the discover script against a tree containing ./package-lock.json + ./root/package-lock.json:
::error::Unrecognized lockfile dir(s) — neither repo root nor under tests/. Classify build/test commands for:
- root
Right outcome (fail-loud), wrong mechanism — and the new guard adds nothing in this case, contrary to the review's claim.
3. What the review missed
It reported "🐛 Correctness — No bugs found". At that commit the discover pipeline still used sort -u, whose locale-collating behavior under uutils coreutils could drop a colliding directory before the new guard ever saw it, making the PR's headline feature unreachable on such hosts and failing the PR's own new test on a normal developer machine. (Filed separately as local-deep-research#6116; the PR has since replaced that pipeline entirely.)
Not a criticism of missing a subtle bug — but it does mean "No bugs found" was doing more work than the analysis behind it supported.
Suggested changes
- Run the check or withhold the verdict. When the reviewer can name the exact command that resolves an open question, it should execute it. If tool access doesn't permit that, the finding should downgrade the verdict (e.g. "Approved pending: run
git grep outputs.slug") rather than sit inside an approval as an aside. - Separate verified claims from inferred ones in the output format. The review labels one inference honestly but presents others (the
root-dir claim) in the same confident register as executed checks. A consistent marker for "traced/executed" vs "read from the diff" would make the difference legible. - Reserve "No bugs found" for the paths actually traced, and state which those were.
Reported from a multi-agent re-review of that PR; the reviewer's security analysis and its mutation-hardening observations held up well under checking — this is narrowly about verification discipline in the verdict.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
No project files or tests are named in this issue; start by locating the review-generation path and the output format used for verdicts and findings. Reproduce the cited review behavior, then trace how checks are represented and how control-flow claims are reported. Done means deferred checks and inferred claims are clearly distinguished, and unsupported approval or “No bugs found” wording is prevented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- ai, devtools
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100