Consolidate UI diagrams, deploy deps, and version policy#18
Conversation
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughWorkflows shifted from multi-runtime to Deno-centric, adding conditional UI dependency installation steps in deploy workflows; CI and deploy workflows were substantially rewritten for Deno. Added a dependency version policy in AGENTS.md. Minor UI package.json reordering. PromptInputSubmit now accepts an Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Deno as Deno
participant UI as src/ui (package scripts)
participant Playwright as Playwright/Browsers
GH->>Deno: Setup Deno runtime
GH->>UI: Check `src/ui/package.json`
alt UI package exists
GH->>UI: Run `cd src/ui && deno install --node-modules-dir --allow-scripts`
GH->>UI: Run `deno task ui:build` or fallback build
end
GH->>Playwright: Install browsers / run e2e
GH->>GH: Collect results / proceed to deploy
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Summary of ChangesHello @venikman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request consolidates several key aspects of the project's development and documentation. It introduces a formal dependency versioning policy to maintain consistency and security across the codebase. Furthermore, it refines the UI build process for Deno Deploy by ensuring necessary Node.js and UI dependencies are installed upfront. A significant user interface enhancement is also included, allowing for the dynamic rendering of protocol diagrams using Mermaid and presenting a detailed comparison matrix directly within the application, offering a more comprehensive and interactive understanding of different architectural approaches. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new dependency versioning policy, adds support for rendering Mermaid diagrams in the UI, and updates dependencies accordingly. The changes are well-structured. The new MermaidDiagram component is a great addition for visualizing protocol flows.
My review includes a couple of suggestions for the MermaidDiagram component to improve its robustness and maintainability:
- Using
nanoidfor generating unique IDs instead ofMath.random(). - Enhancing error logging to aid in debugging diagram rendering issues.
Overall, this is a solid contribution that improves the developer experience and documentation within the UI.
There was a problem hiding this comment.
Pull request overview
This PR consolidates protocol visualization in the UI, ensures build dependencies are installed before deployment, and establishes a repository-wide version policy for dependencies.
Changes:
- Added dependency version policy requiring latest stable version checks before adding/updating dependencies
- Modified deployment workflows to install Node.js and UI dependencies before building for Deno Deploy
- Implemented Mermaid diagram rendering in the UI with protocol comparison matrices for Streamdown, A2UI, MCP, and json-render
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| AGENTS.md | Added dependency version policy section |
| .github/workflows/deploy-deno.yml | Added Node.js setup and UI dependency installation steps |
| .github/workflows/deploy-staging.yml | Added Node.js setup and UI dependency installation steps |
| src/ui/package.json | Added mermaid dependency (v11.12.2) |
| src/ui/package-lock.json | Lock file updates for mermaid and transitive dependencies |
| src/ui/src/styles.css | Added responsive styling for mermaid diagrams |
| src/ui/src/components/mermaid-diagram.tsx | New component for rendering Mermaid diagrams with theme support |
| src/ui/src/content/protocol-bakeoff.ts | New file with protocol diagrams and comparison matrix data |
| src/ui/src/App.tsx | Integrated protocol bakeoff section with diagrams and comparison table |
Files not reviewed (1)
- src/ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/deploy-staging.yml:
- Around line 24-32: The CI workflow uses actions/setup-node@v4 and
node-version: "20" in the "Setup Node" step; update that step to use
actions/setup-node@v6 and set node-version: "24" so the job runs on the current
Active LTS. Locate the "Setup Node" job/step and change the action reference
from actions/setup-node@v4 to actions/setup-node@v6 and update the node-version
value from "20" to "24" (keep existing cache and cache-dependency-path
settings).
🧹 Nitpick comments (3)
src/ui/src/App.tsx (1)
617-687: Consider adding table accessibility attributes and Playwright tests.The Protocol Bakeoff section is well-structured and follows existing patterns. Two observations:
- Accessibility: Add
scopeattributes to table headers for better screen reader support:♻️ Suggested accessibility improvement
- <th className="pb-2 pr-4">Dimension</th> - <th className="pb-2 pr-4">Streamdown (A)</th> - <th className="pb-2 pr-4">A2UI (B)</th> - <th className="pb-2 pr-4">MCP (C)</th> - <th className="pb-2">json-render (D)</th> + <th scope="col" className="pb-2 pr-4">Dimension</th> + <th scope="col" className="pb-2 pr-4">Streamdown (A)</th> + <th scope="col" className="pb-2 pr-4">A2UI (B)</th> + <th scope="col" className="pb-2 pr-4">MCP (C)</th> + <th scope="col" className="pb-2">json-render (D)</th>
- Testing: As per coding guidelines, UI changes in
src/ui/**should include Playwright tests. Please add tests covering the diagram grid rendering and the comparison matrix table.src/ui/src/components/mermaid-diagram.tsx (1)
21-61: Add unit tests for the MermaidDiagram component.As per coding guidelines, logic changes should include unit tests. Consider adding tests covering:
- Successful diagram rendering
- Error fallback display when diagram code is invalid
- Theme switching behavior
- Title/subtitle conditional rendering
src/ui/src/content/protocol-bakeoff.ts (1)
147-176: Consider adding a type for the protocol matrix.The matrix data is well-organized. For better type safety and discoverability, consider adding an explicit type:
♻️ Optional type annotation
+type ProtocolMatrixRow = { + dimension: string; + streamdown: string; + a2ui: string; + mcp: string; + jsonRender: string; +}; + -export const protocolMatrix = [ +export const protocolMatrix: ProtocolMatrixRow[] = [
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.github/workflows/deploy-deno.yml.github/workflows/deploy-staging.ymlAGENTS.mdsrc/ui/package.jsonsrc/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsxsrc/ui/src/content/protocol-bakeoff.tssrc/ui/src/styles.css
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Include unit tests for all logic changes
Files:
src/ui/src/content/protocol-bakeoff.tssrc/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsx
**/src/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/ui/**/*.{ts,tsx,jsx,js}: Include Playwright tests for all UI changes
Use Figma for design context; reference design tokens and specs from Figma in UI component files
Files:
src/ui/src/content/protocol-bakeoff.tssrc/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsx
{AGENTS.md,.agents/roles.yaml,.codex/skills/**,docs/adr/**}
📄 CodeRabbit inference engine (AGENTS.md)
Keep AGENTS.md file concise at under 200 lines; push detailed documentation into skills in .codex/skills/, role definitions in .agents/roles.yaml, and decision records in docs/adr/
Files:
AGENTS.md
**/AGENTS.md
📄 CodeRabbit inference engine (AGENTS.md)
Subdirectories may override root AGENTS.md rules with their own AGENTS.md files; use nearest AGENTS.md for scope precedence
Files:
AGENTS.md
🧠 Learnings (5)
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/src/ui/**/*.{ts,tsx,jsx,js} : Use Figma for design context; reference design tokens and specs from Figma in UI component files
Applied to files:
src/ui/src/content/protocol-bakeoff.tssrc/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsx
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/src/ui/**/*.{ts,tsx,jsx,js} : Include Playwright tests for all UI changes
Applied to files:
src/ui/src/App.tsx
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/AGENTS.md : Subdirectories may override root AGENTS.md rules with their own AGENTS.md files; use nearest AGENTS.md for scope precedence
Applied to files:
AGENTS.md
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Deploy only through GitHub Actions pipelines; no manual production deploys allowed
Applied to files:
AGENTS.md
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Use Deno as the primary runtime for scripts, tooling, and TypeScript code
Applied to files:
AGENTS.md
🧬 Code graph analysis (2)
src/ui/src/App.tsx (2)
src/ui/src/content/protocol-bakeoff.ts (2)
protocolDiagrams(8-145)protocolMatrix(147-176)src/ui/src/components/mermaid-diagram.tsx (1)
MermaidDiagram(21-87)
src/ui/src/components/mermaid-diagram.tsx (1)
src/ui/src/lib/utils.ts (1)
cn(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Playwright Tests
- GitHub Check: Agent
🔇 Additional comments (9)
src/ui/src/styles.css (1)
138-142: LGTM!The responsive SVG styling is appropriate for Mermaid diagrams. Using
max-width: 100%andheight: autoensures diagrams scale correctly across viewport sizes while maintaining aspect ratio.AGENTS.md (1)
17-23: Well-structured policy addition.The Dependency Version Policy is clear and actionable. It appropriately scopes to npm, Deno, and GitHub Actions, and the requirement to document deviations in PR summaries ensures traceability. File remains within the 200-line limit per coding guidelines.
.github/workflows/deploy-deno.yml (1)
24-32: Consistent with staging workflow—good practice.The Node setup and UI dependency installation mirror
deploy-staging.yml, ensuring parity between environments. The same version verification comments from the staging workflow apply here.src/ui/package.json (1)
33-33: No action needed. The mermaid version^11.12.2is the latest stable release and already compliant with dependency versioning requirements.src/ui/src/App.tsx (1)
37-42: LGTM!The imports are correctly structured, bringing in the new
MermaidDiagramcomponent and the protocol bakeoff data. The named imports align with the exports from the respective modules.src/ui/src/components/mermaid-diagram.tsx (2)
34-61: LGTM on the rendering effect.The async rendering pattern with the
cancelledflag correctly prevents updates after unmount. ThesecurityLevel: "strict"configuration is appropriate for rendering user-facing diagrams.
63-86: LGTM!The component structure is clean, with appropriate conditional rendering for optional title/subtitle and proper overflow handling for wide diagrams.
src/ui/src/content/protocol-bakeoff.ts (2)
1-6: LGTM!The
ProtocolDiagramtype is well-defined with clear, required fields that match the component's props interface.
8-145: LGTM!The diagram definitions are well-structured with unique IDs, descriptive titles, and valid Mermaid syntax. The overview flowchart and sequence diagrams provide clear visual documentation of the protocol scenarios.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Addressed review feedback + ensured Playwright passes:
Tests:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/ui/src/components/mermaid-diagram.tsx (1)
1-69: Add Playwright tests and unit tests for MermaidDiagram component.The implementation correctly:
- Uses
nanoidfor unique render IDs (line 40)- Separates
mermaid.initializeinto its own effect dependent only ontheme(lines 31-36)- Generates a fresh
renderIdinside the render effect to satisfy mermaid's "never reused ID" requirement- Logs detailed error context to console (lines 50-54)
- Implements proper cleanup with
cancelledflagHowever, per coding guidelines, UI components in
src/ui/require Playwright tests for all UI changes, and logic changes require unit tests. Add tests covering:
- Diagram rendering with valid and invalid Mermaid syntax
- Theme switching behavior
- Error handling and display
- Component cleanup and cancellation logic
🧹 Nitpick comments (3)
.github/workflows/ci.yaml (1)
179-190: Useworking-directoryfor consistency with other jobs in the workflow.The
cd src/ui && npm cipattern works, but usingworking-directorywould be more consistent with other jobs in this workflow (e.g., thebunjob).Suggested improvement
- name: Install UI dependencies - run: cd src/ui && npm ci + working-directory: src/ui + run: npm cisrc/ui/src/components/mermaid-diagram.tsx (1)
15-20: Consider typingsecurityLevelexplicitly.The
securityLevel: "strict"is a string literal, but mermaid'sSecurityLeveltype expects specific values. While"strict"is valid, explicitly typing the config object ensures type safety if mermaid's API changes.🔧 Optional: Add type annotation
+import type { MermaidConfig } from "mermaid"; + -const mermaidConfig = { +const mermaidConfig: MermaidConfig = { startOnLoad: false, securityLevel: "strict", flowchart: { curve: "basis" }, sequence: { showSequenceNumbers: false }, };src/ui/src/App.tsx (1)
805-839: Table accessibility could be enhanced withscopeattributes.The comparison matrix table uses semantic
thead/tbodybut header cells would benefit from explicitscopeattributes for screen reader clarity.♿ Optional: Add scope attributes to table headers
<thead> <tr className="border-b border-border text-left text-xs uppercase tracking-[0.2em] text-muted-foreground"> - <th className="pb-2 pr-4">Dimension</th> - <th className="pb-2 pr-4">Streamdown (A)</th> - <th className="pb-2 pr-4">A2UI (B)</th> - <th className="pb-2 pr-4">MCP (C)</th> - <th className="pb-2">json-render (D)</th> + <th scope="col" className="pb-2 pr-4">Dimension</th> + <th scope="col" className="pb-2 pr-4">Streamdown (A)</th> + <th scope="col" className="pb-2 pr-4">A2UI (B)</th> + <th scope="col" className="pb-2 pr-4">MCP (C)</th> + <th scope="col" className="pb-2">json-render (D)</th> </tr> </thead>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/ci.yaml.github/workflows/deploy-deno.yml.github/workflows/deploy-staging.ymlAGENTS.mdsrc/ui/src/App.tsxsrc/ui/src/components/ai-elements/prompt-input.tsxsrc/ui/src/components/mermaid-diagram.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/deploy-deno.yml
- AGENTS.md
- .github/workflows/deploy-staging.yml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Include unit tests for all logic changes
Files:
src/ui/src/components/ai-elements/prompt-input.tsxsrc/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsx
**/src/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/ui/**/*.{ts,tsx,jsx,js}: Include Playwright tests for all UI changes
Use Figma for design context; reference design tokens and specs from Figma in UI component files
Files:
src/ui/src/components/ai-elements/prompt-input.tsxsrc/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsx
🧠 Learnings (3)
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/src/ui/**/*.{ts,tsx,jsx,js} : Include Playwright tests for all UI changes
Applied to files:
.github/workflows/ci.yamlsrc/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsx
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/e2e/**/*.{ts,js,spec.ts,spec.js} : Use Playwright for browser automation in UI testing and bug reproduction
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/src/ui/**/*.{ts,tsx,jsx,js} : Use Figma for design context; reference design tokens and specs from Figma in UI component files
Applied to files:
src/ui/src/App.tsxsrc/ui/src/components/mermaid-diagram.tsx
🧬 Code graph analysis (1)
src/ui/src/components/mermaid-diagram.tsx (1)
src/ui/src/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (3)
.github/workflows/ci.yaml (1)
21-21: No action required. Node.js 24 is in Active LTS status as of January 2026 (entered Active LTS on October 28, 2025) and is a stable, production-ready choice that aligns with dependency stability policies.Likely an incorrect or invalid review comment.
src/ui/src/App.tsx (1)
242-390: ProtocolScenarioPreview is well-structured for demo purposes.The switch-based rendering for different protocol types is clear and maintainable. Each case returns properly structured Artifact/Plan components with appropriate styling.
Minor note: The hardcoded content (e.g., "Jordan Lee", "$132,900") is suitable for a bakeoff/demo view but should be documented if this is intended to remain static.
src/ui/src/components/ai-elements/prompt-input.tsx (1)
1044-1061: Good accessibility improvement for aria-label handling.The logic correctly:
- Allows explicit
aria-labeloverride via prop- Omits
aria-labelwhen visible text is present (avoiding redundancy)- Defaults to
"Submit"for icon-only buttonsOne edge case: if
childrenis a React element containing text (e.g.,<span>Submit</span>),hasTextLabelwill befalseandaria-label="Submit"will be set, which is redundant but harmless.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/ui/src/App.tsx
Outdated
| <section id="protocol-bakeoff" className="scroll-mt-24"> | ||
| <div className="space-y-4"> | ||
| <div className="space-y-2"> | ||
| <p className="text-xs uppercase tracking-[0.35em] text-muted-foreground"> | ||
| Protocol Bakeoff | ||
| </p> | ||
| <h2 className="text-2xl font-semibold tracking-tight text-foreground md:text-3xl"> | ||
| Diagrams next to the UI components | ||
| </h2> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Visual flow reference for Streamdown, A2UI, MCP, and | ||
| json-render. | ||
| </p> | ||
| </div> | ||
|
|
||
| <div className="space-y-6"> | ||
| {protocolDiagrams.map((diagram) => ( | ||
| <div | ||
| key={diagram.id} | ||
| className="grid gap-4 lg:grid-cols-[1.15fr_0.85fr]" | ||
| > | ||
| <MermaidDiagram | ||
| code={diagram.code} | ||
| title={diagram.title} | ||
| subtitle={diagram.subtitle} | ||
| theme={theme} | ||
| /> | ||
| <ProtocolScenarioPreview id={diagram.id} /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
|
|
||
| <div className="rounded-none border border-border bg-card p-4"> | ||
| <p className="text-xs font-semibold uppercase tracking-[0.3em] text-muted-foreground"> | ||
| Protocol comparison matrix | ||
| </p> | ||
| <div className="mt-3 overflow-x-auto"> | ||
| <table className="w-full text-sm"> | ||
| <thead> | ||
| <tr className="border-b border-border text-left text-xs uppercase tracking-[0.2em] text-muted-foreground"> | ||
| <th className="pb-2 pr-4">Dimension</th> | ||
| <th className="pb-2 pr-4">Streamdown (A)</th> | ||
| <th className="pb-2 pr-4">A2UI (B)</th> | ||
| <th className="pb-2 pr-4">MCP (C)</th> | ||
| <th className="pb-2">json-render (D)</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {protocolMatrix.map((row) => ( | ||
| <tr | ||
| key={row.dimension} | ||
| className="border-b border-border/60" | ||
| > | ||
| <td className="py-2 pr-4 font-semibold text-foreground"> | ||
| {row.dimension} | ||
| </td> | ||
| <td className="py-2 pr-4 text-muted-foreground"> | ||
| {row.streamdown} | ||
| </td> | ||
| <td className="py-2 pr-4 text-muted-foreground"> | ||
| {row.a2ui} | ||
| </td> | ||
| <td className="py-2 pr-4 text-muted-foreground"> | ||
| {row.mcp} | ||
| </td> | ||
| <td className="py-2 text-muted-foreground"> | ||
| {row.jsonRender} | ||
| </td> | ||
| </tr> | ||
| ))} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </section> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and examine the App.tsx file to verify the section exists
fd -e tsx -e ts -path "*/src/ui/*" -name "App.tsx" | head -5Repository: venikman/PromptAgent
Length of output: 232
🏁 Script executed:
#!/bin/bash
# Once we locate the file, let's examine the lines referenced (768-843)
FILE=$(find . -path "*/src/ui/*" -name "App.tsx" 2>/dev/null | head -1)
if [ -n "$FILE" ]; then
echo "File: $FILE"
wc -l "$FILE"
sed -n '768,843p' "$FILE" | head -50
fiRepository: venikman/PromptAgent
Length of output: 2407
🏁 Script executed:
#!/bin/bash
# Search comprehensively for Playwright test files related to protocol bakeoff
rg -l "protocol.*bakeoff|Protocol.*Bakeoff|protocolDiagrams|protocolMatrix" --type ts --type tsx --type js --type jsx -i . | grep -E "(test|spec|e2e)"Repository: venikman/PromptAgent
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check the overall test structure in src/ui
find src/ui -type f \( -name "*.spec.*" -o -name "*.test.*" -o -name "*.e2e.*" \) 2>/dev/null | head -20Repository: venikman/PromptAgent
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Look for e2e test directory
fd -t d -e "e2e|__tests__|tests" . | grep -v node_modules | head -10Repository: venikman/PromptAgent
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for test files more broadly
find . -type f \( -name "*.spec.ts" -o -name "*.spec.tsx" -o -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.e2e.ts" \) 2>/dev/null | head -30Repository: venikman/PromptAgent
Length of output: 616
🏁 Script executed:
#!/bin/bash
# Search for any file mentioning "protocol" in test/spec context
rg "protocol.*bakeoff|protocolDiagrams|protocolMatrix" -i --glob="*.test.*" --glob="*.spec.*" --glob="*.e2e.*" 2>/dev/nullRepository: venikman/PromptAgent
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if there's a playwright config or test setup
fd -e "playwright" -e "playwright.config" . 2>/dev/null | head -10Repository: venikman/PromptAgent
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Look for e2e or tests directories
find . -type d \( -name "e2e" -o -name "tests" -o -name "__tests__" -o -name "playwright" \) 2>/dev/null | grep -v node_modulesRepository: venikman/PromptAgent
Length of output: 83
🏁 Script executed:
#!/antml/bash
# Search entire codebase for references to protocol bakeoff in any test context
rg "protocol" . -i -l --glob="*.test.*" --glob="*.spec.*" --glob="*.e2e.*" 2>/dev/nullRepository: venikman/PromptAgent
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Check git status/log to see if this is new code
git log --oneline -5 -- src/ui/src/App.tsx 2>/dev/null || echo "Git not available"Repository: venikman/PromptAgent
Length of output: 103
🏁 Script executed:
#!/antml/bash
# Search for TODO/FIXME related to protocol tests
rg "TODO|FIXME" src/ui/src/App.tsx -A 2 -B 2Repository: venikman/PromptAgent
Length of output: 46
🏁 Script executed:
#!/antml/bash
# Check the actual e2e test file to see test pattern
cat tests/e2e/home.spec.ts | head -50Repository: venikman/PromptAgent
Length of output: 395
Add Playwright tests for the protocol-bakeoff section.
The UI implementation is correct: components are properly mapped, theme is passed consistently, and semantic HTML is used appropriately. However, per coding guidelines, all UI changes in src/ui/ require Playwright test coverage. Add e2e tests for the protocol-bakeoff section including diagram rendering, scenario preview interactions, and matrix table visibility/accessibility.
The protocol diagrams and comparison matrix were experimental content that's no longer needed in the main UI. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/ui/src/App.tsx`:
- Line 38: The import of Button in App.tsx is unused and triggers a
no-unused-vars error; remove the unused import statement "import { Button } from
\"./components/ui/button.tsx\";" from App (or alternatively use the Button
component in the JSX if intended) so that the unused symbol is eliminated and
the CI lint error is resolved.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/ui/src/App.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Include unit tests for all logic changes
Files:
src/ui/src/App.tsx
**/src/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/ui/**/*.{ts,tsx,jsx,js}: Include Playwright tests for all UI changes
Use Figma for design context; reference design tokens and specs from Figma in UI component files
Files:
src/ui/src/App.tsx
🧠 Learnings (5)
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/src/ui/**/*.{ts,tsx,jsx,js} : Include Playwright tests for all UI changes
Applied to files:
src/ui/src/App.tsx
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/src/ui/**/*.{ts,tsx,jsx,js} : Use Figma for design context; reference design tokens and specs from Figma in UI component files
Applied to files:
src/ui/src/App.tsx
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/e2e/**/*.{ts,js,spec.ts,spec.js} : Use Playwright for browser automation in UI testing and bug reproduction
Applied to files:
src/ui/src/App.tsx
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Include unit tests for all logic changes
Applied to files:
src/ui/src/App.tsx
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/api/**/*.{ts,tsx,js,jsx} : Include integration tests for all API changes
Applied to files:
src/ui/src/App.tsx
🪛 GitHub Actions: CI
src/ui/src/App.tsx
[error] 38-38: no-unused-vars: Button is never used
🪛 GitHub Check: check
src/ui/src/App.tsx
[failure] 38-38:
�[0m�[1mButton is never used
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Remove unused Button import from App.tsx (fixes CI lint error) - Delete orphaned mermaid-diagram.tsx component (no longer used after protocol bakeoff removal) - Remove mermaid dependency from package.json - Clean up orphaned .mermaid-diagram CSS rules Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Required for `bun install --frozen-lockfile` to work in CI after removing mermaid dependency. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CI uses Bun, so we only need bun.lock. Having both causes Bun to attempt migration from package-lock.json. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Bun 1.1 lockfile format is incompatible with 1.3.x. Upgrading CI to use latest stable Bun (1.3.6). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove npm cache config (package-lock.json no longer exists) - Add setup-bun step to Playwright and deploy workflows - Use bun install --frozen-lockfile consistently Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Bun handles UI dependencies; Node not needed for deploy jobs. Kept Node only in Playwright job where it's required. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Per AGENTS.md: Deno first, Bun second, no Node. Playwright runs via Deno (deno run npm:@playwright/test). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove .NET from CI (no .NET code exists) - Remove Bun, use Deno for UI deps via --node-modules-dir - Remove bun.lock - Single runtime: Deno 2.x Per AGENTS.md: "Deno first" policy, no Node/Bun needed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yaml:
- Line 137: The CI workflow currently sets continue-on-error: true for the
Playwright job (playwright), which lets failures pass silently; either remove
continue-on-error: true from the playwright job to make failures fail the
workflow, or keep it but add a gate check that inspects needs.playwright.result
(e.g., update the gate condition that references needs.playwright to require
needs.playwright.result == 'success') so Playwright failures will block merges
as intended.
🧹 Nitpick comments (2)
.github/workflows/deploy.yaml (2)
141-148: Verify thedeno task buildfallback behavior.The fallback
deno task build || npm run buildmay behave unexpectedly:
- If
deno task buildfails (non-zero exit) rather than being undefined, the npm fallback won't run as intended.- Consider checking if the task exists first (similar to the grep pattern used elsewhere in this file at line 143).
The current pattern at line 143 checks for
"build"in package.json, butdeno task buildwould look for a task indeno.json, notpackage.json.♻️ Suggested approach for consistent task detection
cd "$frontend_root" deno install --node-modules-dir --allow-scripts - if grep -q '"build"' package.json; then - deno task build || npm run build + if grep -q '"build"' deno.json 2>/dev/null; then + deno task build + elif grep -q '"build"' package.json; then + npm run build else echo "No frontend build script configured, skipping" exit 0 fi
269-269: Consider replacing hardcoded sleep with a polling mechanism.A fixed 60-second sleep is fragile—metrics may not be available yet, or the wait may be unnecessarily long. Consider polling with a timeout or using a dedicated readiness check.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yaml.github/workflows/deploy-deno.yml.github/workflows/deploy-staging.yml.github/workflows/deploy.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/deploy-deno.yml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Use Deno as the primary runtime for scripts, tooling, and TypeScript code
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Use Deno as the primary runtime for scripts, tooling, and TypeScript code
Applied to files:
.github/workflows/ci.yaml.github/workflows/deploy.yaml
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Deploy only through GitHub Actions pipelines; no manual production deploys allowed
Applied to files:
.github/workflows/ci.yaml.github/workflows/deploy.yaml
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/e2e/**/*.{ts,js,spec.ts,spec.js} : Use Playwright for browser automation in UI testing and bug reproduction
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Applies to **/src/ui/**/*.{ts,tsx,jsx,js} : Include Playwright tests for all UI changes
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2026-01-15T13:48:27.166Z
Learnt from: CR
Repo: venikman/PromptAgent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T13:48:27.166Z
Learning: Require environment approvals for all production deployments
Applied to files:
.github/workflows/deploy.yaml
🔇 Additional comments (7)
.github/workflows/deploy-staging.yml (1)
24-28: LGTM! Conditional UI dependency installation aligns with Deno-first policy.The conditional check for
src/ui/package.jsonbefore runningdeno installis a good defensive pattern. The--node-modules-dirand--allow-scriptsflags are appropriate for npm compatibility when using Deno to manage Node dependencies. Based on learnings, this follows the Deno-first runtime policy..github/workflows/deploy.yaml (2)
123-126: LGTM! Deno setup correctly configured.The setup uses
denoland/setup-deno@v2with the environment variable for version, maintaining consistency across the workflow. Based on learnings, this aligns with the Deno-first runtime policy.
193-208: Placeholder deployment commands — verify this is intentional.The staging deployment step only echoes status messages without actual deployment commands. This appears to be placeholder code. If Azure deployment is intended but not yet configured, this is acceptable; otherwise, verify this matches the expected behavior for staging deployments.
The
continue-on-error: trueon Azure login (line 191) combined with the conditional skip (lines 205-208) provides graceful degradation when Azure isn't configured..github/workflows/ci.yaml (4)
38-45: LGTM! Cache configuration is appropriate for Deno.The cache paths (
~/.cache/deno,~/.deno) and key pattern usingdeno.lockanddeno.jsonfollow Deno best practices for dependency caching.
47-51: LGTM! Consistent UI dependency installation pattern.This matches the pattern used in deploy-staging.yml and deploy.yaml, ensuring consistent behavior across workflows.
221-231: CI gate doesn't verifyfpf-doctorresult.The gate lists
fpf-doctorinneedsbut doesn't check its result. Whilefpf-doctorhascontinue-on-error: true(line 105), if FPF Doctor checks are informational only, this is fine. Otherwise, add the check.
61-65: LGTM! Lint and format checks properly configured.Using
deno lintanddeno fmt --checkaligns with the Deno-first policy and matches the testing commands referenced in the PR description (deno task lint,deno task fmt:check).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| - name: Run Playwright tests | ||
| run: deno task test:e2e | ||
| continue-on-error: true # May not exist in all projects | ||
| continue-on-error: true |
There was a problem hiding this comment.
Playwright test failures are silently ignored in CI gate.
With continue-on-error: true, Playwright test failures won't fail the job. However, the CI gate (line 226-228) doesn't check needs.playwright.result, so test failures are effectively silent.
If Playwright failures should block merges, either:
- Remove
continue-on-error: true, or - Add
needs.playwright.resultcheck to the gate
If test flakiness is the concern, consider using retry logic instead.
🔧 Option A: Make Playwright failures block CI
- name: Run Playwright tests
run: deno task test:e2e
- continue-on-error: true🔧 Option B: Check Playwright in CI gate
- name: Check all jobs passed
run: |
if [[ "${{ needs.deno.result }}" == "failure" ]] || \
+ [[ "${{ needs.playwright.result }}" == "failure" ]] || \
[[ "${{ needs.security.result }}" == "failure" ]] || \
([[ "${{ github.event_name }}" == "pull_request" ]] && [[ "${{ needs.release-prep.result }}" == "failure" ]]); thenBased on learnings, Playwright tests are required for UI changes per AGENTS.md policy.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| continue-on-error: true | |
| - name: Run Playwright tests | |
| run: deno task test:e2e |
| continue-on-error: true | |
| - name: Check all jobs passed | |
| run: | | |
| if [[ "${{ needs.deno.result }}" == "failure" ]] || \ | |
| [[ "${{ needs.playwright.result }}" == "failure" ]] || \ | |
| [[ "${{ needs.security.result }}" == "failure" ]] || \ | |
| ([[ "${{ github.event_name }}" == "pull_request" ]] && [[ "${{ needs.release-prep.result }}" == "failure" ]]); then |
🤖 Prompt for AI Agents
In @.github/workflows/ci.yaml at line 137, The CI workflow currently sets
continue-on-error: true for the Playwright job (playwright), which lets failures
pass silently; either remove continue-on-error: true from the playwright job to
make failures fail the workflow, or keep it but add a gate check that inspects
needs.playwright.result (e.g., update the gate condition that references
needs.playwright to require needs.playwright.result == 'success') so Playwright
failures will block merges as intended.
- Remove deploy.yaml (echo-only placeholders, real deploys via deploy-deno.yml) - Remove JS-based release-prep job from ci.yaml - Remove 14 unused npm dependencies from package.json Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Testing
deno task lintdeno task fmt:checkRelease Prep
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.