From gx-arch-review
Perform a Python code review on the provided code. Accept input in any of the following forms:
How this command is triggered — by the user, by Claude, or both
Slash command
/gx-arch-review:gx-review-diFiles this command reads when invoked
The summary Claude sees in its command listing — used to decide when to auto-load this command
# Review Galaxy Code for Dependency Injection Best Practices Perform a Python code review on the provided code. Accept input in any of the following forms: 1. A working directory path (analyze git diff in that directory) 2. A Git commit reference (analyze changes in that commit) 3. A list of Python file paths (analyze those files) 4. A planning document (analyze the Python files in the plan) ## Review Criteria Review the code and ensure it follows best practices for dependency injection in Galaxy: 1. **Avoid FastAPI-style injection** - Prefer Galaxy-style DI using Lagom and type-based i...
Perform a Python code review on the provided code. Accept input in any of the following forms:
Review the code and ensure it follows best practices for dependency injection in Galaxy:
app accessapp property creation - Don't add new properties to the app object when DI can be usedGalaxy's app object was historically a "god object" - an object that knows/does too much. Every component, controller, and web transaction had a reference to app. This creates:
UniverseApplication, which creates that manager)app are needed, hard to mock)Galaxy uses Lagom for type-based DI:
Use StructuredApp interface instead of UniverseApplication to break circular dependencies:
# BAD - creates circular dependency
class MyManager:
def __init__(self, app: UniverseApplication):
...
# GOOD - uses interface, breaks cycle
class MyManager:
def __init__(self, app: StructuredApp):
...
# BETTER - inject only what you need
class MyManager:
def __init__(self, model: GalaxyModelMapping, security: IdEncodingHelper):
...
Anti-pattern - using app directly:
class DatasetCollectionManager:
def __init__(self, app):
self.model = app.model
self.security = app.security
self.hda_manager = hdas.HDAManager(app) # BAD: internal construction
self.history_manager = histories.HistoryManager(app)
Correct pattern - injecting dependencies:
class DatasetCollectionManager:
def __init__(
self,
model: GalaxyModelMapping,
security: IdEncodingHelper,
hda_manager: HDAManager,
history_manager: HistoryManager,
):
self.model = model
self.security = security
self.hda_manager = hda_manager
self.history_manager = history_manager
Benefits:
app objectAnti-pattern - FastAPI Depends():
def get_tags_manager() -> TagsManager:
return TagsManager()
@cbv(router)
class FastAPITags:
manager: TagsManager = Depends(get_tags_manager) # BAD: FastAPI style
Correct pattern - Galaxy's depends():
@cbv(router)
class FastAPITags:
manager: TagsManager = depends(TagsManager) # GOOD: Galaxy style
Anti-pattern:
class TagsController(BaseAPIController):
def __init__(self, app):
super().__init__(app)
self.manager = TagsManager() # BAD: manual construction
Correct pattern:
class TagsController(BaseGalaxyAPIController):
manager: TagsManager = depends(TagsManager) # GOOD: DI
Correct pattern - using @galaxy_task:
@celery_app.task(ignore_result=True)
@galaxy_task
def purge_hda(hda_manager: HDAManager, hda_id):
hda = hda_manager.by_id(hda_id)
hda_manager._purge(hda)
Dependencies are automatically injected based on type annotations.
Task with multiple dependencies:
@celery_app.task
@galaxy_task
def set_metadata(
hda_manager: HDAManager,
ldda_manager: LDDAManager,
dataset_id,
model_class='HistoryDatasetAssociation'
):
...
When reviewing, reference these Galaxy codebase locations for patterns:
lib/galaxy/di/ - DI container setuplib/galaxy/structured_app.py - StructuredApp interface definitionlib/galaxy/app.py - UniverseApplication implementationlib/galaxy/managers/ - Manager implementations using DIlib/galaxy/webapps/galaxy/api/ - FastAPI controllers using DIlib/galaxy/celery/tasks.py - Celery tasks using DIFor each file/change, check:
app access for dependencies - Does code access app.some_manager instead of injecting?StructuredApp instead of UniverseApplication where needed?depends() - Controllers use depends(Type) not FastAPI Depends()?@galaxy_task decorator with type-annotated parameters?For each issue found, report:
Summarize findings with counts by issue type and overall assessment of DI compliance.
npx claudepluginhub jmchilton/gx-arch-review/architecture-reviewReviews and improves system architecture: analyzes high-level structure, design patterns, dependencies, data flow, scalability, security, testing, and suggests enhancements.
/full-reviewOrchestrates phased multi-dimensional code reviews across architecture, security, performance, testing, and best practices using subagents. Produces structured reports in .full-review/. Accepts target and optional flags.
/f5-reviewPerforms automated code review detecting project stack and running linting, type checks, complexity analysis, OWASP security scans, and quality metrics. Generates scored reports. Supports check, full, security, pr subcommands.
/fastapi-reviewReviews a FastAPI codebase for architecture, async correctness, dependency injection, Pydantic schemas, security, performance, and testability. Produces a structured report with severity, file location, issue explanation, and suggested fix.