Skip to content

Close the test gaps declined in review rounds - #99

Merged
polewskm merged 10 commits into
mainfrom
issue-96
Oct 31, 2026
Merged

polewskm merged 10 commits into
mainfrom
issue-96

Conversation

@polewskm

@polewskm polewskm commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Adds the tests declined past the review round limit on #64, #68, #69 and #71. A new test/review-prompts.test.ts checks, through review() with a stub agent, that each part's reviewers get that part's diff with the other part's files marked, that each reviewer call gets its part's rules and its own charter, and that project reviewers run on their configured tier.
test/fallback.test.ts generalizes its helper into reviewOnFakeSdk() and tests the Bedrock rerun of the selection and context calls, plus withoutTools directly, through the fake Agent SDK.
Only test files and CHANGELOG.md change; the judge-spend (#73) and answer() (#75) tests need a non-test refactor first and are left as follow-ups.

Closes #96

@polewskm
polewskm deployed to code-review October 31, 2026 20:10 — with GitHub Actions Active

@ncode-review ncode-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 NCode Review at db53af5: 4 new findings on the changed lines.

Comment thread CHANGELOG.md Resolved
Comment thread CHANGELOG.md Resolved
Comment thread test/review-prompts.test.ts Resolved
Comment thread test/review-prompts.test.ts Resolved
@ncode-review

ncode-review Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Superseded by the review at 738aa16.

@polewskm

Copy link
Copy Markdown
Contributor Author

/review

@polewskm

Copy link
Copy Markdown
Contributor Author

Autopilot: ready to merge as far as this run can tell. One review round: the CodeQL false positive and two nits were fixed (f034e7d, ee8a6d7), and the re-review at ee8a6d7 raised only a repeat of the changelog nit already answered. No blocker or should-fix is open. Follow-ups filed after the merge: the judge-spend test (needs the sum moved out of main() in evals/run.ts) and the answer() test (needs it moved out of src/main.ts). It can merge by rebase or squash; nothing on the branch needs a merge commit.

@polewskm

Copy link
Copy Markdown
Contributor Author

/review

@ncode-review

ncode-review Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

🤖 Review complete

Verdict: ✅ Approve — Nothing in this pull request's own changes differs from the last review at ee8a6d7, so nothing was reviewed again. No further findings above nits.

0 reviewers run — 0 findings (run details)

Reviewer Model Findings Top

Trial: findings are posted as review threads and in the check run. $0.00 on Amazon Bedrock, 4s, at 738aa16. (how to interact)

📋 Outcomes

Merged at 738aa16: 1 fixed, 3 dismissed, 0 acknowledged, 0 ignored.

Finding Reviewer Severity Outcome
F1 hygiene 🟡 Nit 👎 dismissed
F2 conventions 🟡 Nit 👎 dismissed
F3 hygiene 🟡 Nit ✅ fixed
F4 rule-check 🟡 Nit 👎 dismissed

@polewskm
polewskm marked this pull request as ready for review October 31, 2026 20:58
@polewskm
polewskm deployed to code-review October 31, 2026 20:58 — with GitHub Actions Active
@polewskm
polewskm merged commit 7c96a9c into main Oct 31, 2026
7 checks passed
@polewskm
polewskm deleted the issue-96 branch October 31, 2026 20:58
@polewskm
polewskm deployed to code-review October 31, 2026 20:58 — with GitHub Actions Active

This branch was successfully deployed

1 active deployment
code-review — 738aa163 Deployed Oct 31, 2026 by polewskm via review #297
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.

Test gaps declined during review rounds

1 participant