fix: address post-merge review on operating manual and skills#30
Conversation
- Correct the ilmu client fallback in docs to match nudge_agent/service.py: settings.ilmu_model if settings.ilmu_api_key else "claude-sonnet-4-5" - Add the lint gate (npm run lint -w @bajetbuddy/web) to ship-pr pre-flight, matching the CI web job - Note that non-type shared exports import via @bajetbuddy/shared - Fix the Playwright snippet: async IIFE (top-level await + require cannot mix) and let Playwright resolve the browser from PLAYWRIGHT_BROWSERS_PATH, with executablePath only as the fallback https://claude.ai/code/session_017cLXjZu6kkjXRf2BemMPpp
✅ Deploy Preview for bajet-buddy canceled.
|
📝 WalkthroughWalkthroughUpdated repository guidance for Claude model selection, shared imports, web lint validation, and Playwright browser setup. ChangesDeveloper guidance
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
bajet-buddy | 4ae0d09 | Jul 16 2026, 04:49 PM |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates several skill guides and configuration files, including changing the fallback model to claude-sonnet-4-5, adding a linting step to the shipping workflow, and updating import guidelines. It also refactors the Playwright scratchpad script example. The review feedback suggests wrapping the Playwright operations in a try...finally block to ensure the browser is always closed properly, preventing orphaned processes in case of failures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| (async () => { | ||
| const browser = await chromium.launch(); | ||
| const page = await browser.newPage({ viewport: { width: 390, height: 844 } }); | ||
| // ... drive, screenshot, assert ... | ||
| await browser.close(); | ||
| })(); |
There was a problem hiding this comment.
When running Playwright scripts in a scratchpad, any failure during the driving or assertion phase will prevent browser.close() from being called, leaving orphaned browser processes running in the background. Wrapping the page operations in a try...finally block ensures the browser is always closed cleanly.
| (async () => { | |
| const browser = await chromium.launch(); | |
| const page = await browser.newPage({ viewport: { width: 390, height: 844 } }); | |
| // ... drive, screenshot, assert ... | |
| await browser.close(); | |
| })(); | |
| (async () => { | |
| const browser = await chromium.launch(); | |
| try { | |
| const page = await browser.newPage({ viewport: { width: 390, height: 844 } }); | |
| // ... drive, screenshot, assert ... | |
| } finally { | |
| await browser.close(); | |
| } | |
| })(); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/ship-pr/SKILL.md (1)
13-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the stale pre-flight gate count.
This section now runs four commands, but line 17 still says “all three.” Change it to “all four” or use neutral wording such as “all gates.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/ship-pr/SKILL.md around lines 13 - 17, Update the pre-flight gate statement in the ship-pr skill instructions to reflect the four commands now executed, replacing “all three” with “all four” or equivalent neutral wording such as “all gates.”
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/verifier-web/SKILL.md:
- Line 56: Remove the explanatory “drive, screenshot, assert” comment from the
code template in the verifier-web skill. Describe those placeholder actions in
surrounding prose outside the snippet, leaving the template free of
non-essential comments.
---
Outside diff comments:
In @.claude/skills/ship-pr/SKILL.md:
- Around line 13-17: Update the pre-flight gate statement in the ship-pr skill
instructions to reflect the four commands now executed, replacing “all three”
with “all four” or equivalent neutral wording such as “all gates.”
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b977bed4-68ce-4f63-ae94-edc2f474cfc2
📒 Files selected for processing (4)
.claude/skills/new-feature/SKILL.md.claude/skills/ship-pr/SKILL.md.claude/skills/verifier-web/SKILL.mdCLAUDE.md
- ship-pr: pre-flight gates say "all four" now that lint was added as a fourth gate - verifier-web: move the placeholder step description into prose above the snippet instead of an inline comment, per the no-comments rule
2 participants
Summary
Follow-up to #29, applying the valid findings from the cubic review that landed at merge time:
new-featureskill — the ilmu client snippet now matches the actual code inapps/api/app/nudge_agent/service.py(settings.ilmu_model if settings.ilmu_api_key else "claude-sonnet-4-5"), instead of a made-upclaude-opus-4-5fallback.ship-prpre-flight (npm run lint -w @bajetbuddy/web) — the CIwebjob runs eslint before the build, so skipping it locally wasted a CI round-trip per lint error.@bajetbuddy/shared, matching existing usage inconstants.tsandlib/sentinel/.verifier-web— top-levelawaitcan't coexist withrequire(), so the example is now an async IIFE, and it lets Playwright resolve the browser fromPLAYWRIGHT_BROWSERS_PATHwithexecutablePathonly as a documented fallback.Skipped: nothing — all six findings were valid.
Test plan
webandapiCI jobs should pass unchangedapps/api/app/nudge_agent/service.py:276https://claude.ai/code/session_017cLXjZu6kkjXRf2BemMPpp
Generated by Claude Code
Note
Low Risk
Documentation and skill text only; no application code, auth, or data paths change.
Overview
Docs-only follow-up that fixes agent/operator guidance in
CLAUDE.mdand three.claude/skills/*files so snippets match real repo behavior and local gates match CI.Ilmu/Anthropic model selection — Replaces the incorrect
settings.ilmu_model or "claude-opus-4-5"example withsettings.ilmu_model if settings.ilmu_api_key else "claude-sonnet-4-5"inCLAUDE.mdand thenew-featureskill, matchingnudge_agent/service.pyand other API call sites.Ship PR pre-flight — Adds
npm run lint -w @bajetbuddy/webto theship-prskill so eslint runs locally before push, aligned with the blockingwebCI job.Shared package imports — Clarifies in
new-featurethat types use@/typeswhile constants/engines use@bajetbuddy/shared, not relative paths intopackages/.Playwright verifier — Rewrites the
verifier-webexample as an async IIFE (avoids invalid top-levelawaitwithrequire) and documents default browser resolution viaPLAYWRIGHT_BROWSERS_PATH, with explicitexecutablePathonly as a fallback.Reviewed by Cursor Bugbot for commit 947b294. Configure here.
Summary by cubic
Fixes post-merge issues in the operating manual and skills. Aligns examples with the API, adds a local lint gate, and hardens the Playwright snippet with reliable browser cleanup.
apps/api/app/nudge_agent/service.py:settings.ilmu_model if settings.ilmu_api_key else "claude-sonnet-4-5".npm run lint -w @bajetbuddy/webtoship-prpre-flight and updated the gate count copy to “all four”.@/types; non-type exports via@bajetbuddy/shared.PLAYWRIGHT_BROWSERS_PATHauto-resolution withexecutablePathonly as a fallback; moved placeholder steps out of the code block into prose.Written for commit 4ae0d09. Summary will update on new commits.
Summary by CodeRabbit