Conversation
📝 WalkthroughWalkthroughBumps devcontainer and tooling version pins (Claude Code extension, UV, pnpm, ty, Pulumi packages), removes a trailing comma in a devcontainer template, and adds linting/testing guidance to AGENTS.md. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 10-11: The Markdown headings in AGENTS.md (notably the "Testing"
heading and the two subsequent headings referenced in the review) are missing a
blank line after each, triggering MD022 warnings; edit AGENTS.md to insert a
single blank line directly after each of those heading lines so every heading is
followed by an empty line, then re-run markdownlint to confirm MD022 is
resolved.
- Around line 28-30: The guidance is contradictory: the example "cd frontend &&
pnpm test-unit" uses chaining while the rule below forbids chaining; update the
✅ example to a non-chained form (e.g., run two separate commands or show just
the single-tool invocation like "cd frontend" then "pnpm test-unit" in separate
lines or separate bullets) and rewrite the sentence "and causing unnecessary
permission prompts" to "and cause unnecessary permission prompts"; ensure the
rule explicitly states that the example must not use &&, ||, ;, or & unless
explicitly requested and keep the prohibition on backslash continuations.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8338ccf0-6aca-43e1-8819-fe9d68ef1b11
📒 Files selected for processing (5)
.devcontainer/devcontainer.json.devcontainer/install-ci-tooling.pyAGENTS.mdextensions/context.pytemplate/.devcontainer/devcontainer.json.jinja-base
| ## Testing | ||
| - Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types by default. |
There was a problem hiding this comment.
Fix markdown heading spacing to satisfy MD022 warnings.
The headings at Line 10, Line 18, and Line 25 need a blank line after each heading.
Proposed markdownlint-compliant spacing
## Testing
+
- Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types by default.
@@
### Python Testing
+
- When using `mocker.spy` on a class-level method (including inherited ones), the spy records the unbound call, so assertions need `ANY` as the first argument to match self: `spy.assert_called_once_with(ANY, expected_arg)`
@@
## Tooling
+
- Always use `uv run python` instead of `python3` or `python` when running Python commands.Also applies to: 18-19, 25-26
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 10 - 11, The Markdown headings in AGENTS.md (notably
the "Testing" heading and the two subsequent headings referenced in the review)
are missing a blank line after each, triggering MD022 warnings; edit AGENTS.md
to insert a single blank line directly after each of those heading lines so
every heading is followed by an empty line, then re-run markdownlint to confirm
MD022 is resolved.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
AGENTS.md (2)
29-29:⚠️ Potential issue | 🟡 MinorFix grammar in chaining rule sentence.
At Line 29, “and causing unnecessary permission prompts” should be “and cause unnecessary permission prompts.”
✏️ Proposed wording fix
-- When running terminal commands, execute exactly one command per tool call. Do not chain commands with &&, ||, ;, or & unless the user explicitly asks for it. Pipes (|) are allowed for output transformation (e.g., head, tail, grep). If two sequential commands are needed, run them in separate tool calls. Chained commands break the permission allow-list matcher and causing unnecessary permission prompts +- When running terminal commands, execute exactly one command per tool call. Do not chain commands with &&, ||, ;, or & unless the user explicitly asks for it. Pipes (|) are allowed for output transformation (e.g., head, tail, grep). If two sequential commands are needed, run them in separate tool calls. Chained commands break the permission allow-list matcher and cause unnecessary permission prompts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 29, Update the chaining rule sentence to correct grammar: replace "and causing unnecessary permission prompts" with "and cause unnecessary permission prompts" in the sentence that begins "When running terminal commands, execute exactly one command per tool call..." so the clause reads "...Do not chain commands with &&, ||, ;, or & unless the user explicitly asks for it. Pipes (|) are allowed for output transformation (e.g., head, tail, grep). If two sequential commands are needed, run them in separate tool calls. Chained commands break the permission allow-list matcher and cause unnecessary permission prompts."
10-10:⚠️ Potential issue | 🟡 MinorAdd blank lines below headings to satisfy MD022.
Line 10, Line 18, and Line 25 should each be followed by a blank line.
🧹 Proposed markdownlint-compliant spacing
## Testing + - Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types by default. @@ ### Python Testing + - When using `mocker.spy` on a class-level method (including inherited ones), the spy records the unbound call, so assertions need `ANY` as the first argument to match self: `spy.assert_called_once_with(ANY, expected_arg)` @@ ## Tooling + - Always use `uv run python` instead of `python3` or `python` when running Python commands.Also applies to: 18-18, 25-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 10, Add a blank line immediately after each Markdown heading to satisfy MD022: update AGENTS.md so every heading (e.g., the "## Testing" heading and the other level-2 headings in the file) is followed by a single empty line before the next paragraph or content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@AGENTS.md`:
- Line 29: Update the chaining rule sentence to correct grammar: replace "and
causing unnecessary permission prompts" with "and cause unnecessary permission
prompts" in the sentence that begins "When running terminal commands, execute
exactly one command per tool call..." so the clause reads "...Do not chain
commands with &&, ||, ;, or & unless the user explicitly asks for it. Pipes (|)
are allowed for output transformation (e.g., head, tail, grep). If two
sequential commands are needed, run them in separate tool calls. Chained
commands break the permission allow-list matcher and cause unnecessary
permission prompts."
- Line 10: Add a blank line immediately after each Markdown heading to satisfy
MD022: update AGENTS.md so every heading (e.g., the "## Testing" heading and the
other level-2 headings in the file) is followed by a single empty line before
the next paragraph or content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 791dc547-b62d-4043-8af9-ec51cf16b4fb
📒 Files selected for processing (1)
AGENTS.md
There was a problem hiding this comment.
♻️ Duplicate comments (1)
AGENTS.md (1)
10-10:⚠️ Potential issue | 🟡 MinorFix MD022 heading spacing (blank line after headings).
Line 10, Line 18, and Line 25 should each be followed by a blank line to satisfy markdownlint.
Proposed markdownlint-compliant fix
## Testing + - Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types by default. @@ ### Python Testing + - When using `mocker.spy` on a class-level method (including inherited ones), the spy records the unbound call, so assertions need `ANY` as the first argument to match self: `spy.assert_called_once_with(ANY, expected_arg)` @@ ## Tooling + - Always use `uv run python` instead of `python3` or `python` when running Python commands.Also applies to: 18-18, 25-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 10, Add a blank line after each Markdown heading in AGENTS.md to satisfy markdownlint MD022 (e.g., ensure there is an empty line immediately after the "Testing" heading and after the other headings noted in the review); update the file so every heading line is followed by a single blank line, then re-run the linter to confirm the MD022 warning is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@AGENTS.md`:
- Line 10: Add a blank line after each Markdown heading in AGENTS.md to satisfy
markdownlint MD022 (e.g., ensure there is an empty line immediately after the
"Testing" heading and after the other headings noted in the review); update the
file so every heading line is followed by a single blank line, then re-run the
linter to confirm the MD022 warning is resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 75511fff-e719-47c8-9947-1236214fd255
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.devcontainer/devcontainer.jsonAGENTS.mdextensions/context.pypyproject.toml
We can see if this helps any. I happened on this article today and we might want to start looking at hooks next |
I'm sure we're still only scratching the surface, hooks sound like a good next thing. but emprically, these instructions did help (well, the 'stop chaining commands' is still touch and go)---I used them in repos and got claude to be better, and now I'm adding them to the template |
Why is this change necessary?
Better instructions to agents after empirically observing them not doing these things by default
How does this change address the issue?
Adds to AGENTS.md
What side effects does this change have?
N/A
How is this change tested?
Downstream repos
Other
Bumped some misc versions
Summary by CodeRabbit
Chores
Documentation