feat: grammar-constrained AI output + sweep octokit.issues.X out of app.js#30
Merged
feat: grammar-constrained AI output + sweep octokit.issues.X out of app.js#30
Conversation
## Why
Today's live test revealed the small model's failure mode: it can produce
valid JSON of the *wrong shape*. PR #222 got `{"commit_message":"Add
pre-commit configuration files"}` instead of `{verdict, summary,
findings}`. Strict parser correctly rejected; bot fell to silence.
The strict prompt teaches the model the *intent*. The schema enforces the
*shape*. Belt-and-suspenders.
## What
New exports in `src/ai-review-prompt.js`:
- `REVIEW_JSON_SCHEMA` — full JSON Schema for the review payload, with
`verdict` enum, required fields, and `additionalProperties: false`.
- `buildResponseFormat()` — wraps the schema in OpenAI-compat
`response_format: { type: 'json_schema', json_schema: { name, strict,
schema } }`. Ollama (≥0.5) honours this on `/v1/chat/completions` for
grammar-constrained generation — the decoder physically cannot emit
tokens that violate the schema.
`src/ai-review.js`:
- `callLocalAI` accepts a new `responseFormat` option. When set, it goes
on the wire as `response_format`. Older Ollama versions ignore unknown
fields, so the bot degrades gracefully to strict-prompt + parser path.
- Strict mode passes `buildResponseFormat()` automatically. Legacy
freeform path (when `system_prompt` is overridden) does not — that path
expects prose.
## Why this is a bigger deal than it looks
The Netcup VPS is RAM-bound: 3.8 GB total, no swap. Anything bigger than
qwen2.5-coder:3b at Q4 risks OOM on inference. So we cannot just upgrade
the model. Constrained decoding lets the existing 3 B model succeed on
the shape problem the prompt couldn't fix on its own.
## Test plan
- [x] All 773 tests pass (was 766 — added 7 covering schema shape, response
format builder, wire-payload pass-through, legacy-mode omission)
- [x] eslint clean
- [ ] After deploy: trigger `/review-pr` on a recent PR. Wire-level: the
Ollama request body should contain `response_format` field. Behaviour:
no more `"AI review JSON parse failed: invalid verdict: undefined"`
log lines (the previous failure was wrong-shape JSON; this kills it).
## Risk & rollout
- Risk: low. New field is opt-in via the `responseFormat` argument; absent
→ wire payload unchanged from current behaviour. Old Ollama: silently
ignores; falls through to existing parser.
- Rollout: self-update on merge.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Why PR #29 deployed and removed the early-exit guard that silently swallowed chatops, but at app.js:644 the very next line was: const workingComment = await context.octokit.issues.createComment({...}); Logs from netcup at 04:39 UTC after retriggering /review-pr on rivet#213: err: TypeError: Cannot read properties of undefined (reading 'createComment') at file:///opt/temper/src/app.js:644:59 at issue_comment processWebhook Same root cause as PR #22 (ai-review.js) — Probot v14's `context.octokit.issues` is undefined for issue_comment.created webhooks too. The PR #22 fix only covered ai-review.js; app.js had 28 more `.issues.X` call sites that all throw the moment they're hit. ## What - New `issueOps` helper at the top of app.js wraps the four methods that app.js actually uses (`createComment`, `updateComment`, `deleteComment`, `create`/`createIssue`) with `octokit.request(<route>, args)` calls. - `replace_all` sweep replaced all 28 call sites: `context.octokit.issues.X(args)` → `issueOps.X(context.octokit, args)` - Updated `createMockOctokit()` to dispatch `.request(route, params)` into the same namespaced jest.fn()s as before, so existing test assertions on `octokit.issues.createComment` etc. keep working unchanged. - Three /generate-dependabot tests previously used `request.mockResolvedValue({...})` which clobbered the dispatcher; changed to `mockImplementationOnce` so dispatch survives. - E2e webhook test: removing the guard caused the issue_comment.created handler to actually run; with fake test credentials it hits Probot's installation token fetch and returns 401. Added 401 to the accepted status list (alongside the existing 200/202/500). ## Test plan - [x] All 773 tests pass (was 766; net +7 from PR #30's schema work) - [x] eslint clean - [ ] After deploy: `/review-pr` on rivet#213 should now post the "🔍 AI review in progress..." comment within seconds. Then either finish with a real review (oracle + model) or update with an error message — but no longer silently die. - [ ] All other chatops (`/configure-repo`, `/sync-all-repos`, `/check-config`, etc.) keep working. ## Risk & rollout - Risk: medium. Sweeping mechanical change across app.js. Mitigated by: - the 28 sites are all the same shape (object arg → object arg) - the `request()` form is what every other callsite in the codebase already uses successfully - 773 tests including the previously-broken e2e - Rollout: self-update on merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
avrabe
changed the title
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.
Two fixes in one PR. Both unblock the AI-review pipeline.
Fix 1 — Sweep `octokit.issues.X` out of app.js (the 401-on-/review-pr bug)
PR #29 removed an early-exit guard. After deploy, my retry of `/review-pr` on rivet#213 surfaced the next layer:
```
err: TypeError: Cannot read properties of undefined (reading 'createComment')
at file:///opt/temper/src/app.js:644:59
```
Same root cause as PR #22 — Probot v14's `context.octokit.issues` namespace is unreliable. PR #22 only fixed it in `ai-review.js`; app.js had 28 more call sites all throwing the moment they hit.
Fix 2 — Grammar-constrained model output (original scope)
Test plan
Risk & rollout
🤖 Generated with Claude Code