feat(reviewer): internalize review skills and add stack-based injection#532
Merged
feat(reviewer): internalize review skills and add stack-based injection#532
Conversation
Move review skills from external beagle plugin into the Amelia repo, inject them into reviewer prompts server-side for driver-agnostic code review. Adds configurable review types and on-demand review API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8-task plan covering: skill file creation, stack detection, prompt updates, skill injection wiring, multi-review-type state, on-demand review API endpoint, and dashboard Request Review dialog. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy 23 technology-specific code review skill files from beagle plugins into amelia, organized by language (python, react, go, elixir, swift). Add general review guidelines and security-focused review checklist. Strip YAML frontmatter and fix broken references to external files. Include skill markdown files in wheel artifacts via pyproject.toml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implement detect_stack() to identify technology tags from file paths and import patterns, and load_skills() to resolve skill markdown files and concatenate them for injection into reviewer prompts. All 21 tests pass. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Update reviewer prompt to use {review_guidelines} placeholder instead of Skill tool instructions. Add review_guidelines parameter to Reviewer.__init__() for server-side skill injection. Update defaults.py prompt to match new format. All 1732 tests pass.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
anderskev
added
enhancement
Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughThis pull request refactors the reviewer agent to accept injected review guidelines via a new 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
tests/unit/agents/test_reviewer_prompts.py (1)
41-45: Cover the rendered prompt, not just the template token.Line 44 only asserts that the raw template contains
review_guidelines. BecauseReviewer.agentic_promptreturns the unformatted template (amelia/agents/reviewer.py:145-146), this still passes if runtime interpolation breaks. Add a test that renders the prompt with a non-empty guideline string and verifies the placeholder is actually replaced.Possible test shape
reviewer = create_reviewer() # No prompts injected - # Should contain agentic review markers - assert "git diff" in reviewer.agentic_prompt - assert "review_guidelines" in reviewer.agentic_prompt + rendered = reviewer.agentic_prompt.format( + base_commit="abc123", + review_guidelines="STACK-SPECIFIC RULES", + ) + assert "git diff" in rendered + assert "STACK-SPECIFIC RULES" in rendered + assert "{review_guidelines}" not in rendered🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/agents/test_reviewer_prompts.py` around lines 41 - 45, The test currently checks the raw template token but not rendering — change tests/unit/agents/test_reviewer_prompts.py to render the agentic prompt with a real guideline string and assert the placeholder is replaced: obtain the template from Reviewer.agentic_prompt (via create_reviewer()), format or render it by substituting a non-empty value for the "review_guidelines" placeholder (e.g., using .format or the reviewer-provided render utility) and assert the resulting string contains that guideline text and expected markers like "git diff"; this ensures Reviewer.agentic_prompt actually interpolates at runtime rather than only containing the template token.amelia/skills/review/python/postgres.md (1)
16-17: Reword line 16 to account for index type based on query pattern, not just the presence of querying.Line 16 overgeneralizes by prescribing GIN for all queried JSONB columns. In reality:
- GIN is correct for containment/existence queries (e.g.,
@>,?, JSONPath) that search within the document structure.- B-tree expression indexes (e.g.,
(jsonb_col->>'key')) are required for scalar-like patterns:
- Equality filters:
WHERE (data->>'key') = 'value'- Range/ordering:
WHERE (data->>'key') BETWEEN ...orORDER BY (data->>'key')Only B-tree can satisfy
ORDER BYoperations; GIN returns unspecified order. For frequently-accessed single keys, a focused B-tree index is also more efficient than indexing the entire document.Reword to guide reviewers toward "use the appropriate index for the operator/query shape" rather than defaulting to GIN.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/python/postgres.md` around lines 16 - 17, Update the checklist item that currently reads "JSONB columns use GIN indexes when queried" to instruct choosing the appropriate index type based on query shape (e.g., GIN for containment/existence queries using @>, ?, JSONPath; B-tree expression indexes for scalar-like access such as (jsonb_col->>'key') used in equality, range, or ORDER BY). Replace the overgeneralized GIN recommendation with wording like "Use the appropriate index for the query/operator (GIN for containment/existence; B-tree expression indexes for scalar equality/range/ordering on JSONB keys)" so reviewers are guided by operator/query pattern rather than always using GIN.amelia/skills/review/__init__.py (2)
19-19: Consider adding a general React skill file.The
"react"tag maps directly to"react/shadcn.md", which is a specific UI component library. If a project uses React without shadcn, the reviewer would get shadcn-specific guidance that may not apply.Consider either:
- Adding a general
react/react.mdskill file and mapping"react"to it- Keeping shadcn separate and mapping it only via the
"shadcn"tag🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/__init__.py` at line 19, The current skill mapping maps the "react" tag directly to the specific file "react/shadcn.md", which couples the general "react" tag to a shadcn-specific guide; update the mapping to be generic by creating a new file "react/react.md" with general React guidance and change the mapping for the "react" key to point to "react/react.md", and if you want to keep the shadcn content separate add a new "shadcn" tag that maps to "react/shadcn.md" instead so "react" no longer returns shadcn-specific guidance.
145-149: Missing skill files are silently skipped.If a skill file referenced in the registry doesn't exist,
load_skills()silently skips it (line 148). While this prevents crashes, it could hide configuration errors where a skill file path is misspelled or a file was accidentally deleted.Consider adding a warning log when a referenced skill file is not found, to aid debugging during development.
💡 Optional: Log warning for missing files
+from loguru import logger + # Read and concatenate for rel_path in sorted(seen_paths): full_path = _SKILLS_DIR / rel_path if full_path.exists(): sections.append(full_path.read_text(encoding="utf-8").strip()) + else: + logger.warning("Skill file not found", path=rel_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/__init__.py` around lines 145 - 149, The loop that reads skill files over seen_paths in load_skills silently skips missing files; add a warning log when full_path (constructed from _SKILLS_DIR and rel_path) does not exist so missing or misspelled skill paths are visible. Update the block that iterates over seen_paths to call the module logger (e.g., logging.getLogger(__name__) or the existing logger variable) and emit a warning including rel_path and full_path when full_path.exists() is False, leaving the existing append behavior unchanged for present files.amelia/skills/review/python/python.md (1)
16-17: Line length guideline differs from repo standard.This document specifies "Line length ≤79 characters" (standard PEP8), but this repository's coding guidelines specify 100 characters. This discrepancy is understandable if the skill is meant for general Python review across various projects, but could cause confusion when reviewing code in this specific repo.
Consider adding a note that line length limits may vary by project, or updating the value to match the repo's convention if this skill is primarily intended for internal use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/python/python.md` around lines 16 - 17, The checklist item "- [ ] Line length ≤79 characters" in python.md conflicts with this repo's 100‑char guideline; update the document to either (A) change that checklist entry to "- [ ] Line length ≤100 characters" to match the repository convention, or (B) append a short note next to the existing "- [ ] Line length ≤79 characters" entry stating "Note: project-specific limits may differ; follow repo convention (100 chars) when applicable" so reviewers know to prefer the repo standard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@amelia/skills/review/__init__.py`:
- Around line 112-115: The comment claims "only added lines" but the loop over
_IMPORT_PATTERNS currently searches the entire diff_content; update the
implementation to match the comment by first extracting only added lines (lines
beginning with '+') into a separate string (e.g., added_content) and then run
the existing loop against added_content, using the same _IMPORT_PATTERNS and
tags set; alternatively, if searching the whole diff is intended, simply change
the comment to accurately state that the full diff_content is scanned rather
than modifying behavior of _IMPORT_PATTERNS, ensuring you update the comment
near the loop that references diff_content and the tags variable.
In `@amelia/skills/review/elixir/phoenix.md`:
- Around line 44-49: The bullet claiming "`Plug.Conn.halt/1` without send" is
valid is incorrect and will cause Plug.Conn.NotSentError; update the entry
referencing Plug.Conn.halt/1 to state it is invalid unless a response has been
sent first, and show that a response must be sent via send_resp/3, redirect/2,
or similar before calling halt (e.g., "use conn |> send_resp(...) |> halt() or
conn |> redirect(...) |> halt()"); update any explanatory text and examples in
the same section to emphasize that halt/1 does not send a response and must be
paired with a prior send_resp/3 or redirect/2 call.
In `@amelia/skills/review/go/go-concurrency.md`:
- Around line 224-236: The goroutine in fetchWithTimeout uses http.Get which
ignores ctx and can hang; change the goroutine to create a context-aware request
with http.NewRequestWithContext using the provided ctx and perform it with an
http.Client (e.g., http.DefaultClient.Do), capture both resp and err, and send
the result back on ch (or send a wrapper/value indicating error) so the select
can still proceed when ctx is done; update fetchWithTimeout to handle the sent
error/response appropriately and avoid goroutine leaks.
- Around line 50-55: The worker currently calls
job.Execute(context.Background()) which ignores pool/server shutdown; change it
to pass a cancellable context tied to the WorkerPool instead. Locate the
WorkerPool.worker method and replace context.Background() with a context from
the pool (e.g., wp.ctx or a context derived from it like ctx := wp.ctx; or
per-job ctx, ctx, cancel := context.WithCancel(wp.ctx) / WithTimeout if needed)
and pass that ctx into job.Execute; if WorkerPool lacks a context field, add one
(e.g., ctx context.Context set on construction) so workers can respond to pool
shutdown/cancellation.
In `@amelia/skills/review/python/sqlalchemy.md`:
- Around line 29-32: The "When to Load References" section in sqlalchemy.md
references non-existent cross-reference files sessions.md, relationships.md,
queries.md, and migrations.md; fix this by either creating those four
documentation files in the same docs folder with the referenced content
(sessions, model relationships, database queries, Alembic migrations) or
remove/update the links in sqlalchemy.md to point to existing docs; ensure the
changes touch the references to sessions.md, relationships.md, queries.md, and
migrations.md so no broken links remain.
In `@amelia/skills/review/react/react-router.md`:
- Line 29: Update the guidance on line 29 to reframe the distinction: explain
that React Router loaders can run in the browser under SPA/data-router mode
(e.g., createBrowserRouter) and are not inherently server-only, and instruct
reviewers to prefer loaders for route data when appropriate but reserve
useEffect specifically for browser-only APIs (localStorage, window dimensions,
DOM/window access) that loaders cannot access; reference useEffect, loaders, and
createBrowserRouter in the revised sentence so it's clear when each approach is
appropriate.
In `@amelia/skills/review/swift/swiftui.md`:
- Around line 26-31: The "When to Load References" list in the SwiftUI skill
guide points to missing skill files (view-composition.md, state-management.md,
performance.md, accessibility.md); either create those four markdown skill files
with the appropriate content and links, or remove their entries from the list in
the "When to Load References" section so the guide no longer references
nonexistent files—update the section that contains the four bullet items so it
only links to existing skills or newly added files.
In `@docs/plans/2026-03-09-reviewer-skills-internalization.md`:
- Around line 349-360: The mapping in _EXTENSION_TAGS assigns ".ts" and ".js" to
the "typescript" tag but REVIEW_SKILLS has no "typescript" entry, so detected
TS/JS files load no skills; fix by either adding a "typescript" skill and
corresponding registry entry in REVIEW_SKILLS (e.g., a typescript.md skill and a
"typescript": <skill-list> entry), or change _EXTENSION_TAGS to map .ts/.tsx
and/or .js/.jsx to an existing tag like "react" (update _EXTENSION_TAGS
accordingly) so the file extensions point to a registered skill set; update
whichever of _EXTENSION_TAGS or REVIEW_SKILLS you choose to keep them
consistent.
- Around line 477-572: Summary: The design doc uses placeholder
{injected_skill_content} but the implementation and plan use
{review_guidelines}, so the names are inconsistent; pick one and make all
references match. Fix: Update the design documentation to replace every
occurrence of {injected_skill_content} with {review_guidelines} so it matches
the implementation, and verify the implementation symbols
(AGENTIC_REVIEW_PROMPT, Reviewer.__init__ parameter review_guidelines, and
agentic_review formatting call that passes
review_guidelines=self._review_guidelines) remain using {review_guidelines};
alternatively, if you prefer the design name, change those symbols and format
calls to use {injected_skill_content} consistently throughout. Ensure tests and
defaults prompt text also use the chosen placeholder name.
- Around line 13-168: The task omitted creating
amelia/skills/review/react/react.md which causes the registry to map the "react"
tag to shadcn.md; add creating amelia/skills/review/react/react.md to the Task 1
file creation list and include it in Step 2's mapping (source SKILL.md →
amelia/skills/review/react/react.md or create a new general React skill file),
then update the registry mapping referenced around line 320 so the "react" tag
points to react.md (amelia/skills/review/react/react.md) instead of shadcn.md.
---
Nitpick comments:
In `@amelia/skills/review/__init__.py`:
- Line 19: The current skill mapping maps the "react" tag directly to the
specific file "react/shadcn.md", which couples the general "react" tag to a
shadcn-specific guide; update the mapping to be generic by creating a new file
"react/react.md" with general React guidance and change the mapping for the
"react" key to point to "react/react.md", and if you want to keep the shadcn
content separate add a new "shadcn" tag that maps to "react/shadcn.md" instead
so "react" no longer returns shadcn-specific guidance.
- Around line 145-149: The loop that reads skill files over seen_paths in
load_skills silently skips missing files; add a warning log when full_path
(constructed from _SKILLS_DIR and rel_path) does not exist so missing or
misspelled skill paths are visible. Update the block that iterates over
seen_paths to call the module logger (e.g., logging.getLogger(__name__) or the
existing logger variable) and emit a warning including rel_path and full_path
when full_path.exists() is False, leaving the existing append behavior unchanged
for present files.
In `@amelia/skills/review/python/postgres.md`:
- Around line 16-17: Update the checklist item that currently reads "JSONB
columns use GIN indexes when queried" to instruct choosing the appropriate index
type based on query shape (e.g., GIN for containment/existence queries using @>,
?, JSONPath; B-tree expression indexes for scalar-like access such as
(jsonb_col->>'key') used in equality, range, or ORDER BY). Replace the
overgeneralized GIN recommendation with wording like "Use the appropriate index
for the query/operator (GIN for containment/existence; B-tree expression indexes
for scalar equality/range/ordering on JSONB keys)" so reviewers are guided by
operator/query pattern rather than always using GIN.
In `@amelia/skills/review/python/python.md`:
- Around line 16-17: The checklist item "- [ ] Line length ≤79 characters" in
python.md conflicts with this repo's 100‑char guideline; update the document to
either (A) change that checklist entry to "- [ ] Line length ≤100 characters" to
match the repository convention, or (B) append a short note next to the existing
"- [ ] Line length ≤79 characters" entry stating "Note: project-specific limits
may differ; follow repo convention (100 chars) when applicable" so reviewers
know to prefer the repo standard.
In `@tests/unit/agents/test_reviewer_prompts.py`:
- Around line 41-45: The test currently checks the raw template token but not
rendering — change tests/unit/agents/test_reviewer_prompts.py to render the
agentic prompt with a real guideline string and assert the placeholder is
replaced: obtain the template from Reviewer.agentic_prompt (via
create_reviewer()), format or render it by substituting a non-empty value for
the "review_guidelines" placeholder (e.g., using .format or the
reviewer-provided render utility) and assert the resulting string contains that
guideline text and expected markers like "git diff"; this ensures
Reviewer.agentic_prompt actually interpolates at runtime rather than only
containing the template token.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50715f95-09f7-44ac-9339-91210529690f
📒 Files selected for processing (35)
amelia/agents/prompts/defaults.pyamelia/agents/reviewer.pyamelia/skills/__init__.pyamelia/skills/review/__init__.pyamelia/skills/review/elixir/elixir-performance.mdamelia/skills/review/elixir/elixir-security.mdamelia/skills/review/elixir/elixir.mdamelia/skills/review/elixir/exunit.mdamelia/skills/review/elixir/liveview.mdamelia/skills/review/elixir/phoenix.mdamelia/skills/review/general.mdamelia/skills/review/go/go-concurrency.mdamelia/skills/review/go/go-middleware.mdamelia/skills/review/go/go-testing.mdamelia/skills/review/go/go.mdamelia/skills/review/python/fastapi.mdamelia/skills/review/python/postgres.mdamelia/skills/review/python/pytest.mdamelia/skills/review/python/python.mdamelia/skills/review/python/sqlalchemy.mdamelia/skills/review/react/react-flow.mdamelia/skills/review/react/react-router.mdamelia/skills/review/react/shadcn.mdamelia/skills/review/react/vitest.mdamelia/skills/review/security/general.mdamelia/skills/review/swift/combine.mdamelia/skills/review/swift/swift.mdamelia/skills/review/swift/swiftui.mdamelia/skills/review/verification.mddocs/plans/2026-03-09-reviewer-skills-internalization-design.mddocs/plans/2026-03-09-reviewer-skills-internalization.mdpyproject.tomltests/unit/agents/test_reviewer_prompts.pytests/unit/skills/__init__.pytests/unit/skills/test_review.py
…g refs - Fix misleading "only added lines" comment in detect_stack - Clarify halt/1 valid pattern requires action_fallback - Refactor worker pool to propagate cancellable context - Use http.NewRequestWithContext instead of http.Get in example - Correct React Router loader server-side claim for SPA mode - Remove dangling "When to Load References" in sqlalchemy and swiftui Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
amelia/skills/review/python/sqlalchemy.md (2)
5-11: Clarify or complete the Quick Reference table.The Quick Reference table includes a "Reference" column that is empty (all entries show "—"). Consider either:
- Removing the Reference column if it won't be populated
- Adding Markdown anchor links to relevant checklist sections below
- Adding external documentation URLs
This would improve the table's utility as a quick reference guide.
📋 Example with internal anchor links
| Issue Type | Reference | |------------|-----------| -| Session lifecycle, context managers, async sessions | — | -| relationship(), lazy loading, N+1, joinedload | — | -| select() vs query(), ORM overhead, bulk ops | — | -| Alembic patterns, reversible migrations, data migrations | — | +| Session lifecycle, context managers, async sessions | See checklist items 1-3 | +| relationship(), lazy loading, N+1, joinedload | See checklist items 4-6 | +| select() vs query(), ORM overhead, bulk ops | See checklist items 7-8 | +| Alembic patterns, reversible migrations, data migrations | See checklist items 9-12 |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/python/sqlalchemy.md` around lines 5 - 11, The Quick Reference table currently has an empty "Reference" column; update the table in the Quick Reference (the markdown table at the top listing "Session lifecycle..." etc.) by either removing the "Reference" column entirely or populating it with meaningful links: add internal markdown anchor links to the corresponding checklist sections (e.g., "#session-lifecycle", "#relationship-lazy-loading", "#select-vs-query", "#alembic-patterns") or insert external documentation URLs for each row; ensure headers/anchors match the actual section headings used later in the document and keep table formatting consistent.
29-32: Consider varying question phrasing for readability.All four questions begin with "Are," which creates a slightly repetitive pattern. While not incorrect, varying the sentence structure could improve flow.
✍️ Example with varied phrasing
## Review Questions -1. Are all sessions properly managed with context managers? -2. Are relationships configured to avoid N+1 queries? -3. Are queries using SQLAlchemy 2.0 `select()` syntax? -4. Are all migrations reversible and properly tested? +1. Are all sessions properly managed with context managers? +2. Do relationships use appropriate loading strategies to avoid N+1 queries? +3. Does the code use SQLAlchemy 2.0 `select()` syntax instead of legacy `query()`? +4. Are all migrations reversible and properly tested?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/python/sqlalchemy.md` around lines 29 - 32, The four checklist items ("Are all sessions properly managed with context managers?", "Are relationships configured to avoid N+1 queries?", "Are queries using SQLAlchemy 2.0 `select()` syntax?", "Are all migrations reversible and properly tested?") are repetitive because they all start with "Are"; revise at least two to alternate phrasing for readability—for example convert one or two to imperative checks like "Ensure sessions are managed with context managers" or explanatory prompts like "Verify relationships are configured to avoid N+1 queries" and keep the others as questions so the list mixes question and instruction forms while preserving the same verification intent.amelia/skills/review/elixir/phoenix.md (1)
5-11: Optional: Complete the Quick Reference table.The Quick Reference table structure is in place but all reference cells are empty placeholders ("—"). Consider adding section anchors or line references to make the table functional for quick navigation, or remove it if references aren't planned.
📋 Example completion
| Issue Type | Reference | |------------|-----------| -| Bounded contexts, Ecto integration | — | -| Actions, params, error handling | — | -| Pipelines, scopes, verified routes | — | -| Custom plugs, authentication | — | +| Bounded contexts, Ecto integration | [Contexts](`#contexts`) | +| Actions, params, error handling | [Controllers](`#controllers`) | +| Pipelines, scopes, verified routes | [Routing](`#routing`) | +| Custom plugs, authentication | [Plugs](`#plugs`) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/elixir/phoenix.md` around lines 5 - 11, The Quick Reference table in amelia/skills/review/elixir/phoenix.md currently has placeholder entries ("—"); update the table so each Reference cell links to the corresponding section anchor or line within the document (e.g., add anchors or use heading slugs for "Bounded contexts, Ecto integration", "Actions, params, error handling", "Pipelines, scopes, verified routes", and "Custom plugs, authentication") so the table becomes a functional navigation aid, or remove the table entirely if you don't intend to provide intra-doc references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@amelia/skills/review/go/go-concurrency.md`:
- Around line 231-240: The fetchWithTimeout function currently discards errors
from http.NewRequestWithContext and http.DefaultClient.Do, risking (nil, nil);
change the goroutine to send both response and error by replacing ch with a
channel of a result struct (e.g., type result struct { resp *Response; err error
}) or two-value sends, capture the err from NewRequestWithContext and Do inside
the anonymous goroutine, send {resp, err} into ch, and in the select case read
that result and return resp and err (not always nil); keep the buffered channel
to avoid blocking the goroutine.
- Around line 65-72: The Submit method can race with Shutdown and panic when
sending on a closed channel; update Submit (on WorkerPool) to avoid
unconditional sends by selecting between sending to wp.jobs and the pool
shutdown signal (e.g., wp.ctx.Done() or a done channel) so it returns/ignores
the job if the pool is shutting down instead of attempting the send. Also change
Shutdown to signal cancellation first (call wp.cancel()) and only then
close(wp.jobs) so Submit observers the cancellation path and will not try to
write to a channel that will be closed; reference the Submit and Shutdown
methods and the wp.jobs, wp.cancel (and wp.ctx/wp.done if present) fields when
making these changes.
---
Nitpick comments:
In `@amelia/skills/review/elixir/phoenix.md`:
- Around line 5-11: The Quick Reference table in
amelia/skills/review/elixir/phoenix.md currently has placeholder entries ("—");
update the table so each Reference cell links to the corresponding section
anchor or line within the document (e.g., add anchors or use heading slugs for
"Bounded contexts, Ecto integration", "Actions, params, error handling",
"Pipelines, scopes, verified routes", and "Custom plugs, authentication") so the
table becomes a functional navigation aid, or remove the table entirely if you
don't intend to provide intra-doc references.
In `@amelia/skills/review/python/sqlalchemy.md`:
- Around line 5-11: The Quick Reference table currently has an empty "Reference"
column; update the table in the Quick Reference (the markdown table at the top
listing "Session lifecycle..." etc.) by either removing the "Reference" column
entirely or populating it with meaningful links: add internal markdown anchor
links to the corresponding checklist sections (e.g., "#session-lifecycle",
"#relationship-lazy-loading", "#select-vs-query", "#alembic-patterns") or insert
external documentation URLs for each row; ensure headers/anchors match the
actual section headings used later in the document and keep table formatting
consistent.
- Around line 29-32: The four checklist items ("Are all sessions properly
managed with context managers?", "Are relationships configured to avoid N+1
queries?", "Are queries using SQLAlchemy 2.0 `select()` syntax?", "Are all
migrations reversible and properly tested?") are repetitive because they all
start with "Are"; revise at least two to alternate phrasing for readability—for
example convert one or two to imperative checks like "Ensure sessions are
managed with context managers" or explanatory prompts like "Verify relationships
are configured to avoid N+1 queries" and keep the others as questions so the
list mixes question and instruction forms while preserving the same verification
intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a99a0d6c-c750-430a-9a59-3e57647e6a42
📒 Files selected for processing (8)
amelia/skills/review/__init__.pyamelia/skills/review/elixir/phoenix.mdamelia/skills/review/go/go-concurrency.mdamelia/skills/review/python/sqlalchemy.mdamelia/skills/review/react/react-router.mdamelia/skills/review/swift/swiftui.mddocs/plans/2026-03-08-develop-page-design.mddocs/plans/2026-03-08-develop-page-plan.md
💤 Files with no reviewable changes (2)
- docs/plans/2026-03-08-develop-page-design.md
- docs/plans/2026-03-08-develop-page-plan.md
✅ Files skipped from review due to trivial changes (2)
- amelia/skills/review/react/react-router.md
- amelia/skills/review/swift/swiftui.md
🚧 Files skipped from review as they are similar to previous changes (1)
- amelia/skills/review/init.py
Address PR feedback: Submit() now uses select to avoid send-on-closed-channel panic during shutdown, and fetchWithTimeout() propagates HTTP errors instead of silently discarding them. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
amelia/skills/review/go/go-concurrency.md (1)
98-103: Consider showing error handling for Submit in the example.Line 98 ignores the
Submiterror with_. While acceptable for "best effort" background tasks like welcome emails, the example could demonstrate when and how to handle submission failures (e.g., logging, metrics, or retry for critical operations).Optional: Add error handling for demonstration
- _ = s.workers.Submit(r.Context(), Job{ + if err := s.workers.Submit(r.Context(), Job{ ID: "welcome-email-" + user.ID, Execute: func(ctx context.Context) error { return s.emailService.SendWelcome(ctx, user) }, - }) + }); err != nil { + // Non-critical: log but don't fail the request + s.logger.Warn("failed to queue welcome email", "user_id", user.ID, "err", err) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amelia/skills/review/go/go-concurrency.md` around lines 98 - 103, The example currently ignores the return from s.workers.Submit; update the snippet around s.workers.Submit and the Job creation (Job, Execute func, s.emailService.SendWelcome) to check the error result, and on failure perform appropriate handling such as logging the error via your logger/metrics and optionally triggering a retry or fallback for critical flows; keep the example compact by showing an if err := s.workers.Submit(...); err != nil { /* log/metric/retry */ } pattern so readers see how to surface submission failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@amelia/skills/review/go/go-concurrency.md`:
- Around line 98-103: The example currently ignores the return from
s.workers.Submit; update the snippet around s.workers.Submit and the Job
creation (Job, Execute func, s.emailService.SendWelcome) to check the error
result, and on failure perform appropriate handling such as logging the error
via your logger/metrics and optionally triggering a retry or fallback for
critical flows; keep the example compact by showing an if err :=
s.workers.Submit(...); err != nil { /* log/metric/retry */ } pattern so readers
see how to surface submission failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf2605ac-e3cb-4ea4-8267-9350a71a7edd
📒 Files selected for processing (1)
amelia/skills/review/go/go-concurrency.md
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.
Summary
Internalize beagle review skills into
amelia/skills/review/as plain markdown files and inject them into reviewer prompts server-side. This removes the dependency on external beagle plugins and makes the reviewer driver-agnostic (works with OpenAI, Codex, not just Claude).Also includes the Develop page feature (merged from #528) replacing QuickShotModal with a GitHub issue-based workflow creation UI.
Changes
Added
amelia/skills/review/(Python, React, Go, Elixir, Swift, security)detect_stack()— identifies technology tags from file paths and import patterns in diffsload_skills()— resolves tags to markdown files and concatenates them for prompt injection{review_guidelines}placeholder in reviewer prompt for server-side skill injectionreview_guidelinesparameter onReviewer.__init__()GET /api/github/issuesendpoint for listing repo issues viaghCLIGitHubIssueComboboxcomponent with debounced search/developroute andDevelopPagefor creating workflows from GitHub issuesChanged
Skilltool or beagle pluginslast_review→last_reviews: list[ReviewResult]state modelRemoved
QuickShotModalcomponent and related CSS animationsRequestReviewDialogcomponent (superseded by internalized review flow)getWorkflowDefaultsAPI methodtest_skill_injection.pyandtest_review_endpoint.py(replaced by new tests)Motivation
Users had to separately install beagle skills plugins, and only Claude could access the
Skilltool — other LLM drivers couldn't load review guidelines. Server-side injection solves both problems.Closes #530
Testing
Generated with Claude Code