Repository navigation
SAN-1274 PR 1 — Add the verified MDE AI skills foundation - #47
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 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 |
|
Failed to generate code suggestions for PR |
|
/review |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 4 medium |
| BestPractice | 18 medium |
| ErrorProne | 6 high |
| Security | 4 minor 5 critical 11 medium |
| Complexity | 16 medium |
🟢 Metrics 563 complexity · 14 duplication
Metric Results Complexity 563 Duplication 14
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The PR is currently not up to quality standards and contains several blockers that should prevent merging. Most critical are the logic errors in gmaps.py regarding travel mode constants and coordinate validation, alongside insecure subprocess calls in generate_video.py. Additionally, the PR acknowledges a critical Next.js 16.2.6 security advisory that remains unresolved. There is a significant implementation gap regarding test coverage and execution evidence: high-complexity files for Maps and Gemini lack unit tests, and no results were provided for the defined skill evaluations. Finally, the presence of a 900-line clone for the Google Maps script contradicts the acceptance criterion to prune bulk mirrors.
About this PR
- The Next.js 16.2.6 security advisory mentioned in the PR description is a blocker that must be resolved prior to merging to ensure the security of the MDE foundation.
- No execution evidence or automated test results are provided for the complex Python and Node.js utility scripts (gmaps.py, provider-registry.mjs) or the skill evaluation JSONs added in this PR.
- There is significant documentation overlap and potential duplication between the 'official' reference packs and the 'MDE-specific' instruction overlays for Mastra and Supabase. Consider consolidating these to prevent logic drift in agent instructions.
1 comment outside of the diff
[REDACTED:HIGH_ENTROPY]
line 18🟡 MEDIUM RISK
The use of '-printf' in the find command is a GNU extension not supported by BSD find on macOS. For portability across environments, consider using standard 'find' with 'xargs' or 'basename' logic.
Test suggestions
- Execute Cloudinary skill evaluations using prompts defined in evals/evals.json\n- [ ] Execute CopilotKit skill evaluations using prompts defined in evals/evals.json\n- [ ] Execute Gemini skill evaluations using prompts defined in evals/evals.json\n- [ ] Run gmaps.py script to verify connectivity and support for 20+ REST APIs\n- [ ] Run provider-registry.mjs to verify model string generation and provider listing\n- [ ] Run verify-edge-inventory.sh to confirm directory and config.toml synchronization\n- [ ] Unit tests for .claude/skills/gemini/references/official/gemini-omni-flash-api/scripts/video/generate_video.py\n- [ ] Unit tests for .claude/skills/maps/scripts/gmaps.py\n- [ ] Unit tests for .claude/skills/gemini/references/official/gemini-omni-flash-api/scripts/video/prep_video.py
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Execute Cloudinary skill evaluations using prompts defined in evals/evals.json\n- [ ] Execute CopilotKit skill evaluations using prompts defined in evals/evals.json\n- [ ] Execute Gemini skill evaluations using prompts defined in evals/evals.json\n- [ ] Run gmaps.py script to verify connectivity and support for 20+ REST APIs\n- [ ] Run provider-registry.mjs to verify model string generation and provider listing\n- [ ] Run verify-edge-inventory.sh to confirm directory and config.toml synchronization\n- [ ] Unit tests for .claude/skills/gemini/references/official/gemini-omni-flash-api/scripts/video/generate_video.py\n- [ ] Unit tests for .claude/skills/maps/scripts/gmaps.py\n- [ ] Unit tests for .claude/skills/gemini/references/official/gemini-omni-flash-api/scripts/video/prep_video.py
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
|
Task 66 · SAN-1274 PR #47 — Codacy review fixes pushed Exact head: Fixed and verified:
Validation on exact head before push:
All four Codacy inline threads addressed and resolved. The isolated Next.js critical-security change remains intentionally in PR #50; it was not mixed into this PR. |
Task 61 · SAN-1274 PR 1 — Add the verified MDE AI skills foundation
What this PR does
This PR takes the useful, reusable skills out of the very large PR #45 and adds only the canonical MDE skill foundation to
main.Real-world example: when we ask Claude/OpenCode/Codex to fix a CopilotKit rendering bug, a Supabase RLS issue, a Mastra workflow, or a Google Maps integration, the agent should load one clear MDE skill with current project rules instead of searching through duplicated aliases, stale symlinks, and a giant mixed orchestration PR.
This PR adds the stable specialist layer first. It does not add the old router/orchestration system; SAN-1273 will build the new lightweight orchestration layer later.
Why PR #47 is needed
PR #45 is the source branch. PR #47 is the extraction.
flowchart LR A[PR #45\nlarge mixed source] --> B[Extract reviewed reusable skills] B --> C[PR #47\ncanonical MDE skills] A --> D[Old router/orchestration] D --> E[SAN-1273\nnew simplified orchestration]Included skills
copilotkitmastrasupabasegeminimapsstripenextjscloudinaryeventsreal-estateArchitecture / ownership
flowchart TD UI[Frontend\nNext.js + React] --> CK[CopilotKit\nAG-UI / runtime bridge] CK --> MA[Mastra\nagents / tools / workflows] MA --> SU[Supabase\ndata / auth / RLS / realtime] MA --> GE[Gemini\nmodel + grounding] UI --> MAP[Google Maps\nPlaces / map UI] UI --> EV[Events domain] UI --> RE[Real-estate domain] MA --> STR[Stripe\npayments where applicable] CL[Cloudinary\noptional media infrastructure] -. only if in scope .-> UIFrontend setup
/api/copilotkitBackend setup
This PR changes skill/instruction files, not customer-facing screens or runtime business logic. User-facing workflows should therefore remain behaviorally unchanged.
User journey / developer journey
sequenceDiagram participant Dev as Developer / Agent participant Skill as Canonical MDE Skill participant Repo as Current MDE Source participant Vendor as Official / pinned vendor docs participant Test as Verification Dev->>Skill: Ask for CopilotKit / Mastra / Supabase / Maps work Skill->>Repo: Inspect installed code and versions first Skill->>Vendor: Load pinned/current official guidance when needed Skill->>Repo: Make the smallest MDE-safe change Repo->>Test: Run focused tests + build/runtime proof Test-->>Dev: Evidence before DoneEfficient execution model
Use the narrowest specialist directly instead of routing everything through a large orchestration layer:
This reduces context load and makes failures easier to isolate.
Skills / MCP / tools to use for review
For this PR, reviewers should use:
/home/sk/mdeaiDo not use the dirty local
maincheckout as extraction evidence.Verified source and exact scope
1f56d9f4d9ce13b5a22a13b511c2d68cb0f3272329627d470627287a98b6cb981c16681f9d38243ceb6201f09274d41bb4a569cf25e45e5e70b2d0cdBest-practice references
Forensic audit results
Verified good
00git diff --checkpassesErrors / red flags / blockers
npm audit --audit-level=criticalfinds 1 critical Next.js advisory on Next.js16.2.616.3.5security change from PR #50 before treating the final stack as production-readynpm audit fix --forceblindlyImportant CI clarification
The Floor failure does not indicate a broken skill extraction. CI successfully completed:
PR #50 isolates the intended security fix and upgrades Next.js to
16.3.5:#50
Screens / customer workflows affected
No screen implementation is intentionally changed by this PR.
Regression-sensitive surfaces to smoke after the full stack lands:
//chat/eventsand/events/[slug]/rentalsand/rentals/[id]/restaurants/cafes/nightlife/tripsand/trips/[id]/host/*/admin/event-bookings/api/copilotkit/[[...path]]Expected result: existing UI/user journeys continue working; the change improves how coding agents understand and modify the system.
Pre-merge checklist
Skill quality
name+ trigger-orienteddescriptionRepository / CI
git diff --checkScope / safety
/home/sk/mdeaicheckout untouchedProduction-ready success criteria
This PR/foundation is production-ready when:
mainpasses the same validation again;mainwithout importing PR SAN-1272 — Make MDE AI coding agents choose the right skills, workflow, and checks #45 wholesale.Post-merge actions
After this PR lands:
origin/mainand record the exact merge SHA.main; verify its diff contains only core workflow skills.npm run flooron the current landing stack.mainare final.npm audit --audit-level=critical= 0.main.Scores
Merge decision
Do not merge yet.
The extraction itself is sound, but the production gate is not complete because the current branch still inherits the Next.js 16.2.6 critical advisory and there is no independent exact-head review yet.
Fastest safe path:
flowchart LR A[PR #47 skills reviewed] --> B[Resolve critical audit through PR #50 landing strategy] B --> C[Required checks green] C --> D[Independent review] D --> E[Merge #47] E --> F[Retarget #48 to main]Orchestration remains deferred to SAN-1273.