feat(review): generate unit tests for changed code on request - #461
Merged
Conversation
The bot reviews code but cannot propose tests for it, which is the natural way to close a coverage gap surfaced during review. /generate-tests asks the model for unit tests covering the changed code and posts them through the existing suggestion-formatting path, write-gated and refused while paused. Refs #36
Audit follow-ups on the /generate-tests command. Every field rendered into the suggested-tests comment is model output, so a prompt-injected diff must not be able to restructure that comment. The code fence was already widened past any backtick run in the test source, but the target path and the "covers" note were interpolated as-is: a path carrying a backtick could close the heading's code span, and a multi-line note could open a block of its own. Both are now flattened to a single line, and the path has its backticks removed. The quoted-input assertions for the command needed strengthening. The inline code case only held because the slash pattern requires a whitespace boundary that a bare `/generate-tests` span does not provide — it passed with inline-code stripping disabled, so it was not testing the stripping at all. Added a padded span and the mention form, which do depend on it, plus the converse case: a genuine invocation still fires alongside a quoted one, and a higher-precedence command that appears only inside a quote does not win. Also closed the remaining branch gap on the postable-proposal filter (a null path or a missing code field, both of which a model can emit) and listed the new command on the shared partial-coverage disclosure and the wrapped .env.example entry. Refs #36
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
🤖 ThrillhouseBot PR SummaryWhat this PR doesAdds an on-request /generate-tests command that asks the model to propose unit tests for the code a PR changed, posted as a comment with copyable code blocks, with safety gates and robust rendering against prompt injection. Control-Flow Diagram🔀 Show diagramflowchart TD
A["User comments /generate-tests"] --> B["WebhookController receives issue_comment"]
B --> C["TriggerDetector.detectCommand()"]
C --> D{"Command == GENERATE_TESTS?"}
D -->|Yes| E["CommentCommandService.execute()"]
E --> F["handleGenerateTests()"]
F --> G{"generateTestsEnabled?"}
G -->|No| H["Log and return"]
G -->|Yes| I{"Authorized?"}
I -->|No| J["Log unauthorized, return"]
I -->|Yes| K{"PR paused?"}
K -->|Yes| L["Post PAUSED_NOTICE"]
K -->|No| M["UnitTestGenerator.generate()"]
M --> N["loadInputs()"]
N --> O{"Diff available?"}
O -->|No| P["Return null"]
O -->|Yes| Q["Soft-load project stack"]
Q --> R["Call assistant with fenced diff"]
R --> S["Parse JSON response"]
S --> T{"Parsing OK?"}
T -->|No| U["Log warn, return null"]
T -->|Yes| V["Render comment with up to 5 tests"]
V --> W["Append truncation disclosure"]
W --> X["Return comment body"]
M --> Y{"Result null?"}
Y -->|Yes| Z["No comment posted"]
Y -->|No| AA["Post comment via GitHub API"]
Changes Overview
Changed Files
…and 4 more file(s). Risk Assessment
Key Findings
|
| Check | Type | Status | Detail |
|---|---|---|---|
| frontend | check-run | ⏳ Pending | - |
| changes | check-run | ⏳ Pending | - |
| trivy | check-run | ⏳ Pending | - |
| format | check-run | ⏳ Pending | - |
| actionlint | check-run | ⏳ Pending | - |
| test | check-run | ⏳ Pending | - |
| build | check-run | ⏳ Pending | - |
| dependency-review | check-run | ⏳ Pending | - |
Automated review by ThrillhouseBot. Reply with /review to re-run.
![thrillhousebot[bot]](https://avatars.githubusercontent.com/in/3996985?s=60&v=4){kind=small}
Comment on lines
+160
to
+162
| private static String notesLine(String notes) { | ||
| return notes.isBlank() ? "" : "\n**Not covered:** " + notes.strip() + "\n"; | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM — Model-supplied notes not flattened to a single line, weakening comment injection hardening
The project hardens the path and covers fields by flattening them to a single line (via SuggestionFormatter.oneLine()) to prevent model output from injecting markdown that restructures the comment. The notes field, however, is rendered through notesLine() which only calls notes.strip(), preserving internal newlines. A malicious or accidental model response containing newlines and markdown (e.g., a fenced code block) could break out of the intended comment structure. The PR description explicitly states that "the rendering is hardened against a prompt-injected diff," so this omission weakens that defense. Flatten notes to a single line like path and covers to close the gap.
Suggested change
| private static String notesLine(String notes) { | |
| return notes.isBlank() ? "" : "\n**Not covered:** " + notes.strip() + "\n"; | |
| } | |
| private static String notesLine(String notes) { | |
| return notes.isBlank() ? "" : "\n**Not covered:** " + notes.strip().replaceAll("\\s+", " ") + "\n"; | |
| } |
thrillhousebot
Bot
added
enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Aug 8, 2026
/improve (#452) and /generate-tests both register a new comment command, so the enum, the ordered pattern map, the command switch, the /help table, the config key, and every README/.env/docs listing collided additively. Both sides are kept throughout, with /generate-tests ordered after /improve everywhere. AbstractPrSuggestionGenerator now resolves the reviewable file list before rendering the diff, so UnitTestGeneratorTest stubs the two-argument ReviewDiffFormatter.buildDiffStringWithStats(files, reviewable) that loadInputs(...) calls. Refs #36
…tests /improve and /generate-tests are now adjacent entries in TriggerDetector's ordered pattern map, and a comment carrying more than one command resolves to the first entry that matches. Two properties are worth locking: Quoted context is stripped from the whole body before any pattern runs, so a quoted /improve cannot divert a genuine /generate-tests (or the reverse) no matter what the map order is. Merely quoting a command must never spend the operator's AI budget or post generated content. Map order therefore only decides a genuine-vs-genuine contest, which is pinned so a reorder cannot silently re-route an invocation to the other command. Refs #36
…sts comment The "notes" field of a /generate-tests reply is model output spliced straight into the comment body, but unlike the proposed path and the "covers" note it was rendered with only strip(), keeping its internal newlines. A reply whose notes carried a blank line and a fence closed the **Not covered:** line and rendered everything after it as live markdown — a heading, a fenced block, or anything else — inside a comment the bot posts under its own identity. Route it through the same SuggestionFormatter.oneLine(...) rule the path and covers note already use, rather than restating the regex, so the three model-supplied prose fields cannot drift apart. Refs #36
|
devops-thiago
added a commit
that referenced
this pull request
Aug 8, 2026
…ared-command-batching #461 merged first, so /generate-tests landed on base still using the line cap and reading Inputs.omittedFiles, which this branch removes. The migration its PR body deferred to #461 therefore has to happen here: without it base does not compile. Resolves README.md, where both sides rewrote the max-diff-lines row and the large-diff limitation to list which commands batch. Both now list /generate-tests among the batched ones.
devops-thiago
added a commit
that referenced
this pull request
Aug 9, 2026
…463) ## What type of PR is this? - [ ] 🐛 Bug fix - [x] ✨ Feature - [x] 📝 Documentation - [x] 🔧 Refactor - [ ] 🚀 Performance - [ ] ✅ Test - [ ] 🔒 Security - [ ] 📦 Dependency update - [ ] 🏗️ CI/CD ## Description `max-diff-lines` predates token budgeting and had become a second, cruder ceiling sitting in front of it. The review path stopped using it when map-reduce (#53) landed, and `/improve` was moved onto that in #316 — but `/describe` and `/changelog` still shrank a large PR to the first `max-diff-lines` of its rendered diff. A description was therefore written from a partial diff, and a CHANGELOG entry drafted from one, with whole files never reaching the model at all. **The seam.** The three methods `/improve` proved — batch planning, the shared prompt overhead, and the per-repo ignore re-filter — are lifted into `AbstractPrSuggestionGenerator`, parameterised by each command's own prompt constants. `/improve` is refactored onto the lifted versions rather than keeping a private copy. Every on-request suggestion command now plans batches over the reviewable **file list** under the per-call token budget and makes one call per batch. One call: ```java var plan = planBatches(reviewable, inputs, ownSystemPrompt, ownUserPrompt, reservedCalls); ``` **The reduce step is per-command, because the reductions genuinely differ.** Batching is only the map step: | Command | Reduce | Extra AI call? | |---|---|---| | `/describe` | Per-batch partial descriptions **synthesized** into one coherent title + description | Yes — reserved, spent only when >1 batch | | `/changelog` | Per-batch candidate entries **merged** into one entry | Yes — reserved, spent only when >1 candidate | | `/improve` | Local union of per-batch suggestions, deduped by `file:line` | No | | `/generate-tests` | Local union of per-batch test files, deduped by path | No | Nothing is concatenated. Stapling `/describe`'s partials together repeats the overview once per part and reads as several pull requests; for `/changelog`, a deterministic merge could unify headings and drop identical bullets, but the duplicates that actually arise are *not* identical — two batches that saw different files of one feature describe that change in two different sentences, which only a reader that understands them can collapse. Both reduce calls are reserved out of `max-ai-calls` up front, the same way the review path reserves one for its summary, so a run never exceeds the ceiling of one review; a single-batch PR still costs exactly one call. **Also in this change** - The shared overhead is assembled from each command's **own** prompts. Sizing a batch against another command's prompts would let an "in-budget" batch overshoot the real input limit. - Coverage disclosure now comes from `BudgetPlan.omittedFiles()` / `clippedFiles()` — files **named**, not counted — keeping #296's wording. `Inputs.omittedFiles` (the line-cap count) is gone, so nothing can reach for the wrong number. - Coverage on a huge PR is bounded by `max-ai-calls`, not the file list. Files that never got a batch are named. When *no* file fit any batch at all, every command says so and names the files rather than going silent — a misconfigured budget must not look like a bot that ignored the command. An empty plan that omitted *nothing*, because the repository ignores every changed file, is the opposite case and posts nothing. - Per-repo ignore patterns (#449) are applied on top of the global set for every command, and the filtered list stays authoritative for everything downstream — batches *and* the line map alike. That is the bug #452's audit found; the lifted method carries the invariant in its javadoc. - `max-input-tokens <= 0` keeps budgeting off as a single uncapped batch rather than regressing to the line-capped string. - A batch whose model call fails is skipped rather than failing the run, and the shortfall is disclosed. - Sizing callers reach the prompts through `systemPrompt()` / `userPrompt()` accessors: a reference to a `static final String` is inlined into the caller's class file at compile time, and a third copy of a multi-kilobyte prompt trips SpotBugs' `HSC_HUGE_SHARED_STRING_CONSTANT`. The accessors are deliberately *not* named `system()` / `user()` — differing from the constant only by capitalization reads as a typo at the call site. **`/add-docs` is deliberately out of scope.** It does not extend `AbstractPrSuggestionGenerator`; it orders its loading around a hard head-SHA precondition (every output is an inline suggestion, so no head SHA means nothing postable and a distinct user-facing message), and it feeds its assistant a different input set — project stack, a combined `PromptSections.prContext(...)` block, and a pre-rendered instructions section built from `ResolvedInstructions` rather than the content string the shared `Inputs` carries. Folding it in therefore means changing the shared `Inputs` contract at the same time as first lifting the seam, on the command that posts committable edits. It is worth doing and should be tracked separately; `/add-docs` remains line-capped and the README now says so precisely. Note it also still has **no per-repo ignore filter at all**, which is worth carrying into that follow-up. **Path-scoped instructions (#460) are not part of what a command batch carries.** `PathScopedInstructions` is resolved only by `ReviewContextLoader` and rendered only into `ReviewPromptAssembler`'s trailing-guidance slot, so it reaches the review prompt and nothing else. The `repoInstructions` slot of the batched commands is fed solely by `InstructionsResolver.resolve(...).content()` — the global instructions file — which `sharedPromptOverhead(...)` already counts in full. No batch is mis-sized by the scoped rules. ## `/generate-tests` is migrated onto the seam in this PR #461 merged before this one, so `/generate-tests` landed on `release/v0.6.0` still line-capped and still reading `Inputs.omittedFiles`, which this PR removes. The migration #461's agent was going to perform *after* this merged is therefore done **here** — there is no "later", because without it base does not compile. `UnitTestGenerator` now: 1. resolves its effective file list once via `respectPerRepoIgnores(...)` and plans from it, 2. plans token-budgeted batches and sends `batch.text()` per batch, 3. discloses coverage from `disclosure(plan)`, 4. reserves **0** calls — its reduce is a local union, so the whole `max-ai-calls` allowance buys batches. **Two things this surfaced that are worth reading closely.** **The shared overhead was not sufficient for this command, and using it unchanged would have been a real bug.** `sharedPromptOverhead(...)` counts system + user + fence + title + body + instructions. `/generate-tests` also sends the resolved **project stack** on every call — dependency manifests, kilobytes, not a rounding error — so the estimate would have undercounted every batch by the size of the stack and let "in-budget" batches overshoot the model's real input limit. That is precisely the failure the overhead exists to prevent. `planBatches(...)` therefore gains a six-argument form taking the command's own extra per-call sections, and `/generate-tests` declares the stack there. A future adopter with its own extra section must do the same rather than reach for the five-argument form. **It uses its own prompt templates, not the shared ones.** `UnitTestAssistant` is annotated with `UnitTestAssistantPrompts.SYSTEM` / `UnitTestAssistantPrompts.USER` — it does **not** share `PrSuggestionPrompts.USER`, because its user template carries the project-stack section. Sizing its batches against the shared user template would measure the wrong prompt. `UnitTestAssistantPrompts` had no accessors (it is new from #461), so `systemPrompt()` / `userPrompt()` are added to it — SpotBugs failed the build without them, exactly as the accessor javadoc predicts. **Why dedupe by path rather than merge.** Batches partition the file list, so two batches usually propose disjoint test paths. When they do collide, each proposal's `code` is a *complete* compilable file — package, imports and fixtures included, posted verbatim to paste at that path — so two of them at one path are alternatives, not additions. Rendering both would invite pasting the second over the first and silently losing the first's cases, and merging them properly would need a model call for a rare collision. The first wins and the rest are counted in a disclosure line, so the maintainer can re-run for the others. ## Related Issues Fixes #457 ## How Has This Been Tested? - [x] Unit tests - [ ] Integration tests - [ ] Manual testing Every new behavior was validated red/green: the test was written, the production change was mutated to neutralize exactly that behavior, the test was confirmed to **fail**, and the mutation was reverted to confirm it passes. **One mutation initially stayed green and the test was rewritten.** `proposesTestsForFilesThatTheLineCapWouldHaveDroppedEntirely` first asserted only that each batch *contained* its file — which is also true when every call is handed the whole-PR diff, the very behavior being replaced. Strengthened to assert the partition (batch 1 contains `Foo` and **not** `Other`, batch 2 the reverse), it goes red properly: ``` [ERROR] UnitTestGeneratorTest.proposesTestsForFilesThatTheLineCapWouldHaveDroppedEntirely:439 [[THRILLHOUSEBOT-UNTRUSTED-DATA-3d24804df8d0a8a72a18bb0d9f6a121f]] ``` **The project stack is counted in the budget.** Mutation: use the five-argument `planBatches(...)`, leaving the stack out of the overhead. With a 20k-character stack no file can honestly fit, so the correct run makes no call at all; the mutant ships batches that overshoot: ``` [ERROR] UnitTestGeneratorTest.countsTheProjectStackInTheBudgetSoBatchesAreNotOversized:496 No interactions wanted here: ``` **Per-repo ignores stay authoritative.** Mutation: plan from `inputs.reviewableFiles()` instead of the filtered list. ``` [ERROR] UnitTestGeneratorTest.leavesFilesTheRepositoryAskedTheBotToIgnoreOutOfScope:520 [[THRILLHOUSEBOT-UNTRUSTED-DATA-5c207a4fc3e69a526faf47a8f7df5769]] [ERROR] UnitTestGeneratorTest.staysSilentWhenEveryChangedFileIsOutOfScope:574 ``` **Same-path proposals are deduped.** Mutation: drop the `seenPaths` guard. ``` [ERROR] UnitTestGeneratorTest.keepsOneProposalPerPathAndSaysHowManyWereLeftOut:479 ## 🤖 ThrillhouseBot — suggested unit tests ``` **Disclosure comes from the budget plan.** Mutation: `disclosure(plan)` returns `""` and the empty-plan branch returns `null`. ``` [ERROR] UnitTestGeneratorTest.disclosesPartialCoverageEvenWhenNoTestsWereProposed:321 🧪 ThrillhouseBot found nothing in this PR's changes that warrants a new unit test. ==> expected: <true> but was: <false> [ERROR] UnitTestGeneratorTest.namesTheFilesLeftUncoveredWhenTheBatchBudgetRunsOut:303 ## 🤖 ThrillhouseBot — suggested unit tests [ERROR] UnitTestGeneratorTest.namesTheFilesWhenTheBudgetCouldNotCoverASingleOne:555 expected: not <null> ``` Earlier rounds for `/describe`, `/changelog` and `/improve` (batch text vs. line-capped render, synthesis vs. concatenation, reserved reduce call, per-repo ignores, disclosure, budgeting-disabled, nothing-covered, merge declines) all went red as recorded before; the `<= 0` guard mutation remains the one that does not, because `max-input-tokens=0` reaches `Integer.MAX_VALUE` down the fall-through path and yields the same single batch. Build results, on the merge of `release/v0.6.0` at `ad36d22` (#458, #460, #464, #459, #461): ``` ./mvnw -B spotless:apply # clean ./mvnw -B clean compile spotbugs:check spotless:check # BUILD SUCCESS ./mvnw -B clean test # Tests run: 2226, Failures: 0, Errors: 0, Skipped: 0 cd website && npm run build # "All internal links are valid." ``` ## Checklist - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors ## Notes for anything still in flight `Inputs.omittedFiles` is gone. It carried how many files the `max-diff-lines` render dropped; once a command plans its own token-budgeted batches that number describes a render nothing sends to a model, so it is not merely redundant but wrong. Nothing on `release/v0.6.0` or in this tree still reads it. Any command extending `AbstractPrSuggestionGenerator` that is still in flight needs the same four steps `/generate-tests` just took: 1. `disclosure(plan)` rather than a line-cap count. 2. `planBatches(reviewable, inputs, <its own system prompt>, <its own user prompt>, reservedCalls)` — its **own** prompt constants, and the six-argument form if it repeats a section the shared overhead does not know about. Check the merged tree for whether the class exposes `systemPrompt()` / `userPrompt()` accessors or only constants; adding them is required if a sizing reference would inline a third copy. 3. `batch.text()` per batch, never `inputs.diff()`. 4. `respectPerRepoIgnores(target, COMMAND, inputs.reviewableFiles())`, with that same list used for anything that anchors onto the diff. ## Additional Notes **Operator-visible cost change.** `/describe` and `/changelog` on a PR that needs more than one batch now cost one more model call than the batches alone, reserved out of `REVIEW_MAX_AI_CALLS`, so the ceiling per run is unchanged. `/improve` and `/generate-tests` reserve nothing. Documented in the config table, the "AI call budget" section, the command prose, and the Known limitations bullet. **The ignore filter is authoritative for the line resolver, not just the planner.** `/improve` threads the resolved list into both `planBatches(...)` and `post(...)`, where the resolver is built as `new DiffLineResolver(diffFormatter().patchesByReviewableFiles(reviewable))` — never from `inputs.reviewableFiles()`. `PrImprovementServiceTest.neverCommitsASuggestionToAFileTheRepositoryAskedTheBotToIgnore` pins it. `/describe`, `/changelog` and `/generate-tests` build no line map (a proposed test file is a new file with no diff line to anchor to), so they cannot exercise it, but the rule is stated in `respectPerRepoIgnores(...)`'s javadoc for future adopters. **New prompts.** `PrDescribeAssistantPrompts.SYNTHESIS_SYSTEM`/`SYNTHESIS_USER` and `ChangelogAssistantPrompts.MERGE_SYSTEM`/`MERGE_USER`, with matching `synthesize(...)` / `merge(...)` methods on the assistants. Both user templates mirror `PrSuggestionPrompts.USER` — same context sections, same random-fence untrusted-data block — with the partials/candidates in place of the diff. `AiServicePromptRenderingTest` drives both through the real rendering pipeline and asserts every `@V` reaches the message. **No new config keys.**
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
1 participant
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.
What type of PR is this?
Description
Adds
/generate-tests, an on-request command that asks the model to propose unit testsfor the code the PR changed — a way to close coverage gaps surfaced during review without
leaving the PR.
How a proposal is presented. A generated test is normally a whole new file. GitHub's
committable
suggestionblock replaces an anchored line range on an inline review comment,so a new file has nothing to anchor to; forcing one in would produce a broken commit when
applied. Each proposed file is therefore rendered through
SuggestionFormatteras acopy-paste block headed by the exact repository path it belongs at — the same
"show the draft, don't commit it" shape
/add-docsalready falls back to when adeclaration can't be pinned to a hunk. Nothing is committed and no file is edited.
Everything in that comment is model output, so the rendering is hardened against a
prompt-injected diff: the fence is widened past the longest backtick run in the test
source, the language tag is dropped unless it looks like a language tag, and every
model-supplied prose field — the path, the "covers" note, and the trailing "not covered"
notes — is flattened to a single line through one shared rule (with the path's backticks
removed), so none of them can break out of the structure around it.
Gating and failure behaviour. The handler runs
ManualReviewAuthorizerthenPrPauseService, in the same order as the other on-request commands, behind theREVIEW_GENERATE_TESTS_ENABLEDflag. Every load fails soft: no diff, an assistant error,or an unparseable reply all degrade to posting nothing rather than a noisy error on the PR.
When the model judges nothing testable, the bot says so instead of staying silent — the
maintainer asked explicitly. At most 5 files are rendered per comment, with a line naming
how many were held back. When the diff was over budget the comment carries the shared
partial-coverage disclosure, on the "nothing to test" outcome too, so that verdict can
never read as a verdict on the whole PR.
Files:
review/UnitTestGenerator.java— loads the diff/PR context/instructions/project stack,calls the assistant, renders the comment. Extends
AbstractPrSuggestionGeneratoranddoes not touch its diff loading, so it inherits token-budgeted batching when that lands.
review/ai/UnitTestAssistant.java,UnitTestAssistantPrompts.java— the LangChain4jservice and its prompts; the diff, PR body, stack and repo instructions are escaped and
framed as untrusted data.
review/ai/UnitTestGenerationParser.java,UnitTestGenerationResponse.java— JSONparsing, null-entry tolerance, and the postable-proposal filter.
review/SuggestionFormatter.java—formatGeneratedTestFile(...)plus the fence,language-tag and single-line hardening.
webhook/CommentCommand.java,TriggerDetector.java,CommentCommandService.java—the new command, its slash and mention patterns, the handler and the
/helprow.config/ThrillhouseConfig.java,application.properties,.env.example,README.md,docs/ARCHITECTURE.md,website/src/content/docs/index.md— the flag and its docs.The flag defaults to
true, matchingREVIEW_ADD_DOCS_ENABLED: the command never runsautomatically, only when a write-access holder asks for it, so the flag is the operator's
kill switch for the AI budget rather than an opt-in.
Related Issues
Fixes #36
How Has This Been Tested?
./mvnw -B clean test spotless:check spotbugs:check— 2196 tests green,BugInstance size is 0, spotless clean. JaCoCo reports 100% line and branch coverageon all four new classes and on
SuggestionFormatter,TriggerDetectorandCommentCommandService.cd website && npm ci && npm run build(thedocs.ymlbuild job) — 66 pages,"All internal links are valid". The rendered
/commands/,/configuration/and indexpages all carry the new command and key.
Every new test was mutation-proven: the production code was neutralized one behaviour at
a time and the test had to fail. Verbatim failures below.
Command routing and quoted-input safety
patterns.put(CommentCommand.GENERATE_TESTS, ...)removedTriggerDetectorTest.shouldDetectEachSlashCommand:62 expected: <GENERATE_TESTS> but was: <NONE>andshouldDetectEachMentionCommand:76 expected: <GENERATE_TESTS> but was: <NONE>FENCED_CODEstripping disabledshouldNotDetectGenerateTestsInsideQuotedContext:161 expected: <NONE> but was: <GENERATE_TESTS>~~~dropped fromFENCED_CODEshouldNotDetectGenerateTestsInsideQuotedContext:172 expected: <NONE> but was: <GENERATE_TESTS>BLOCKQUOTE_LINEstripping disabledshouldNotDetectGenerateTestsInsideQuotedContext:170 expected: <NONE> but was: <GENERATE_TESTS>INLINE_CODEstripping disabledshouldNotDetectGenerateTestsInsideQuotedContext:166 expected: <NONE> but was: <GENERATE_TESTS>shouldStillDetectGenerateTestsAlongsideAQuotedOne:184 expected: <GENERATE_TESTS> but was: <NONE>GENERATE_TESTSexcluded from the webhook's command routingWebhookControllerTest.shouldRouteGenerateTestsCommandToCommandService:880 Wanted but not invoked: commentCommandService.handle(...) Actually, there were zero interactions with this mock.The inline-code assertion originally in this PR (
run `/generate-tests` to propose)survived the inline-code mutation: an unpadded span already fails the slash pattern's
whitespace boundary, so it never exercised the stripping. It was replaced with a padded
span and the mention form, both of which do depend on it — the row above is the failure
from the hardened version.
Gating
generateTestsEnabled()gate removedCommentCommandServiceTest.generateTestsIgnoredWhenDisabled:376 No interactions wanted here ... But found these interactions on mock 'authorizer'authorized(ctx)gate removedgenerateTestsIgnoredWhenUnauthorized:364 No interactions wanted here ... But found these interactions on mock 'testGenerator'prPauseService.isPaused(...)gate removedgenerateTestsPostsPausedNoticeWhenPaused:354 Wanted but not invoked: commentClient.createComment(...) Actually, there were zero interactions with this mock.case GENERATE_TESTS ->removed from the switchgenerateTestsPostsTheGeneratedSuggestion:333 Wanted but not invoked: commentClient.createComment(...)suggestion == nullguard removedgenerateTestsPostsNothingWhenGeneratorReturnsNull:344 ... But invoked here: ... CreateCommentRequest[body=null]/generate-testsrow removed fromHELP_TEXThelpListsTheGenerateTestsCommand:384 expected: <true> but was: <false>Generation flow
UnitTestGeneratorTest.rendersEachProposedTestFileAsACopyPasteBlock:113 ... ==> expected: <true> but was: <false>MAX_TEST_FILEScap removedcapsTheNumberOfRenderedTestFiles:180 ... expected: <true> but was: <false>(Foo5/Foo6 rendered)reportsThatNothingWarrantsATestInsteadOfStayingSilent:194 expected: <true> but was: <false>reportsThatNothingWarrantsATestInsteadOfStayingSilent:195 ... expected: <true> but was: <false>appendsPartialCoverageDisclosureWhenTheDiffWasTruncated:225 expected: <\n\n> ⚠️ **Large PR — partial coverage.** 48 file(s) were omitted ...> but was: <>anddisclosesPartialCoverageEvenWhenNoTestsWereProposed:243 expected: <true> but was: <false>appendsNoDisclosureWhenNothingWasOmitted:255 ... expected: <true> but was: <false>""instead ofnullreturnsNullWhenThereIsNoDiff:263 expected: <null> but was: <>""instead ofnullreturnsNullWhenTheResponseIsNotUsableJson:284 expected: <null> but was: <>returnsNullWhenTheAssistantThrows:274->generate:101 » Runtime model downstillGeneratesWhenPrDetailsFetchFails:329->generate:101 » Runtime 404stillGeneratesWhenTheProjectStackCannotBeResolved:316->generate:101 » Runtime github downfencesTheDiffAndPassesTheProjectStackToTheAssistant:302 expected: <true> but was: <false>fencesTheDiffAndPassesTheProjectStackToTheAssistant:305 expected: <pom.xml: junit> but was: <>{{projectStack}}removed from the user promptAiServicePromptRenderingTest.unitTestPromptIncludesEveryContextVariable:133 projectStack missing ==> expected: <true> but was: <false>@UserMessagemoved from the method to a parameterAiServiceUserMessagePlacementTest.unitTestAssistantPutsUserMessageOnTheMethod:51 UnitTestAssistant.generate must declare @UserMessage on the method so the template is rendered ==> expected: <true> but was: <false>Rendering and parsing
SuggestionFormatterTest.shouldFormatGeneratedTestFileAsAPathHeadedCodeBlock:187 ... expected: <true> but was: <false>shouldKeepAModelSuppliedPathInsideItsHeadingCodeSpan:225 ... expected: <true> but was: <false>(the injected## Injectedheading escaped the code span)coversnote not flattenedshouldFlattenAMultiLineCoversNote:234 ... expected: <true> but was: <false>coversline dropped entirelyshouldFormatGeneratedTestFileAsAPathHeadedCodeBlock:188 ... expected: <true> but was: <false>shouldTolerateAMissingPathCoversAndCode:214 ### \null` ... expected: but was: `shouldTolerateAMissingPathCoversAndCode:215 ... expected: <false> but was: <true>shouldWidenTheFencePastBacktickRunsInTheTestSource:198 ... expected: <true> but was: <false>andUnitTestGeneratorTest.widensTheFenceWhenTheTestSourceContainsAFencedBlock:137 ... expected: <true> but was: <false>shouldOmitAnUnusableLanguageTag:206 ... expected: <true> but was: <false>andUnitTestGeneratorTest.dropsAModelSuppliedLanguageThatIsNotALanguageTag:155 ... expected: <true> but was: <false>(the injected heading landed on the fence line)UnitTestGenerationParserTest.unwrapsAFencedJsonReply:50 » IllegalArgument Model response is not valid generate-tests JSONparsesTheProposedTestFiles:39 expected: <src/test/java/FooTest.java> but was: <class FooTest {}>notesnot normalized to""normalizesMissingTestsAndNotes:66 expected: <> but was: <null>dropsNullEntriesAndKeepsOnlyPostableProposals:73 » IllegalArgument Model response is not valid generate-tests JSONisPostable()always truedropsNullEntriesAndKeepsOnlyPostableProposals:82 expected: <1> but was: <3>isPostable()dropsNullEntriesAndKeepsOnlyPostableProposals:84 expected: <1> but was: <2>rejectsAnEmptyOrUnparseableReply:88 Unexpected exception type thrown, expected: <java.lang.IllegalArgumentException> but was: <java.lang.NullPointerException>notesnot flattened to one lineUnitTestGeneratorTest.flattensTheModelSuppliedNotesLine:212— see belowThe last row closes a gap found in review:
pathand thecoversnote were flattenedthrough
SuggestionFormatter.oneLine(...), butnoteswas rendered with onlystrip(),so a reply whose notes carried a blank line and a fence broke out of the
**Not covered:**line and rendered as live markdown. It now goes through the same
oneLine(...)rule ratherthan restating the regex, so the three model-supplied prose fields cannot drift apart. With
that flattening reverted, the test fails with the injected fence and heading rendering live:
Command precedence is also pinned now that
/improve(#452) is an adjacent entry inTriggerDetector's ordered pattern map. Quoted context is stripped from the whole bodybefore any pattern runs, so a quoted
/improvecannot divert a genuine/generate-tests(or the reverse) whatever the map order is; order only decides a genuine-vs-genuine
contest, and that is pinned so a reorder cannot silently re-route an invocation to the
other command's AI spend.
shouldNotLetAQuotedNeighborCommandStealARealOne:227 expected: <GENERATE_TESTS> but was: <IMPROVE>shouldNotLetAQuotedNeighborCommandStealARealOne:232 expected: <GENERATE_TESTS> but was: <IMPROVE>shouldNotLetAQuotedNeighborCommandStealARealOne:235 expected: <GENERATE_TESTS> but was: <IMPROVE>IMPROVE/GENERATE_TESTSmap order swappedshouldResolveACommentCarryingBothImproveAndGenerateTestsToTheFirstEntry:248 expected: <IMPROVE> but was: <GENERATE_TESTS>Checklist
Screenshots / Logs
Shape of the posted comment (one section per proposed file):
Additional Notes
AbstractPrSuggestionGeneratoris deliberately untouched —git diff origin/release/v0.6.0...HEAD -- .../AbstractPrSuggestionGenerator.javais empty. Thecommand loads its diff through the shared
loadInputs(...)/Inputspath as-is./improve) has since merged, andrelease/v0.6.0is merged into this branch inad07735. Both commands register a new comment command, so the enum, the ordered patternmap, the command switch, the
/helptable, the config key and every README/.env/docslisting collided additively; both sides are kept, with
/generate-testsordered after/improveeverywhere. No behaviour of this command changed in the merge; the onlyadjustment was to a test, because
loadInputs(...)now resolves the reviewable file listbefore rendering the diff, so
UnitTestGeneratorTeststubs the two-argumentReviewDiffFormatter.buildDiffStringWithStats(files, reviewable).inputs.omittedFiles()atUnitTestGenerator.java:133for its partial-coverage disclosure. feat(review): batch /describe and /changelog under the token budget #463 lifts the batchingseam into
AbstractPrSuggestionGeneratorand removes that field; once it merges thiscommand adopts the seam —
disclosure(plan),planBatches(...)with this command's ownprompts, per-batch text rather than the whole diff, and the per-repo-ignore-filtered file
list as the authoritative one downstream. That call site is a known, tracked follow-up
rather than an oversight;
PrDescriptionGeneratorandChangelogEntryGeneratorread thesame field and migrate with it.
length-capped against GitHub's 65,536-character limit. Five whole test files could in
principle exceed it; the request then fails soft (logged, nothing posted). Worth a
follow-up issue that caps all of the generated comments, rather than solving it for one
command here.