-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/drc 2089 integration change to use the claude agent sdk to replace #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Major architectural refactor to implement a modular, multi-agent system with proper separation of concerns and dependency injection. Changes: - Refactor agent.ts to use AgentCore with modular architecture - Add dependency injection for prompt builders, providers, and lifecycle - Implement comprehensive logging with pino (structured JSONL + pretty console) - Add provider abstraction layer for GitHub/GitLab/Bitbucket support - Create prompt builder system with composable prompts - Add lifecycle hooks for extensibility (onStart, onComplete, onError, etc.) - Organize code into logical directories (core/, logging/, prompts/, providers/) - Add TypeScript types for hooks, prompts, and providers - Update configuration to support provider selection and feature flags - Add comprehensive documentation (AGENTS.md, CLAUDE.md) - Update .gitignore to exclude .env and .cursor/ Architecture improvements: - Separation of concerns: Core logic, prompts, providers, logging isolated - Dependency injection: Easier testing and modularity - Provider abstraction: Easy to add new Git platforms - Extensible hooks: Custom behavior without modifying core - Type safety: Comprehensive TypeScript interfaces 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This commit fixes two critical issues in the multi-agent architecture: 1. **Subagent Tool Calling**: - Changed subagent tools from prefix patterns ['mcp__recce'] to explicit lists - Added anti-fabrication rules to prevent subagents from making up results - Subagents must now report ERROR status if unable to call tools - Updated recce-analysis and preset-check-executor subagents 2. **Output Format**: - Aligned with ms3-response-format.md specification - Changed heading levels: ## for main sections, ### for subsections - Added date to title: "# PR Validation Summary [YYYY-MM-DD]" - Enhanced Suggested Checks with Recce link references 3. **Error Handling**: - Tool call failures now treated as merge blockers - Added explicit ERROR status for tool execution failures - Enhanced merge recommendation logic Files changed: - src/prompts/index.ts: Explicit tool lists for subagents - src/prompts/fragments/preset_checks.ts: Anti-fabrication rules - src/prompts/fragments/recce_analysis.ts: Honest error reporting - src/prompts/fragments/pr_summary_format.ts: Corrected heading levels and format 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
PR Analysis Summary: DataRecce/jaffle_shop_agentic #3Status: PR Overview
File Changes Summary
dbt Model ChangesAdded Models
Modified Models
Removed ModelsNone Lineage ImpactDependency Chain: Critical Observation: The
Schema Changes✅ No breaking schema changes detected for Preset Check ResultsOverall Status: ❌ FAIL (Validation Blocked)
Summary:
Check Details✅ Check 1: Schema of customers, orders and modified nodes
❌ Check 2: Row count of customers, orders and modified table models
|
| Concern | Severity | Description |
|---|---|---|
| Missing validation | HIGH | Cannot validate row counts and CLV values due to DB errors |
| Hardcoded payment logic | MEDIUM | payment_count = 1 may not reflect actual data |
| Layer inversion | MEDIUM | Dimension table sourcing from fact table violates typical architecture |
| Downstream impact | MEDIUM | Two models (customer_segments, customer_order_pattern) affected by changes |
📊 Data Quality Assessment
Current Validation Score:
- 1 check passed out of 4
- 2 critical checks blocked by database errors
- 1 check degraded to warning level
Recommendation: DO NOT MERGE until:
- Production database connectivity is restored
- All 4 preset checks return PASS status
- Layer architecture is reviewed and confirmed acceptable
- Payment data logic is validated
Recommendations
Priority 1: Environmental Issues (BLOCKER)
- Verify production database connection - Recce cannot reach
'jaffle_shop'.'prod'schema - Check database credentials - Ensure proper access permissions are configured
- Verify table deployment - Confirm
customerstable exists in production - Re-run validation - Once connectivity is restored, re-execute all 4 preset checks
Priority 2: Architecture Review
- Review layer hierarchy - Confirm if
customersshould depend onfac_ordersor if this is a modeling error - Assess downstream impact - Validate
customer_segmentsandcustomer_order_patternproduce correct outputs with the modified source - Document design decision - If intentional, document why dimension feeds from fact table
Priority 3: Data Validation
- Validate payment logic - Review why
payment_countis hardcoded to 1; verify each order has exactly one payment record - Test joins - Ensure LEFT JOINs on customers and payments don't introduce unexpected NULL values
- Run dbt tests - Execute existing dbt tests to catch data quality issues before merge
Merge Decision
Current Status: ❌ BLOCKED
- Merge should NOT proceed until validation errors are resolved
- Validation should be fully green (all checks PASS) before approval
Summary Table
| Aspect | Status | Details |
|---|---|---|
| Schema Changes | ✅ Safe | No breaking column changes |
| Data Validation | ❌ Blocked | DB connectivity issues prevent validation |
| Model Changes | 1 added, 1 modified, 2 impacted downstream | |
| Architecture | Potential layer inversion (dimension ← fact) | |
| Overall Status | ❌ BLOCKED | Environmental issues + architecture concerns |
Generated: 2025-11-05
PR Link: DataRecce/jaffle_shop_agentic#3
src/agent.ts
Outdated
| ...providerMcpConfig, | ||
| recce: { | ||
| type: 'sse' as const, | ||
| url: 'http://0.0.0.0:8080/sse', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change here to use stdio mode mcp
PR Analysis Summary: DataRecce/jaffle_shop_agentic #3PR OverviewPR Title: SummaryThis PR introduces a new fact table File Changes
CI/CD Status: ⏳ Pending (no checks currently configured) dbt Model ChangesAdded Models
Modified Models
Downstream Impact AnalysisThe
Schema Changes✅ No column-level schema changes detected across customers, orders, and modified models. Changes appear to be logic-level or data-level modifications rather than structural changes. Row Count DifferencesPreset Check ResultsOverall Status: ❌ BLOCKEDSummary Table:
Score: 1/4 Passed | 3/4 Errors Detailed Findings✅ Check 1: Schema Validation - PASS
❌ Check 2-4: Data Validation Checks - ERRORAll three data validation checks (row counts, value matching, aggregate metrics) failed with the same critical issue: Error: Specific Failures:
Impact Analysis🚨 Critical Issues1. Environment Configuration Blocker
2. High-Risk Model Changes
3. Upstream Source Changes
4. Downstream Cascade Risk
|
## Changes ### 1. Refactor agent.ts into Clean Architecture modules - Split 829-line monolith into 7 focused modules (~80 lines each) - New structure: src/agent/ with types, mcp-connector, preset-loader, message-handler, agent-executor, pr-analyzer - Maintains backward compatibility via thin wrapper in src/agent.ts - Benefits: Better testability, maintainability, and extensibility ### 2. Fix GitHub MCP tool registration - Updated tool names to match actual MCP server tools - Changed from mcp__github__get_pull_request to mcp__github__pull_request_read - Fixed in: src/prompts/index.ts and src/agent/pr-analyzer.ts - Result: GitHub MCP now registers 26 tools (previously 0) ### 3. Improve logging system - Multi-line log alignment: Changed from spaces to tabs for consistency - Unified logging: Replaced console.log with logInfo/logError for timestamps - Modified: src/logging/agent_logger.ts, src/recce/preset_service.ts ### 4. Fix TypeScript type errors - Added ProviderType re-export in src/types/index.ts - Fixed gitUrl undefined handling in src/index.ts - All index.ts TypeScript errors resolved ### 5. Update documentation - Updated CLAUDE.md to reflect new modular architecture - Added architecture diagram and design principles - Documented key changes and improvements ## Testing - ✅ Build successful: pnpm run build - ✅ GitHub MCP: 26 tools registered - ✅ Recce MCP: 6 tools registered - ✅ PR analysis: Functional end-to-end - ✅ Log alignment: Consistent across terminal and files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Removed LOG_DIR and LOG_PER_TURN (not used in code) - Commented out all optional configurations with defaults - Keep only required fields (ANTHROPIC_API_KEY, GIT_TOKEN) uncommented - Improves clarity for users setting up the project 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Migrate to Claude Agent SDK Multi-Agent Architecture
Overview
Refactors the PR analysis agent from custom orchestration to Claude Agent SDK with a multi-agent delegation pattern.
Architecture
Key Changes:
agentsconfig instead of custom tool routingDependencies
This PR adds the MCP HTTP server that
recce-summary-agentconnects to via SSE transport.Usage
1. Configure Environment
2. Start Recce MCP Server
Replace
/path/to/dbt/projectwith your dbt project path:Example with actual path:
Server starts at
http://0.0.0.0:8080with SSE endpoint at/sse.3. Run Agent
Example:
File Structure
Changes Summary
src/core/agent_core.ts), old prompt systemStatus: ✅ Ready for review
Build:
dist/index.js847.5kbDependencies: Requires DataRecce/recce#932 (MCP HTTP server)