refactor: restructure PM Analysis Agent as a LangGraph subgraph #3018
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
WalkthroughRefactors the workflow into a modular multi-agent design by introducing a PM Agent subgraph and integrating it into the main graph. Removes the class-based PMAnalysisAgent, adds a functional PM analysis invoker, updates graph construction and tests, narrows public exports, and revises documentation to reflect the new PM and DB subgraph architectures. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant pmAgent
participant SaveArtifactTool
participant dbAgent
participant Usecase as generateUsecase
participant DML as prepareDML
participant Validate as validateSchema
participant Finalize as finalizeArtifacts
User->>pmAgent: Messages
pmAgent->>pmAgent: analyzeRequirements (loop w/ retries)
pmAgent->>SaveArtifactTool: invokeSaveArtifactTool (optional, iterative)
pmAgent-->>dbAgent: handoff
dbAgent->>dbAgent: designSchema / invokeSchemaDesignTool
dbAgent-->>Usecase: continue workflow
Usecase-->>DML: generate
DML-->>Validate: validate
Validate-->>Finalize: finalize
Finalize-->>User: Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Updates to Preview Branch (refactor-pm-agent-to-subgraph) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
MH4GF
force-pushed
the
refactor-pm-agent-to-subgraph
branch
from
401db6b to
293e26e
Compare
🤖 Agent Deep Modeling ExecutionStarted at: 2025-08-15 02:42:47 UTC Command Output✅ Status: Completed successfully Finished at: 2025-08-15 02:54:54 UTC |
MH4GF
requested review from
hoshinotsuyoshi,
FunamaYukina,
junkisai and
NoritakaIkeda
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the PM Analysis Agent from a class-based approach to a functional LangGraph subgraph pattern, aligning it with the existing DB Agent architecture for better consistency and maintainability.
- Replaced PMAnalysisAgent class with functional
invokePmAnalysisAgentapproach - Created dedicated PM Agent subgraph with proper node organization
- Updated file structure by moving PM Agent nodes to dedicated directory
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts |
Updated imports and replaced PMAnalysisAgent class usage with invokePmAnalysisAgent function |
frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts |
New functional implementation of PM analysis logic, replacing the class-based approach |
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts |
New subgraph creation function with retry policies and node configuration |
frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/index.ts |
Removed export of deprecated PMAnalysisAgent class |
frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/agent.ts |
Deleted PMAnalysisAgent class implementation |
frontend/internal-packages/agent/src/langchain/agents/index.ts |
Removed PMAnalysisAgent export from agents index |
frontend/internal-packages/agent/src/createGraph.ts |
Integrated PM Agent subgraph into main graph structure |
frontend/internal-packages/agent/src/createGraph.test.ts |
Updated test expectations to reflect new subgraph structure |
frontend/internal-packages/agent/src/chat/workflow/nodes/index.ts |
Removed exports of PM agent nodes from workflow nodes |
frontend/internal-packages/agent/src/chat/workflow/README.md |
Updated documentation to reflect new PM Agent subgraph architecture |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or 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
🔭 Outside diff range comments (1)
frontend/internal-packages/agent/src/chat/workflow/README.md (1)
186-189: Make edge logic reflect subgraph encapsulation and unify node names.The README mixes top-level edge language with internal subgraph node names: the file documents
invokeSaveArtifactTool/saveRequirementsToArtifactToolbut the conditional bullets usesaveRequirementToArtifact. Please scope edges to the pmAgent subgraph and use consistent node/tool names.Locations to update:
- frontend/internal-packages/agent/src/chat/workflow/README.md — Nodes list (≈lines 74–78)
- Node graph/flow (≈lines 92–100)
- invokeSaveArtifactTool node section (≈lines 113–117) — shows "Performed by: saveRequirementsToArtifactTool"
- Conditional Edge Logic block (≈lines 184–189) — currently inconsistent
Proposed replacement diff for the Conditional Edge Logic bullets:
- **analyzeRequirements**: Routes to `saveRequirementToArtifact` when requirements are successfully analyzed, retries `analyzeRequirements` with retry count tracking (max 3 attempts), fallback to `finalizeArtifacts` when max retries exceeded - **saveRequirementToArtifact**: Always routes to `dbAgent` after processing artifacts (workflow termination node pattern) + **pmAgent**: Internally routes between `analyzeRequirements` and `invokeSaveArtifactTool`. On successful analysis it persists artifacts via the `saveRequirementsToArtifactTool`; it retries `analyzeRequirements` up to 3 attempts. On max retries/critical error, control flows to `finalizeArtifacts`. + **invokeSaveArtifactTool (internal)**: After saving, the subgraph exits and the main workflow proceeds to `dbAgent`. - **dbAgent**: DB Agent subgraph handles internal routing between designSchema and invokeSchemaDesignTool nodes, routes to `generateUsecase` on completion - **validateSchema**: Routes to `finalizeArtifacts` on success, `dbAgent` on validation errorPlease apply the naming consistently (use
invokeSaveArtifactToolfor the node andsaveRequirementsToArtifactToolfor the performer) and ensure the diagram/flow text reflects subgraph encapsulation.
🧹 Nitpick comments (5)
frontend/internal-packages/agent/src/createGraph.test.ts (1)
30-36: Test asserts the generated graph exactly matches the expected diagram.Looks good and deterministic for this graph. If you ever see flaky failures due to whitespace differences from upstream LangGraph changes, consider trimming both sides before comparison.
frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts (1)
52-56: Guard against missing additional_kwargs to avoid runtime TypeError.Accessing response.additional_kwargs['reasoning'] can throw if additional_kwargs is undefined. Optional chaining is safer and keeps behavior (safeParse will just fail and return null reasoning).
- const parsedReasoning = v.safeParse( - reasoningSchema, - response.additional_kwargs['reasoning'], - ) + const parsedReasoning = v.safeParse( + reasoningSchema, + response.additional_kwargs?.['reasoning'], + )frontend/internal-packages/agent/src/chat/workflow/README.md (3)
76-82: Tighten node descriptions and fix minor grammar.Clarify “performed by dbAgent” (tautological) and fix “to database” → “to the database”.
-2. **dbAgent**: DB Agent subgraph that handles database schema design - contains designSchema and invokeSchemaDesignTool nodes (performed by dbAgent) +2. **dbAgent**: DB Agent subgraph for database schema design — internal nodes: designSchema and invokeSchemaDesignTool ... -6. **finalizeArtifacts**: Generates and saves comprehensive artifacts to database, handles error timeline items (performed by dbAgentArtifactGen) +6. **finalizeArtifacts**: Generates and saves comprehensive artifacts to the database, and handles error timeline items (performed by dbAgentArtifactGen)
92-103: Replace hard tabs with spaces in PM Agent subgraph diagram (MD010).Same lint concern as the top-level diagram; use spaces for consistent rendering.
- __start__([<p>__start__</p>]):::first - analyzeRequirements(analyzeRequirements) - invokeSaveArtifactTool(invokeSaveArtifactTool) - __end__([<p>__end__</p>]):::last - __start__ --> analyzeRequirements; - invokeSaveArtifactTool --> analyzeRequirements; - analyzeRequirements -.-> invokeSaveArtifactTool; - analyzeRequirements -.-> __end__; - classDef default fill:#f2f0ff,line-height:1.2; - classDef first fill-opacity:0; - classDef last fill:#bfb6fc; + __start__([<p>__start__</p>]):::first + analyzeRequirements(analyzeRequirements) + invokeSaveArtifactTool(invokeSaveArtifactTool) + __end__([<p>__end__</p>]):::last + __start__ --> analyzeRequirements; + invokeSaveArtifactTool --> analyzeRequirements; + analyzeRequirements -.-> invokeSaveArtifactTool; + analyzeRequirements -.-> __end__; + classDef default fill:#f2f0ff,line-height:1.2; + classDef first fill-opacity:0; + classDef last fill:#bfb6fc;
107-118: Align terminology with the refactor (functional API) and fix minor grammar.Replace class-based phrasing and model/version mention with the new functional API; clarify database phrasing.
#### 1. analyzeRequirements Node - **Purpose**: Analyzes and structures user requirements into BRDs -- **Performed by**: PM Analysis Agent with GPT-5 +- **Performed by**: invokePmAnalysisAgent (functional API) - **Retry Policy**: maxAttempts: 3 (internal to subgraph) - **Timeline Sync**: Automatic message synchronization ... #### 2. invokeSaveArtifactTool Node -- **Purpose**: Saves analyzed requirements as artifacts to database -- **Performed by**: saveRequirementsToArtifactTool +- **Purpose**: Saves analyzed requirements as artifacts to the database +- **Performed by**: saveRequirementsToArtifactTool (tool) - **Retry Policy**: maxAttempts: 3 (internal to subgraph) - **Tool Integration**: Direct database artifact storage
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
frontend/internal-packages/agent/src/chat/workflow/README.md(2 hunks)frontend/internal-packages/agent/src/chat/workflow/nodes/index.ts(0 hunks)frontend/internal-packages/agent/src/createGraph.test.ts(1 hunks)frontend/internal-packages/agent/src/createGraph.ts(3 hunks)frontend/internal-packages/agent/src/langchain/agents/index.ts(0 hunks)frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/agent.ts(0 hunks)frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/index.ts(0 hunks)frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts(1 hunks)frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts(1 hunks)frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts(2 hunks)
💤 Files with no reviewable changes (4)
- frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/index.ts
- frontend/internal-packages/agent/src/langchain/agents/index.ts
- frontend/internal-packages/agent/src/chat/workflow/nodes/index.ts
- frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/agent.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Use TypeScript for all components and functions.
Use runtime type validation withvalibotfor API responses and external data.
Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names. Also, event functions should be named with a "handle" prefix, like "handleClick" for onClick and "handleKeyDown" for onKeyDown.
Use consts instead of functions, for example, "const toggle = () =>". Also, define a type if possible.
Follow thetsconfig.jsonpaths settings and always use the correct alias for import paths.
Avoid usingdefault export; always usenamed export.
Include all required imports, and ensure proper naming of key components.
**/*.{ts,tsx}: Use TypeScript for all components and functions.
Avoid using type assertions (askeyword) in TypeScript code.
Use runtime type validation withvalibotinstead of type assertions for API responses and external data.
Use type predicates orinstanceofchecks for DOM element type narrowing.
Follow thetsconfig.jsonpaths settings and always use the correct alias for import paths.
Avoid wrapping standard Supabase.js calls in try-catch blocks unnecessarily; check the error property instead.
**/*.{ts,tsx}: Use runtime type validation with valibot for external data validation
Use early returns for readability
Follow existing import patterns and tsconfig paths
Use consts instead of functions for declarations (e.g., const toggle = () => {})
Files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.tsfrontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.tsfrontend/internal-packages/agent/src/createGraph.test.tsfrontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.tsfrontend/internal-packages/agent/src/createGraph.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.clinerules/general.md)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names. Event functions should be named with a 'handle' prefix, like 'handleClick' for onClick and 'handleKeyDown' for onKeyDown.
Use consts instead of functions (e.g., 'const toggle = () =>'), and define a type if possible.
Avoid usingdefault export; always usenamed export.
Files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.tsfrontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.tsfrontend/internal-packages/agent/src/createGraph.test.tsfrontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.tsfrontend/internal-packages/agent/src/createGraph.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use named exports only (no default exports)
Files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.tsfrontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.tsfrontend/internal-packages/agent/src/createGraph.test.tsfrontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.tsfrontend/internal-packages/agent/src/createGraph.ts
🧠 Learnings (1)
📚 Learning: 2025-08-06T08:16:37.158Z
Learnt from: MH4GF
PR: liam-hq/liam#2926
File: frontend/internal-packages/agent/scripts/executeDesignProcess.ts:42-48
Timestamp: 2025-08-06T08:16:37.158Z
Learning: In frontend/internal-packages/agent/scripts/executeDesignProcess.ts, the checkpointer is intentionally not passed to createDbAgentGraph() because this is scaffolding work - the checkpoint functionality hasn't been implemented yet and will be done in future PRs.
Applied to files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.tsfrontend/internal-packages/agent/src/createGraph.test.tsfrontend/internal-packages/agent/src/createGraph.ts
🧬 Code Graph Analysis (4)
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts (5)
frontend/internal-packages/agent/src/shared/errorHandling.ts (1)
RETRY_POLICY(22-32)frontend/internal-packages/agent/src/chat/workflow/shared/langGraphUtils.ts (1)
createAnnotations(29-63)frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts (1)
analyzeRequirementsNode(13-62)frontend/internal-packages/agent/src/pm-agent/nodes/invokeSaveArtifactToolNode.ts (1)
invokeSaveArtifactToolNode(10-22)frontend/internal-packages/agent/src/pm-agent/routing/routeAfterAnalyzeRequirements.ts (1)
routeAfterAnalyzeRequirements(8-20)
frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts (1)
frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts (1)
invokePmAnalysisAgent(25-63)
frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts (7)
frontend/internal-packages/agent/src/langchain/utils/types.ts (1)
Reasoning(8-8)frontend/internal-packages/agent/src/chat/workflow/types.ts (1)
WorkflowConfigurable(42-48)frontend/internal-packages/agent/src/utils/messageCleanup.ts (1)
removeReasoningFromMessages(60-64)frontend/internal-packages/agent/src/pm-agent/prompts/pmAnalysisPrompts.ts (1)
PM_ANALYSIS_SYSTEM_MESSAGE(5-91)frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts (1)
saveRequirementsToArtifactTool(117-183)frontend/internal-packages/agent/src/langchain/utils/schema.ts (1)
reasoningSchema(3-11)frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/agent.ts (1)
PMAnalysisAgent(21-65)
frontend/internal-packages/agent/src/createGraph.ts (2)
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts (1)
createPmAgentGraph(25-47)frontend/internal-packages/agent/src/db-agent/createDbAgentGraph.ts (1)
createDbAgentGraph(25-47)
🪛 markdownlint-cli2 (0.17.2)
frontend/internal-packages/agent/src/chat/workflow/README.md
11-11: Hard tabs
Column: 1
(MD010, no-hard-tabs)
12-12: Hard tabs
Column: 1
(MD010, no-hard-tabs)
13-13: Hard tabs
Column: 1
(MD010, no-hard-tabs)
14-14: Hard tabs
Column: 1
(MD010, no-hard-tabs)
15-15: Hard tabs
Column: 1
(MD010, no-hard-tabs)
16-16: Hard tabs
Column: 1
(MD010, no-hard-tabs)
17-17: Hard tabs
Column: 1
(MD010, no-hard-tabs)
18-18: Hard tabs
Column: 1
(MD010, no-hard-tabs)
19-19: Hard tabs
Column: 1
(MD010, no-hard-tabs)
20-20: Hard tabs
Column: 1
(MD010, no-hard-tabs)
21-21: Hard tabs
Column: 1
(MD010, no-hard-tabs)
22-22: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1
(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1
(MD010, no-hard-tabs)
100-100: Hard tabs
Column: 1
(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1
(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 LanguageTool
frontend/internal-packages/agent/src/chat/workflow/README.md
[grammar] ~81-~81: There might be a mistake here.
Context: ...rates and saves comprehensive artifacts to database, handles error timeline items ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...onents #### 1. analyzeRequirements Node - Purpose: Analyzes and structures user ...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...ormed by**: PM Analysis Agent with GPT-5 - Retry Policy: maxAttempts: 3 (internal...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...*: maxAttempts: 3 (internal to subgraph) - Timeline Sync: Automatic message synch...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...ion #### 2. invokeSaveArtifactTool Node - Purpose: Saves analyzed requirements a...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...aves analyzed requirements as artifacts to database - Performed by: saveRequir...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...med by**: saveRequirementsToArtifactTool - Retry Policy: maxAttempts: 3 (internal...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...*: maxAttempts: 3 (internal to subgraph) - Tool Integration: Direct database arti...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...(when requirements are fully analyzed) 2. **Iterative Saving**:START → analyzeRequ...
(QB_NEW_EN)
⏰ 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: Supabase Preview
- GitHub Check: Supabase Preview
🔇 Additional comments (13)
frontend/internal-packages/agent/src/createGraph.test.ts (2)
10-22: Mermaid diagram update aligns with new PM Agent subgraph.The replacement of analyzeRequirements/invokeSaveArtifactTool with a single pmAgent node and the updated edges (START → pmAgent, pmAgent → dbAgent) look correct and consistent with the new architecture.
39-44: README diagram consistency check is helpful.Asserting the README contains the expected Mermaid block ensures documentation stays honest with the code.
frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts (3)
34-37: Refactor to functional invokePmAnalysisAgent is clean and improves testability.Dropping the PMAnalysisAgent class usage for a pure function call is simpler and keeps the node lean. Error handling and configuration extraction remain intact.
58-61: Confirm intent to replace messages array with only the latest AI response.Setting messages: [response] discards prior conversation context. This is fine if downstream routing (e.g., routeAfterAnalyzeRequirements) only needs the last AI message, but it will prevent accumulation/history within the subgraph.
If you intended to preserve history, change to appending:
- return { - ...state, - messages: [response], - } + return { + ...state, + messages: [...state.messages, response], + }
18-26: Configurable extraction and error mapping are handled correctly.Good use of getConfigurable and WorkflowTerminationError to halt on bad configuration early.
frontend/internal-packages/agent/src/createGraph.ts (3)
22-27: Embedding pmAgent as a subgraph keeps the top-level workflow concise.The composition pattern mirrors the DB Agent and improves modularity. Node naming is consistent with the tests and README.
41-46: Start and wiring updates are correct.START → pmAgent → dbAgent, then dbAgent → generateUsecase → prepareDML → validateSchema preserves the existing flow order with the new PM Agent entry point.
20-24: Verify checkpointer usage across nested graphs.You pass the same checkpointer into both subgraphs and also compile the top-level graph with it. Depending on LangGraph internals, double-wiring the same checkpointer may be redundant or could lead to unexpected state behavior.
If the intended design is a single checkpointer at the root, consider compiling subgraphs without a checkpointer and only supplying it at the top level.
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts (2)
37-42: Verify exit condition mapping from subgraph to parent.Routing 'dbAgent' → END within the subgraph is correct if the parent graph always connects pmAgent → dbAgent, which it does. Just ensure no other parent edges assume internal pmAgent states.
29-35: Node registration with retry policy looks good.Once unified with shared RETRY_POLICY, this will mirror DB Agent’s resilience profile.
frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts (2)
25-63: Solid functional API with valibot validation and neverthrow ResultAsync.
- Good practice removing prior reasoning from messages and adding a domain-specific system prompt.
- Tools are bound explicitly; response parsing via valibot safeParse avoids throwing.
- Returning ResultAsync<AnalysisWithReasoning, Error> integrates cleanly with node error handling.
41-46: Tool binding config is explicit and consistent with prior agent behavior.parallel_tool_calls: false and tool_choice: 'required' match the intent to force a single, explicit tool call path when needed.
frontend/internal-packages/agent/src/chat/workflow/README.md (1)
121-122: PM Agent flow patterns read clearly and match the subgraph diagram.No issues spotted.
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts (1)
4-4: Shared RETRY_POLICY import aligned with DB Agent — LGTMSwitching to the shared RETRY_POLICY resolves the prior inconsistency and ensures WorkflowTerminationError is not retried, matching DB Agent behavior.
🧹 Nitpick comments (1)
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts (1)
20-21: Nit: prefer lower camelCase for variablesChatStateAnnotation is a value, not a type. Using lower camelCase avoids type/value naming ambiguity.
Apply:
- const ChatStateAnnotation = createAnnotations() - const pmAgentGraph = new StateGraph(ChatStateAnnotation) + const chatStateAnnotation = createAnnotations() + const pmAgentGraph = new StateGraph(chatStateAnnotation)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Use TypeScript for all components and functions.
Use runtime type validation withvalibotfor API responses and external data.
Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names. Also, event functions should be named with a "handle" prefix, like "handleClick" for onClick and "handleKeyDown" for onKeyDown.
Use consts instead of functions, for example, "const toggle = () =>". Also, define a type if possible.
Follow thetsconfig.jsonpaths settings and always use the correct alias for import paths.
Avoid usingdefault export; always usenamed export.
Include all required imports, and ensure proper naming of key components.
**/*.{ts,tsx}: Use TypeScript for all components and functions.
Avoid using type assertions (askeyword) in TypeScript code.
Use runtime type validation withvalibotinstead of type assertions for API responses and external data.
Use type predicates orinstanceofchecks for DOM element type narrowing.
Follow thetsconfig.jsonpaths settings and always use the correct alias for import paths.
Avoid wrapping standard Supabase.js calls in try-catch blocks unnecessarily; check the error property instead.
**/*.{ts,tsx}: Use runtime type validation with valibot for external data validation
Use early returns for readability
Follow existing import patterns and tsconfig paths
Use consts instead of functions for declarations (e.g., const toggle = () => {})
Files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.clinerules/general.md)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names. Event functions should be named with a 'handle' prefix, like 'handleClick' for onClick and 'handleKeyDown' for onKeyDown.
Use consts instead of functions (e.g., 'const toggle = () =>'), and define a type if possible.
Avoid usingdefault export; always usenamed export.
Files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use named exports only (no default exports)
Files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: MH4GF
PR: liam-hq/liam#3018
File: frontend/internal-packages/agent/src/chat/workflow/README.md:11-22
Timestamp: 2025-08-15T02:21:21.491Z
Learning: In the LIAM codebase, Mermaid diagrams in README files (particularly in frontend/internal-packages/agent/src/chat/workflow/README.md) are automatically generated by LangGraph. These diagrams should not be manually edited as it would corrupt the output. Formatting issues like hard tabs (MD010) in these auto-generated Mermaid blocks should not be flagged in code reviews.
📚 Learning: 2025-08-06T08:16:37.158Z
Learnt from: MH4GF
PR: liam-hq/liam#2926
File: frontend/internal-packages/agent/scripts/executeDesignProcess.ts:42-48
Timestamp: 2025-08-06T08:16:37.158Z
Learning: In frontend/internal-packages/agent/scripts/executeDesignProcess.ts, the checkpointer is intentionally not passed to createDbAgentGraph() because this is scaffolding work - the checkpoint functionality hasn't been implemented yet and will be done in future PRs.
Applied to files:
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts
🧬 Code Graph Analysis (1)
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts (5)
frontend/internal-packages/agent/src/chat/workflow/shared/langGraphUtils.ts (1)
createAnnotations(29-63)frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts (1)
analyzeRequirementsNode(13-62)frontend/internal-packages/agent/src/shared/errorHandling.ts (1)
RETRY_POLICY(22-32)frontend/internal-packages/agent/src/pm-agent/nodes/invokeSaveArtifactToolNode.ts (1)
invokeSaveArtifactToolNode(10-22)frontend/internal-packages/agent/src/pm-agent/routing/routeAfterAnalyzeRequirements.ts (1)
routeAfterAnalyzeRequirements(8-20)
⏰ 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). (6)
- GitHub Check: Supabase Preview
- GitHub Check: frontend-lint
- GitHub Check: frontend-ci
- GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
- GitHub Check: agent-deep-modeling
- GitHub Check: Supabase Preview
🔇 Additional comments (3)
frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts (3)
23-29: Consistent retryPolicy on both nodes — LGTMBoth analyzeRequirements and invokeSaveArtifactTool nodes now uniformly use the shared retry policy.
31-36: Conditional routing looks correctMapping 'dbAgent' to END is appropriate for a subgraph and matches routeAfterAnalyzeRequirements return values. This lets the parent graph wire pmAgent → dbAgent cleanly.
19-21: No change needed — PM and DB subgraphs already use the same optional checkpointer patternBoth subgraphs accept an optional BaseCheckpointSaver and the top-level graph passes the parent checkpointer into each — so the parity concern is incorrect.
- frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts — accepts checkpointer and returns:
checkpointer ? pmAgentGraph.compile({ checkpointer }) : pmAgentGraph.compile()- frontend/internal-packages/agent/src/db-agent/createDbAgentGraph.ts — same pattern:
checkpointer ? dbAgentGraph.compile({ checkpointer }) : dbAgentGraph.compile()- frontend/internal-packages/agent/src/createGraph.ts — passes
checkpointerinto both subgraphs (createPmAgentGraph(checkpointer),createDbAgentGraph(checkpointer)) and compiles the top-level graph with it (return checkpointer ? graph.compile({ checkpointer }) : graph.compile())Action: ignore the suggested diff — no changes required here.
Likely an incorrect or invalid review comment.
- Replace PMAnalysisAgent class with functional invokePmAnalysisAgent - Create dedicated PM Agent subgraph with createPmAgentGraph - Move PM Agent nodes to pm-agent/nodes directory for better organization - Add proper retry policies for PM Agent nodes - Simplify main graph by using PM Agent as a subgraph - Update tests and documentation to reflect new architecture This change aligns the PM Agent architecture with the DB Agent pattern, making the codebase more consistent and maintainable. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…solution The tool_choice: 'required' configuration from PR #3013 was accidentally removed during the refactoring. This commit restores it to ensure the PM Analysis Agent always uses tools as intended.
Remove local RETRY_POLICY definition and import shared version from errorHandling module to align with DB Agent and ensure consistent retry behavior including WorkflowTerminationError handling. Co-authored-by: Hirotaka Miyagi <MH4GF@users.noreply.github.com>
MH4GF
force-pushed
the
refactor-pm-agent-to-subgraph
branch
from
1ab66ea to
9b0d4cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/internal-packages/agent/src/chat/workflow/README.md (3)
76-82: Minor grammar/polish in Nodes listTighten phrasing in the finalizeArtifacts description.
Apply this diff:
-6. **finalizeArtifacts**: Generates and saves comprehensive artifacts to database, handles error timeline items (performed by dbAgentArtifactGen) +6. **finalizeArtifacts**: Generates and saves comprehensive artifacts to the database; handles error timeline items (performed by dbAgentArtifactGen)
105-118: Avoid hard-coding a specific model in docs ("GPT-5")Model names change; keeping it generic reduces future doc churn and vendor lock-in signals.
Apply this diff:
-- **Performed by**: PM Analysis Agent with GPT-5 +- **Performed by**: PM Analysis Agent (LLM-backed)
186-190: Fix: “Conditional Edge Logic” references internal pmAgent nodes; align to subgraph boundaries and namingThis section mixes top-level workflow edges with internal pmAgent node names and uses the outdated
saveRequirementToArtifactidentifier. Recommend describing top-level edges and explicitly noting that pmAgent encapsulates its own internal routing and retries.Apply this diff:
-- **analyzeRequirements**: Routes to `saveRequirementToArtifact` when requirements are successfully analyzed, retries `analyzeRequirements` with retry count tracking (max 3 attempts), fallback to `finalizeArtifacts` when max retries exceeded -- **saveRequirementToArtifact**: Always routes to `dbAgent` after processing artifacts (workflow termination node pattern) -- **dbAgent**: DB Agent subgraph handles internal routing between designSchema and invokeSchemaDesignTool nodes, routes to `generateUsecase` on completion -- **validateSchema**: Routes to `finalizeArtifacts` on success, `dbAgent` on validation error +- **pmAgent**: Internally routes between `analyzeRequirements` and `invokeSaveArtifactTool` with retry policy (max 3 attempts). On successful completion, routes to `dbAgent`. Falls back to `finalizeArtifacts` on unrecoverable errors. +- **dbAgent**: Subgraph handles internal routing between `designSchema` and `invokeSchemaDesignTool`; routes to `generateUsecase` on completion. +- **validateSchema**: Routes to `finalizeArtifacts` on success, or back to `dbAgent` on validation error.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
frontend/internal-packages/agent/src/chat/workflow/README.md(2 hunks)frontend/internal-packages/agent/src/chat/workflow/nodes/index.ts(0 hunks)frontend/internal-packages/agent/src/createGraph.test.ts(1 hunks)frontend/internal-packages/agent/src/createGraph.ts(3 hunks)frontend/internal-packages/agent/src/langchain/agents/index.ts(0 hunks)frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/agent.ts(0 hunks)frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/index.ts(0 hunks)frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts(1 hunks)frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts(1 hunks)frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts(2 hunks)
💤 Files with no reviewable changes (4)
- frontend/internal-packages/agent/src/chat/workflow/nodes/index.ts
- frontend/internal-packages/agent/src/langchain/agents/index.ts
- frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/index.ts
- frontend/internal-packages/agent/src/langchain/agents/pmAnalysisAgent/agent.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/internal-packages/agent/src/pm-agent/createPmAgentGraph.ts
- frontend/internal-packages/agent/src/pm-agent/nodes/analyzeRequirementsNode.ts
- frontend/internal-packages/agent/src/createGraph.test.ts
- frontend/internal-packages/agent/src/createGraph.ts
- frontend/internal-packages/agent/src/pm-agent/invokePmAnalysisAgent.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: MH4GF
PR: liam-hq/liam#3018
File: frontend/internal-packages/agent/src/chat/workflow/README.md:11-22
Timestamp: 2025-08-15T02:21:21.491Z
Learning: In the LIAM codebase, Mermaid diagrams in README files (particularly in frontend/internal-packages/agent/src/chat/workflow/README.md) are automatically generated by LangGraph. These diagrams should not be manually edited as it would corrupt the output. Formatting issues like hard tabs (MD010) in these auto-generated Mermaid blocks should not be flagged in code reviews.
📚 Learning: 2025-08-15T02:21:21.491Z
Learnt from: MH4GF
PR: liam-hq/liam#3018
File: frontend/internal-packages/agent/src/chat/workflow/README.md:11-22
Timestamp: 2025-08-15T02:21:21.491Z
Learning: In the LIAM codebase, Mermaid diagrams in README files (particularly in frontend/internal-packages/agent/src/chat/workflow/README.md) are automatically generated by LangGraph. These diagrams should not be manually edited as it would corrupt the output. Formatting issues like hard tabs (MD010) in these auto-generated Mermaid blocks should not be flagged in code reviews.
Applied to files:
frontend/internal-packages/agent/src/chat/workflow/README.md
📚 Learning: 2025-07-29T09:15:54.413Z
Learnt from: MH4GF
PR: liam-hq/liam#2753
File: frontend/internal-packages/agent/src/chat/workflow/README.md:106-109
Timestamp: 2025-07-29T09:15:54.413Z
Learning: In the LIAM codebase, HTML entities like ` ` in Mermaid diagram edge labels are intentionally kept as-is, even though they may render as literal text rather than parsed entities. The user MH4GF has confirmed this syntax should remain unchanged in the README documentation.
Applied to files:
frontend/internal-packages/agent/src/chat/workflow/README.md
🪛 markdownlint-cli2 (0.17.2)
frontend/internal-packages/agent/src/chat/workflow/README.md
11-11: Hard tabs
Column: 1
(MD010, no-hard-tabs)
12-12: Hard tabs
Column: 1
(MD010, no-hard-tabs)
13-13: Hard tabs
Column: 1
(MD010, no-hard-tabs)
14-14: Hard tabs
Column: 1
(MD010, no-hard-tabs)
15-15: Hard tabs
Column: 1
(MD010, no-hard-tabs)
16-16: Hard tabs
Column: 1
(MD010, no-hard-tabs)
17-17: Hard tabs
Column: 1
(MD010, no-hard-tabs)
18-18: Hard tabs
Column: 1
(MD010, no-hard-tabs)
19-19: Hard tabs
Column: 1
(MD010, no-hard-tabs)
20-20: Hard tabs
Column: 1
(MD010, no-hard-tabs)
21-21: Hard tabs
Column: 1
(MD010, no-hard-tabs)
22-22: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1
(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
99-99: Hard tabs
Column: 1
(MD010, no-hard-tabs)
100-100: Hard tabs
Column: 1
(MD010, no-hard-tabs)
101-101: Hard tabs
Column: 1
(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 LanguageTool
frontend/internal-packages/agent/src/chat/workflow/README.md
[grammar] ~81-~81: There might be a mistake here.
Context: ...rates and saves comprehensive artifacts to database, handles error timeline items ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...onents #### 1. analyzeRequirements Node - Purpose: Analyzes and structures user ...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...ormed by**: PM Analysis Agent with GPT-5 - Retry Policy: maxAttempts: 3 (internal...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...*: maxAttempts: 3 (internal to subgraph) - Timeline Sync: Automatic message synch...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...ion #### 2. invokeSaveArtifactTool Node - Purpose: Saves analyzed requirements a...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...med by**: saveRequirementsToArtifactTool - Retry Policy: maxAttempts: 3 (internal...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...*: maxAttempts: 3 (internal to subgraph) - Tool Integration: Direct database arti...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...(when requirements are fully analyzed) 2. **Iterative Saving**:START → analyzeRequ...
(QB_NEW_EN)
⏰ 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). (6)
- GitHub Check: Supabase Preview
- GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
- GitHub Check: frontend-lint
- GitHub Check: frontend-ci
- GitHub Check: agent-deep-modeling
- GitHub Check: Supabase Preview
🔇 Additional comments (3)
frontend/internal-packages/agent/src/chat/workflow/README.md (3)
11-23: Architecture diagram update aligns with new pmAgent entry and routingThe top-level flow now starts at pmAgent and routes to dbAgent, matching the subgraph refactor intent.
83-103: PM Agent subgraph section reads clearly and matches the refactorDiagrams and section structure accurately present analyzeRequirements and invokeSaveArtifactTool as internal pmAgent nodes with subgraph boundaries.
119-123: PM Agent flow patterns look correct and consistent with subgraph designThe Simple and Iterative patterns reflect internal pmAgent routing without leaking into the top-level graph.
Issue
Why is this change needed?
This refactoring restructures the PM Analysis Agent to follow the same subgraph pattern as the DB Agent, improving codebase consistency and maintainability.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit