♻️(app,agent): Refactor analyzed requirements extraction and streaming #3744
Conversation
|
Unexpected error on running flow
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Updates to Preview Branch (refactor_analyzed_requirement_extraction) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
WalkthroughReplaces artifact-centric flows with analyzedRequirements across frontend and agent layers: adds getAnalyzedRequirements and SSE event ANALYZED_REQUIREMENTS, updates useStream to handle it, threads analyzedRequirements into SessionDetailPage and Output, removes realtime artifact hook and getArtifact repository methods, and updates PM tool to emit analyzedRequirements. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Server as SessionDetailPage (Server)
participant Client as SessionDetailPageClient
participant Stream as useStream (SSE)
participant Agent as Agent/PM Tool
participant Output
User->>Server: GET session page
Server->>Agent: getAnalyzedRequirements(config)
Agent-->>Server: analyzedRequirements | null
Server-->>Client: initialMessages, initialAnalyzedRequirements
rect rgba(230,245,255,0.5)
Note over Client,Stream: Streaming phase (SSE)
Client->>Stream: connect (initial state + analyzedRequirements)
Agent-->>Stream: SSE MESSAGES
Agent-->>Stream: SSE ANALYZED_REQUIREMENTS
Stream-->>Client: messages update / analyzedRequirements update
end
Client->>Output: render with analyzedRequirements, versions
Output->>Output: format analyzedRequirements -> markdown
Output-->>User: render tabs and content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Check changeset necessityStatus: NOT REQUIRED Reason:
Changeset (copy & paste):<!-- No changeset required for this PR (only @liam-hq/app and @liam-hq/agent were changed) --> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
frontend/internal-packages/agent/src/getAnalyzedRequirements.ts (1)
66-85: Redundant null check after continue.Line 68 continues if
!childState, so the subsequent checkchildState && ...on line 70 is redundant.Apply this diff to simplify:
for (const task of tasks) { const childState = task.state if (!childState) continue if ( - childState && typeof childState === 'object' && 'values' in childState && 'tasks' in childState ) {frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts (2)
90-96: Message copy is outdated (no artifact persistence anymore)The content still says “saved successfully to artifact.” Update to reflect the analyzed requirements streaming flow.
- content: 'Requirements saved successfully to artifact', + content: 'Analyzed requirements ready',
102-108: Rename artifactSaveSuccessful to avoid confusionYou’re no longer saving artifacts here. Consider renaming to analyzedRequirementsAvailable (or remove if unused) to better reflect semantics. This may touch consumers, so treat as follow‑up refactor.
Can you confirm whether artifactSaveSuccessful is still read downstream? If not, I can draft a cleanup PR to remove/rename it.
frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx (1)
12-21: Switch to analyzedRequirements looks correctData flow and markdown conversion are consistent with the new model.
Small UX nit: align empty state text with new terminology.
- if (!analyzedRequirements) { - return <div>No artifact available yet</div> - } + if (!analyzedRequirements) { + return <div>No analyzed requirements yet</div> + }frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
166-189: Optional: stop processing after ERROR eventYou set isStreaming=false on ERROR but keep iterating. Consider early break to avoid processing further chunks in an errored session.
- if (ev.event === SSE_EVENTS.ERROR) { - handleErrorEvent(ev) - return 'error' - } + if (ev.event === SSE_EVENTS.ERROR) { + handleErrorEvent(ev) + return 'error' + }And in the loop:
- if (result === 'error') { - } + if (result === 'error') { + break + }Also applies to: 193-205
frontend/internal-packages/agent/src/getAnalyzedRequirements.test.ts (2)
1-192: Comprehensive test coverage for extraction utilities.The test suite thoroughly covers edge cases for
extractAnalyzedRequirementsFromStateandcollectAnalyzedRequirementsFromTasks, including null handling, invalid structures, and recursive traversal.Consider adding a test for the main
getAnalyzedRequirementsfunction, which is the primary public entry point. While the current tests cover the helper utilities well, an integration test would verify the end-to-end graph building and state fetching flow. This could be deferred if the graph dependencies make mocking complex.
30-30: Minor: deterministic IDs preferred over Math.random().Using
Math.random()for test IDs is acceptable but not ideal. Consider using a counter or fixed strings for more predictable test behavior.- id: `task-${Math.random()}`, + id: `task-${Date.now()}-${Math.random()}`,or
+let taskIdCounter = 0 + const createMockTask = ( state: StateSnapshot, ): StateSnapshot['tasks'][number] => ({ state, - id: `task-${Math.random()}`, + id: `task-${++taskIdCounter}`, name: 'test-task', interrupts: [], })frontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsx (1)
61-63: Optional: consider parallelizing independent async operations.Lines 61-63 await three independent operations sequentially. While this follows the existing pattern in the codebase, you could improve page load time by fetching them in parallel.
- const baseMessages = await getMessages(config) - const messages = serializeMessages(baseMessages) - const initialAnalyzedRequirements = await getAnalyzedRequirements(config) + const [baseMessages, initialAnalyzedRequirements] = await Promise.all([ + getMessages(config), + getAnalyzedRequirements(config), + ]) + const messages = serializeMessages(baseMessages)Note: This suggestion applies to the broader function pattern, not just this change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx(1 hunks)frontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsx(6 hunks)frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx(5 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx(5 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/hooks/useRealtimeArtifact.ts(0 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsx(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsx(4 hunks)frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts(5 hunks)frontend/internal-packages/agent/src/getAnalyzedRequirements.test.ts(1 hunks)frontend/internal-packages/agent/src/getAnalyzedRequirements.ts(1 hunks)frontend/internal-packages/agent/src/index.ts(1 hunks)frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts(2 hunks)frontend/internal-packages/agent/src/repositories/InMemoryRepository.ts(0 hunks)frontend/internal-packages/agent/src/repositories/supabase.ts(0 hunks)frontend/internal-packages/agent/src/repositories/types.ts(0 hunks)frontend/internal-packages/agent/src/streaming/constants.ts(1 hunks)
💤 Files with no reviewable changes (4)
- frontend/internal-packages/agent/src/repositories/types.ts
- frontend/internal-packages/agent/src/repositories/InMemoryRepository.ts
- frontend/internal-packages/agent/src/repositories/supabase.ts
- frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/hooks/useRealtimeArtifact.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/internal-packages/agent/src/streaming/constants.tsfrontend/internal-packages/agent/src/index.tsfrontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.tsfrontend/internal-packages/agent/src/getAnalyzedRequirements.tsfrontend/internal-packages/agent/src/getAnalyzedRequirements.test.tsfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Use runtime type validation with valibot for external data validation
Prefer early returns for readability
Write simple, direct code without backward compatibility shims; update all call sites together
Use const-assigned arrow functions instead of function declarations for small utilities (e.g., const toggle = () => {})
Follow existing import patterns and tsconfig path aliases
Files:
frontend/internal-packages/agent/src/streaming/constants.tsfrontend/internal-packages/agent/src/index.tsfrontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsxfrontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.tsfrontend/internal-packages/agent/src/getAnalyzedRequirements.tsfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/internal-packages/agent/src/getAnalyzedRequirements.test.tsfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx
frontend/internal-packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Infra and tooling (e2e, configs, storybook, agent) live under frontend/internal-packages
Files:
frontend/internal-packages/agent/src/streaming/constants.tsfrontend/internal-packages/agent/src/index.tsfrontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.tsfrontend/internal-packages/agent/src/getAnalyzedRequirements.tsfrontend/internal-packages/agent/src/getAnalyzedRequirements.test.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
**/*.tsx: Prefix React event handler functions with "handle" (e.g., handleClick)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Files:
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsxfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsxfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx
**/!(page).tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use named exports only (no default exports) for React/TSX modules
Files:
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsxfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests with filenames ending in .test.ts or .test.tsx colocated near source
Files:
frontend/internal-packages/agent/src/getAnalyzedRequirements.test.ts
🧠 Learnings (1)
📚 Learning: 2025-09-12T11:37:37.587Z
Learnt from: NoritakaIkeda
PR: liam-hq/liam#3463
File: frontend/internal-packages/agent/src/qa-agent/testcaseGeneration/saveToolNode.ts:12-22
Timestamp: 2025-09-12T11:37:37.587Z
Learning: In the frontend/internal-packages/agent codebase, state.currentRequirement.requirementId is guaranteed to always be present, so guard checks for missing requirementId are unnecessary in saveToolNode.ts.
Applied to files:
frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts
🧬 Code graph analysis (9)
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsx (3)
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/VersionDropdown/VersionDropdown.tsx (1)
VersionDropdown(22-73)frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/utils/formatArtifactToMarkdown.ts (1)
formatArtifactToMarkdown(53-98)
frontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsx (2)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/internal-packages/agent/src/index.ts (1)
getAnalyzedRequirements(8-8)
frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts (1)
frontend/internal-packages/agent/src/streaming/constants.ts (1)
SSE_EVENTS(1-6)
frontend/internal-packages/agent/src/getAnalyzedRequirements.ts (2)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/internal-packages/agent/src/types.ts (1)
WorkflowConfigurable(13-19)
frontend/internal-packages/agent/src/getAnalyzedRequirements.test.ts (2)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/internal-packages/agent/src/getAnalyzedRequirements.ts (2)
extractAnalyzedRequirementsFromState(32-52)collectAnalyzedRequirementsFromTasks(60-89)
frontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx (2)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx (1)
ArtifactContainer(12-21)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (3)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/types.ts (1)
Version(10-13)frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
useStream(73-361)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (3)
frontend/internal-packages/artifact/src/schemas/artifact.ts (2)
AnalyzedRequirements(30-32)analyzedRequirementsSchema(25-28)frontend/internal-packages/agent/src/streaming/constants.ts (1)
SSE_EVENTS(1-6)frontend/apps/app/components/SessionDetailPage/hooks/useStream/parseSse.ts (1)
parseSse(7-68)
frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx (3)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/utils/formatArtifactToMarkdown.ts (1)
formatArtifactToMarkdown(53-98)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/Artifact.tsx (1)
Artifact(37-218)
⏰ 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). (5)
- GitHub Check: frontend-ci
- GitHub Check: frontend-lint
- GitHub Check: Supabase Preview
- GitHub Check: security-review
- GitHub Check: agent-deep-modeling
🔇 Additional comments (12)
frontend/internal-packages/agent/src/index.ts (1)
8-8: LGTM!The export follows the established pattern in this barrel file and properly exposes the new
getAnalyzedRequirementsutility.frontend/internal-packages/agent/src/streaming/constants.ts (1)
5-5: LGTM!The new event constant follows the existing naming convention and extends the SSE event types appropriately.
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsx (1)
67-94: LGTM!The story data correctly uses the
AnalyzedRequirementsstructure instead of the previousartifactDocstring, maintaining realistic test data for the component.frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsx (3)
20-27: LGTM!The prop signature change correctly replaces
artifactDocwithanalyzedRequirements, moving the formatting responsibility into this component.
66-69: LGTM!Deriving
artifactDocfromanalyzedRequirementswithin the component is clean and maintains the existing interface for downstream components likeExportDropdown.
99-99: No change needed:disabled={!artifactDoc}is safe.formatArtifactToMarkdownalways produces non-empty markdown (it unconditionally pushes header and goal), soartifactDocis truthy when present.frontend/internal-packages/agent/src/getAnalyzedRequirements.ts (1)
97-119: Handle errors fromcreateGraphandgraph.getState
Wrap these calls intry/catchor attach a.catch()handler to avoid unhandled promise rejections.frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx (1)
98-98: Prop wiring LGTMPassing initialAnalyzedRequirements={null} aligns with the new streaming-first flow.
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
352-355: Return shape update LGTMExposing analyzedRequirements from the hook is correct and matches downstream consumers.
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (1)
36-55: Analyzed requirements integration looks solid
- Initial tab derivation and switch-on-arrival behavior are sensible.
- Output/Chat wiring and hook usage align with the new stream flow.
Also applies to: 70-76, 102-113, 114-123, 126-128, 206-207
frontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx (1)
3-3: LGTM! Clean refactor from artifact to analyzedRequirements.The props and data flow are correctly updated throughout the component. The type definition with
AnalyzedRequirements | nulland the optional prop correctly handle the case where analyzed requirements may not yet be available.Also applies to: 28-28, 39-39, 78-78, 94-94
frontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsx (1)
4-4: LGTM! Proper integration of analyzed requirements.The refactor correctly replaces artifact fetching with
getAnalyzedRequirements. The data flow is sound: fetch from agent, return in Result type, pass to client component. The type system properly represents the nullable nature of analyzed requirements.Also applies to: 8-8, 40-40, 63-63, 98-98, 119-119, 142-142
| const handleAnalyzedRequirementsEvent = useCallback( | ||
| (ev: { data: string }) => { | ||
| const parsedData = JSON.parse(ev.data) | ||
| const [serialized] = parsedData | ||
| const result = safeParse(analyzedRequirementsSchema, serialized) | ||
| if (result.success) { | ||
| setAnalyzedRequirements(result.output) | ||
| } | ||
| }, | ||
| [], | ||
| ) |
There was a problem hiding this comment.
Harden ANALYZED_REQUIREMENTS parsing (avoid crashes and accept both shapes)
Uncaught JSON.parse throws will break streaming; also you assume array‑wrapped payload. Make parsing defensive and accept object or [object].
- const handleAnalyzedRequirementsEvent = useCallback(
- (ev: { data: string }) => {
- const parsedData = JSON.parse(ev.data)
- const [serialized] = parsedData
- const result = safeParse(analyzedRequirementsSchema, serialized)
- if (result.success) {
- setAnalyzedRequirements(result.output)
- }
- },
- [],
- )
+ const handleAnalyzedRequirementsEvent = useCallback(
+ (ev: { data: string }) => {
+ try {
+ const parsed = JSON.parse(ev.data)
+ const payload = Array.isArray(parsed) ? parsed[0] : parsed
+ const result = safeParse(analyzedRequirementsSchema, payload)
+ if (result.success) setAnalyzedRequirements(result.output)
+ } catch {
+ // Ignore malformed event; keep stream alive
+ }
+ },
+ [],
+ )🤖 Prompt for AI Agents
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts
around lines 149 to 159: the handler assumes JSON.parse always succeeds and that
the payload is an array containing the object; wrap the parse in a try/catch to
avoid uncaught exceptions, then normalize the parsed value to accept either an
object or an array (if array, take the first element) before passing it to
safeParse, and if parsing/validation fails simply bail out or log the error
instead of throwing so streaming continues.
| if (!('testcases' in value) || typeof value.testcases !== 'object') { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Type guard allows null testcases.
typeof null === 'object' in JavaScript, so the type guard will accept { goal: 'x', testcases: null } as valid. This could lead to runtime errors downstream.
Apply this diff to fix the type guard:
- if (!('testcases' in value) || typeof value.testcases !== 'object') {
+ if (!('testcases' in value) || typeof value.testcases !== 'object' || value.testcases === null) {
return false
}📝 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.
| if (!('testcases' in value) || typeof value.testcases !== 'object') { | |
| return false | |
| } | |
| if (!('testcases' in value) || typeof value.testcases !== 'object' || value.testcases === null) { | |
| return false | |
| } |
🤖 Prompt for AI Agents
In frontend/internal-packages/agent/src/getAnalyzedRequirements.ts around lines
20 to 22, the current type guard uses "typeof value.testcases !== 'object'"
which still treats null as an object; update the condition to explicitly exclude
null (e.g. check value.testcases !== null && typeof value.testcases === 'object'
or use value.testcases && typeof value.testcases === 'object') so null testcases
are rejected and the function returns false for { testcases: null }.
| await dispatchCustomEvent( | ||
| SSE_EVENTS.ANALYZED_REQUIREMENTS, | ||
| analyzedRequirements, | ||
| ) |
There was a problem hiding this comment.
Ensure SSE payload shape matches client parsing
useStream currently does JSON.parse(ev.data) then const [serialized] = parsedData. If dispatchCustomEvent doesn’t array‑wrap, this will be undefined and can crash.
Emit an array-wrapped payload to match the client’s expectation.
- await dispatchCustomEvent(
- SSE_EVENTS.ANALYZED_REQUIREMENTS,
- analyzedRequirements,
- )
+ await dispatchCustomEvent(
+ SSE_EVENTS.ANALYZED_REQUIREMENTS,
+ [analyzedRequirements],
+ )Alternatively, relax the client to accept both array and object payloads (see comment in useStream.ts).
🤖 Prompt for AI Agents
In
frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts
around lines 97 to 100, the SSE payload currently dispatches
analyzedRequirements directly but the client (useStream) expects an array and
does JSON.parse(ev.data) then const [serialized] = parsedData; wrap the emitted
payload in an array so the client receives [analyzedRequirements] (or otherwise
serialize an array containing the object) to avoid serialized being undefined
and potential crashes.
🤖 Agent Deep Modeling ExecutionStarted at: 2025-10-10 08:28:15 UTC Command Output
RUN v3.2.4 /home/runner/work/liam/liam/frontend/internal-packages/agent (node:7626) ExperimentalWarning: WASI is an experimental feature and might change at any time ✅ [INFO] 2025-10-10T08:28:18.168Z 🧠 Thinking (pm)... Structuring test cases for database QA I need to create an object with categorized keys like Users, Roles & Permissions, and Authentication. Each category will list test case titles and their types, primarily focusing on SQL operations. For example, authentication could verify credentials with SELECT commands, while other scenarios must confirm constraints like duplicate emails failing and roles in many-to-many relationships. I’ll ensure negative scenarios are included and save these requirements after my analysis with a clear summary of the goal. Constructing the BRD for user management I need to outline the final Business Requirements Document (BRD) for a simple user management system, which will include features like user registration, login, profile management, role-based access control, password resets, email verification, and admin user management. I'll define the user entity with attributes such as id, username, email, phone, password details, verification status, and login timestamps. There are additional fields for managing user roles and activities. Once complete, I will utilize the appropriate tool to save it. Defining roles, permissions, and session details I’m laying out details for roles and permissions in the user management system. Each role will have attributes like id, name, and description, while permissions will include unique keys and descriptions, along with timestamps and a system flag. I'll include junction tables for role-permissions and user-roles, as well as session details such as token types and expiration. Audit logs will track actions, and verification codes will validate user requests, ensuring constraints like email format, username uniqueness, and admin management. Outlining workflows and security measures I'm detailing workflows for the user management system. The registration process involves signing up with email and password, setting the user status to PENDING_VERIFICATION, and sending an email verification token. Once verified, the status changes to ACTIVE. For admin invited users, I’ll create them with an INVITED status and send an invite. Login is restricted to ACTIVE users with verified emails only. Password resets will use temporary tokens, and I'll enforce strict role management to avoid duplicates. I need to ensure secure storage of passwords using hashing algorithms while considering non-functional requirements like performance for up to 100k users. Creating test case categories and structure I'm organizing test cases into categories such as User Registration, Email Verification, Authentication, Profile Management, and more. Each category will contain specific test cases focused on SQL operations, like checking for unique emails, creating tokens, or verifying user logins. I'll ensure there's a clear structure including a Business Goal, Scope, and Functional Requirements, along with constraints and validation rules. I also need to mention data types, indexing, and soft delete policies. Finally, I’ll prepare to save this structured information using the appropriate tool with a concise purpose and inputs. Compiling the BRD Alright, it's time to put together the Business Requirements Document (BRD). I’ll clarify that we didn't need to use web search previews, as it wasn’t necessary for this process. I'll acknowledge that the database schema is currently empty, but we can still design it logically based on the requirements we've gathered. It's important to finalize this document to ensure clear guidelines for building the system. Let’s make sure everything is organized and ready to go! ⏺ (pm): Planned actions:
Business Requirements Document (BRD)
Testcases (for QA; business-focused, mapped to SQL operations)
Preparing to save artifact: purpose = persist BRD goal and categorized testcases; inputs = goal summary and structured testcases. 🔧 saveRequirementsToArtifactTool() ⎿ unknown: Requirements saved successfully to artifact 🔧 schemaDesignTool() 🧠 Thinking (db)... Preparing database modifications I need to consider all modifications. The developer emphasized using the tool for any creation needed. Since I need to create tables, I’ll call the tool to add all tables again, but I must fix I’ll also correct 🔧 schemaDesignTool() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { ⏺ (qa): { 🔧 saveTestcase() email_verifications token that is expired.\n-- 2ifications\n) Ensure the user is not yet active.\n\n-- token as consumed,-- We perform set the user's an verified update that_at ⏺ (qa): { 🔧 saveTestcase() attempts ensure to consume token cannot be reused.\ ann " expired token\n-- Assumptions:\sql and verifyn--": that no changes to - "-- user status token is unique 🔧 saveTestcase() in email_ver Testifications.token occur and token\n-- - We remains un verify againstconsumed a specific.\ user_idn\n-- Case 🔧 saveTestcase() 🔧 saveTestcase() Step 1: Create test 🔧 saveTestcase() 🔧 saveTestcase() user (pending 🔧 saveTestcase(): Change) and an expired verification token\nWITH test_user AS (\n INSERT INTO users (id, email, email_lc, status, created password with correct current password updates hash and timestamp\n-- Category: Password Management\n-- Title: ⏺ (qa): { 🔧 saveTestcase() and token value (placeholders below to be replaced in tests).\n-- - The operation is performed within a transaction to ensure atomicity.\n\n-- Input placeholders (to be replaced by test harness):\n-- :in_user_id UUID of the user being verified\n-- :in_token Text token value to verify\n\n 🔧 saveTestcase() 🔧 saveTestcase()_at, password_hash, verified_at)\n VALUES (gen_random_uuid(), '[email protected]', LOWER('[email protected]'), 'pending', NOW(), NULL, NULL)\n RETURNING id\n), test_token AS (\n INSERT INTO email_verifications (id, token, user_id, created_at, expires_at, consumed_at)\n VALUES (gen_random_uuid(), 'EXPIRED_TOKEN_ABC123', (SELECT id FROM test Change password with correct current password updates hash and timestamp\n-- Type: UPDATE\n\n-- Assumptions:\n-- - We identify the target user by id (parameter: p_user_id)\n-- - We receive the current (old) password (could be null for system actions)\n-- - Metadata includes: role_id, user_id, and reason for assignment; adjust as needed by your app\n\nWITH ins_actor AS (\n -- Example: actor who performed the grant; replace with actual UUID in your environment\n SELECT left(gen_random_uuid()::text, 36)::uuid AS actor_id\n), ins_target AS (\n -- Target user receiving the role; replace with actual user_id in your environment\n SELECT left(gen_random_uuid()::text, 36)::uuid AS target_user_id\n), ins_audit AS (\n INSERT INTO audits (\n id,\n action,\n success,\n actor_id,\n metadata,\n created_at,\n target_user_id\n ) VALUES (\n gen_random_uuid(), -- id\nBEGIN;\n\n-- 1) Ensure the token exists, is not consumed, and has not expired\n-- We fetch the token row for locking in update to avoid race conditions.\nSELECT id, user_id, expires_at, consumed_at\nFROM email_verifications\nWHERE user_id = :in_user_id\n AND token = :in_token\n FOR UPDATE;\n\n-- If no row returned, raise a notice (then rollback in tests). We implement as conditional control.\nDO $$\nBEGIN\n IF NOT EXISTS (\n SELECT 1\n FROM email_verifications\n WHERE user_id = :in_user_id AND token = :in_token\n AND consumed_at IS NULL\n ) THEN\n RAISE EXCEPTION 'Email verification token not found or already consumed';\n END IF;\nEND\n$$;\n\n-- 2) Perform the actual update atomically:\n-- - Mark token as consumed (consumed_at = now())\n-- - Set user's verified_at = now()\n-- - Update password reset or audit as needed (audit handled separately if required)\nUPDATE email_verifications\nSET consumed_at = NOW()\nWHERE user_id = :in_user_id\n AND token = :in_token\n AND consumed_at IS NULL;\ 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase()_user), NOW() - INTERVAL '2 days', NOW() - INTERVAL '1 day', NULL)\n RETURNING user_id, expires_at\n)\nSELECT 1;\n\n-- Step 2: Attempt to verify using the expired token (should not activate the account)\nUPDATE users u\nSET status = u.status,\n verified_at = NULL\nFROM email_verifications ev\nWHERE ev.user_id = u.id\n AND ev.token = 'EXPIRED_TOKEN_ABC123'\n AND ev.expires_at > NOW()\n -- Only update if token is not expired\n \n-- The above attempted update will have no rows affected when token is expired.\n-- To demonstrate the intended behavior, we simulate verification attempt with an expired token\n-- by explicitly performing a check that ensures expired tokens do not activate accounts.\n\n-- Step 3: Verify that no update occurred to the user's status to 'active' and that token remains unconsumed\nWITH try_expired_verify AS (\n UPDATE email_verifications\n SET consumed_at = consumed_at\n WHERE token = 'EXPIRED_TOKEN_ABC123'\n RETURNING 1\n)\nSELECT\n (SELECT status FROM users WHERE email_lc = '[email protected]') AS user_status,\n (SELECT consumed_at IS NULL FROM email_verifications WHERE token = 'EXPIRED_TOKEN_ABC123') AS token_not_consumed\n;\n" ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase()(node:7626) Warning: The 'import-in-the-middle' hook has already been initialized 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() true,\n NULL, -- actor_id could be set if the operation is performed by an authenticated user\n jsonb_build_object('field','profile','before','', 'after', jsonb_build_object('display_name', (SELECT display_name FROM users WHERE id = (SELECT target_user_id FROM audit_target)), 'phone', (SELECT phone FROM users WHERE id = (SELECT target_user_id FROM audit_target)))),\n NOW(),\n (SELECT target_user_id FROM audit_target)\nFROM audit_target;\n" ❌ Test Case: Register a new user with unique email creates a pending accountError:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
**/*.tsx: Prefix React event handler functions with "handle" (e.g., handleClick)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Files:
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Use runtime type validation with valibot for external data validation
Prefer early returns for readability
Write simple, direct code without backward compatibility shims; update all call sites together
Use const-assigned arrow functions instead of function declarations for small utilities (e.g., const toggle = () => {})
Follow existing import patterns and tsconfig path aliases
Files:
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx
**/!(page).tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use named exports only (no default exports) for React/TSX modules
Files:
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx
⏰ 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-ci
- GitHub Check: frontend-lint
- GitHub Check: agent-deep-modeling
- GitHub Check: Supabase Preview
🔇 Additional comments (4)
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx (4)
1-1: LGTM! Import changes align with the refactoring objective.The new imports support the shift from artifact-based to analyzedRequirements-based flow.
48-48: LGTM! Organization ID is correctly included.Adding
organization_idto the select statement is necessary for the organization-aware repository setup introduced later in the function.
93-96: LGTM! Proper organization validation.The organization ID extraction and validation follows the established pattern in this function, with appropriate early return via
notFound().
114-114: LGTM! Correct prop for the refactored flow.Passing
initialAnalyzedRequirementstoSessionDetailPageClientaligns with the PR objective to replace artifact-based flows with analyzedRequirements.
| const repositories = createSupabaseRepositories(supabase, organizationId) | ||
| const config = { | ||
| configurable: { | ||
| repositories, | ||
| thread_id: designSessionId, | ||
| }, | ||
| } | ||
| const initialAnalyzedRequirements = await getAnalyzedRequirements(config) |
There was a problem hiding this comment.
Add error handling for getAnalyzedRequirements.
The getAnalyzedRequirements call lacks error handling, which is inconsistent with the rest of this function. All other data fetching operations (lines 40-42, 52-54, 69-71, 75-77, 82-84) check for errors and call notFound() when issues occur. If getAnalyzedRequirements throws, rejects, or returns unexpected data, the page will crash for users.
Apply this diff to add error handling:
const repositories = createSupabaseRepositories(supabase, organizationId)
const config = {
configurable: {
repositories,
thread_id: designSessionId,
},
}
- const initialAnalyzedRequirements = await getAnalyzedRequirements(config)
+ let initialAnalyzedRequirements
+ try {
+ initialAnalyzedRequirements = await getAnalyzedRequirements(config)
+ } catch (error) {
+ notFound()
+ }
+
+ if (!initialAnalyzedRequirements) {
+ notFound()
+ }📝 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.
| const repositories = createSupabaseRepositories(supabase, organizationId) | |
| const config = { | |
| configurable: { | |
| repositories, | |
| thread_id: designSessionId, | |
| }, | |
| } | |
| const initialAnalyzedRequirements = await getAnalyzedRequirements(config) | |
| const repositories = createSupabaseRepositories(supabase, organizationId) | |
| const config = { | |
| configurable: { | |
| repositories, | |
| thread_id: designSessionId, | |
| }, | |
| } | |
| let initialAnalyzedRequirements | |
| try { | |
| initialAnalyzedRequirements = await getAnalyzedRequirements(config) | |
| } catch (error) { | |
| notFound() | |
| } | |
| if (!initialAnalyzedRequirements) { | |
| notFound() | |
| } |
🤖 Prompt for AI Agents
In
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx
around lines 98 to 105, the call to getAnalyzedRequirements lacks error
handling; wrap the await getAnalyzedRequirements(config) call in a try/catch (or
handle rejected promise) and validate its result, and on any thrown error,
rejection, or unexpected/empty return value call notFound() to match the
existing fetch error handling pattern used elsewhere in this function.
- Add `getAnalyzedRequirements` function to extract analyzed requirements from state and tasks. - Introduce utility functions `extractAnalyzedRequirementsFromState` and `collectAnalyzedRequirementsFromTasks`. - Create unit tests for the new functionality in `getAnalyzedRequirements.test.ts`. - Update event constants to include `ANALYZED_REQUIREMENTS` for better event handling. - Export the new function in the agent's index file for accessibility.
- Pass analyzedRequirements from useStream through component tree to ArtifactContainer - Auto-switch to Artifact tab on first analyzedRequirements reception - Prioritize analyzedRequirements over legacy artifact data in display - Update shouldShowOutputSection condition to include analyzedRequirements 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…nents - Add initialAnalyzedRequirements prop to PublicSessionDetailPage and SessionDetailPage. - Update loadSessionData to fetch and pass initialAnalyzedRequirements. - Modify SessionDetailPageClient to handle initialAnalyzedRequirements and manage its state. - Enhance useStream hook to initialize analyzedRequirements with the provided prop. This update improves the handling of analyzed requirements across session detail components, ensuring they are properly integrated and displayed.
- Remove `getArtifact` method from `InMemoryRepository` and `SupabaseSchemaRepository` to streamline the repository interface. - Update related types in `types.ts` to reflect the removal of the `getArtifact` method. - Simplify the `saveRequirementsToArtifactTool` by eliminating unnecessary artifact saving logic. This refactoring enhances the clarity and maintainability of the agent's repository structure.
…ements Complete Phase 6.4 of the artifact-to-analyzedRequirements migration: **Deleted:** - useRealtimeArtifact hook (Supabase Realtime subscription) **Modified:** - SessionDetailPage: Remove artifact fetch from database - SessionDetailPageClient: Remove useRealtimeArtifact, fix initial tab selection for analyzedRequirements - Output: Remove artifact/artifactError props - Header: Generate markdown from analyzedRequirements instead of artifactDoc - ArtifactContainer: Simplify to only accept analyzedRequirements - Header.stories: Update to use AnalyzedRequirements format - PublicSessionDetailPage: Remove artifact fetch code **Key improvements:** - Initial tab now correctly shows ARTIFACT when only analyzedRequirements exist - Removes all Supabase Realtime dependencies for requirements - Fully transitions to SSE + checkpointer architecture 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…requirements - Added organization ID retrieval from building schema and integrated it into the session detail page. - Updated the query to include organization ID in the fetched data. - Implemented initial analyzed requirements fetching using the new organization ID. - Passed the initial analyzed requirements to the PublicLayout component. This update enhances the session detail page by ensuring it correctly handles organization-specific data and analyzed requirements.
sasamuku
force-pushed
the
refactor_analyzed_requirement_extraction
branch
from
f557ad5 to
ba5dc10
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx (1)
100-108: WrapgetAnalyzedRequirementsin error/null handling.Line 108 calls
getAnalyzedRequirementswithout guarding failures, unlike every other data fetch in this file. If the agent throws or returnsnull, the page will 500 instead of falling back tonotFound(). Please follow the existing pattern and catch errors/null responses.const repositories = createSupabaseRepositories(supabase, organizationId) const config = { configurable: { repositories, thread_id: designSessionId, }, } - const initialAnalyzedRequirements = await getAnalyzedRequirements(config) + let initialAnalyzedRequirements: Awaited< + ReturnType<typeof getAnalyzedRequirements> + > + try { + initialAnalyzedRequirements = await getAnalyzedRequirements(config) + } catch { + notFound() + } + + if (!initialAnalyzedRequirements) { + notFound() + }frontend/internal-packages/agent/src/getAnalyzedRequirements.ts (1)
20-24: Rejectnulltestcases in the type guard.
typeof null === 'object', so the current check lets{ goal: 'x', testcases: null }slip through. Downstream code assumestestcasesis iterable and will crash. Explicitly blocknull.- if (!('testcases' in value) || typeof value.testcases !== 'object') { + if ( + !('testcases' in value) || + typeof value.testcases !== 'object' || + value.testcases === null + ) { return false }frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
149-159: Critical: Unguarded JSON.parse can crash the stream (flagged previously).This handler still lacks defensive parsing despite being flagged in a previous review.
JSON.parseon Line 151 can throw ifev.datais malformed, and Line 152 assumes the payload is an array without validation. Both issues will terminate the stream unexpectedly.Apply the diff from the previous review to harden parsing:
const handleAnalyzedRequirementsEvent = useCallback( (ev: { data: string }) => { - const parsedData = JSON.parse(ev.data) - const [serialized] = parsedData - const result = safeParse(analyzedRequirementsSchema, serialized) - if (result.success) { - setAnalyzedRequirements(result.output) - } + try { + const parsed = JSON.parse(ev.data) + const payload = Array.isArray(parsed) ? parsed[0] : parsed + const result = safeParse(analyzedRequirementsSchema, payload) + if (result.success) setAnalyzedRequirements(result.output) + } catch { + // Ignore malformed event; keep stream alive + } }, [], )
🧹 Nitpick comments (1)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
203-204: Consider logging or documenting the empty error branch.The error case at Lines 203-204 is handled by setting state in
handleErrorEvent, but the emptyifblock here is unclear. Consider adding a comment explaining that errors are already handled, or log for observability.const result = handleStreamEvent(ev) if (result === 'end') { endEventReceived = true break } if (result === 'error') { + // Error already set in state by handleErrorEvent; continue reading stream } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsx(4 hunks)frontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsx(6 hunks)frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx(5 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx(5 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/hooks/useRealtimeArtifact.ts(0 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsx(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsx(4 hunks)frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts(5 hunks)frontend/apps/app/eslint-suppressions.json(2 hunks)frontend/internal-packages/agent/src/getAnalyzedRequirements.test.ts(1 hunks)frontend/internal-packages/agent/src/getAnalyzedRequirements.ts(1 hunks)frontend/internal-packages/agent/src/index.ts(1 hunks)frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts(2 hunks)frontend/internal-packages/agent/src/repositories/InMemoryRepository.ts(0 hunks)frontend/internal-packages/agent/src/repositories/supabase.ts(0 hunks)frontend/internal-packages/agent/src/repositories/types.ts(0 hunks)frontend/internal-packages/agent/src/streaming/constants.ts(1 hunks)
💤 Files with no reviewable changes (4)
- frontend/internal-packages/agent/src/repositories/supabase.ts
- frontend/internal-packages/agent/src/repositories/types.ts
- frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/hooks/useRealtimeArtifact.ts
- frontend/internal-packages/agent/src/repositories/InMemoryRepository.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/apps/app/eslint-suppressions.json
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/internal-packages/agent/src/getAnalyzedRequirements.test.ts
- frontend/apps/app/components/SessionDetailPage/SessionDetailPage.tsx
- frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.stories.tsx
- frontend/internal-packages/agent/src/pm-agent/tools/saveRequirementsToArtifactTool.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/internal-packages/agent/src/index.tsfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/internal-packages/agent/src/getAnalyzedRequirements.tsfrontend/internal-packages/agent/src/streaming/constants.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}: Use runtime type validation with valibot for external data validation
Prefer early returns for readability
Write simple, direct code without backward compatibility shims; update all call sites together
Use const-assigned arrow functions instead of function declarations for small utilities (e.g., const toggle = () => {})
Follow existing import patterns and tsconfig path aliases
Files:
frontend/internal-packages/agent/src/index.tsfrontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/internal-packages/agent/src/getAnalyzedRequirements.tsfrontend/internal-packages/agent/src/streaming/constants.tsfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx
frontend/internal-packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Infra and tooling (e2e, configs, storybook, agent) live under frontend/internal-packages
Files:
frontend/internal-packages/agent/src/index.tsfrontend/internal-packages/agent/src/getAnalyzedRequirements.tsfrontend/internal-packages/agent/src/streaming/constants.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
**/*.tsx: Prefix React event handler functions with "handle" (e.g., handleClick)
Import UI components from @liam-hq/ui when available
Import icons from @liam-hq/ui
Files:
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.tsfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx
**/!(page).tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use named exports only (no default exports) for React/TSX modules
Files:
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsxfrontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsxfrontend/apps/app/components/PublicSessionDetailPage/PublicSessionDetailPage.tsxfrontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx
🧬 Code graph analysis (6)
frontend/apps/app/components/SessionDetailPage/components/Output/components/Header/Header.tsx (2)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/utils/formatArtifactToMarkdown.ts (1)
formatArtifactToMarkdown(53-98)
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (3)
frontend/internal-packages/artifact/src/schemas/artifact.ts (2)
AnalyzedRequirements(30-32)analyzedRequirementsSchema(25-28)frontend/internal-packages/agent/src/streaming/constants.ts (1)
SSE_EVENTS(1-6)frontend/apps/app/components/SessionDetailPage/hooks/useStream/parseSse.ts (1)
parseSse(7-68)
frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx (3)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/utils/formatArtifactToMarkdown.ts (1)
formatArtifactToMarkdown(53-98)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/Artifact.tsx (1)
Artifact(37-218)
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (3)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/types.ts (1)
Version(10-13)frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (1)
useStream(73-361)
frontend/internal-packages/agent/src/getAnalyzedRequirements.ts (2)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/internal-packages/agent/src/types.ts (1)
WorkflowConfigurable(13-19)
frontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx (2)
frontend/internal-packages/artifact/src/schemas/artifact.ts (1)
AnalyzedRequirements(30-32)frontend/apps/app/components/SessionDetailPage/components/Output/components/Artifact/ArtifactContainer.tsx (1)
ArtifactContainer(12-21)
⏰ 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). (8)
- GitHub Check: CodeQL
- GitHub Check: _e2e-tests (Mobile Safari)
- GitHub Check: Supabase Preview
- GitHub Check: agent-deep-modeling
- GitHub Check: codeql / languages (javascript) / Perform CodeQL for javascript
- GitHub Check: frontend-lint
- GitHub Check: frontend-ci
- GitHub Check: Supabase Preview
🔇 Additional comments (6)
frontend/apps/app/components/SessionDetailPage/components/Output/Output.tsx (1)
3-3: LGTM! Clean refactoring to AnalyzedRequirements.The migration from
ArtifacttoAnalyzedRequirementsis consistent across imports, props, and component wiring. Type safety is maintained, and the simplified data flow improves clarity.Also applies to: 28-29, 39-39, 78-78, 94-94
frontend/apps/app/components/SessionDetailPage/hooks/useStream/useStream.ts (2)
166-189: LGTM! Centralized event handling improves maintainability.The
handleStreamEventfunction consolidates SSE event routing with clear control-flow signals ('end' | 'error' | 'continue'), making the streaming logic easier to follow and extend.
70-70: LGTM! Analyzed requirements integrated into streaming state.The hook now correctly threads
initialAnalyzedRequirementsthrough props, state, and return value, aligning with the refactoring goals.Also applies to: 76-76, 93-94, 354-354
frontend/apps/app/components/SessionDetailPage/SessionDetailPageClient.tsx (3)
37-55: LGTM! Clear initial tab determination logic.The
determineInitialTabfunction correctly prioritizes ERD when versions exist and falls back to ARTIFACT only whenanalyzedRequirementsare present. The logic is straightforward and aligns with the refactoring objectives.
73-75: LGTM! Proper tracking of analyzed requirements arrival.The state tracking (
hasReceivedAnalyzedRequirements) and effect hook correctly switch to the ARTIFACT tab when new analyzed requirements arrive during streaming, while preventing duplicate triggers via the ref comparison.Also applies to: 114-123
102-112: LGTM! Successfully migrated from artifact to analyzed requirements flow.The integration with
useStreamis correct, passinginitialAnalyzedRequirementsand consuminganalyzedRequirementsfrom the hook's return value. The removal ofuseRealtimeArtifactaligns with the PR's goal of consolidating data flow through SSE streaming.Also applies to: 206-206
sasamuku
requested review from
MH4GF,
NoritakaIkeda and
junkisai
and removed request for
a team
1 participant
Issue
Why is this change needed?
This PR refactors the analyzed requirements handling to improve the separation of concerns and streamline the data flow:
getAnalyzedRequirementsfunction in the agent package to extract and collect analyzed requirements from tool calls, making the logic reusable and testableanalyzedRequirementsfrom SSE instead of separate real-time artifact subscriptionsKey Changes:
getAnalyzedRequirements.tswith comprehensive test coverage (192 lines of tests)analyzed_requirementsevent typeSessionDetailPagecomponents to useanalyzedRequirementsfrom unified streaminguseRealtimeArtifacthook in favor of simpler streaming approach🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]
Summary by CodeRabbit
New Features
Refactor
Tests
Chores