🐛(agent): Fix DB Agent infinite loop by enhancing convertSchemaToText #3750
Conversation
Added aCheckConstraint to the public exports to make it available for external use. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…nd checks support - Add support for constraints (FOREIGN KEY, UNIQUE, CHECK) - Add support for indexes (UNIQUE and NON-UNIQUE) - Add support for column-level CHECK constraints - Refactor to use typed exports (Table, Columns, Constraints, Indexes) from @liam-hq/schema - Remove unnecessary null checks based on schema type definitions - Add comprehensive test coverage (18 test cases) using toMatchInlineSnapshot This fixes the DB Agent infinite loop issue where schema information was incomplete, causing QA Agent to repeatedly report the same missing constraints and indexes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Unexpected error on running flow
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Updates to Preview Branch (investigate-schema-design-tool) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
WalkthroughAdds a full test suite for convertSchemaToText; refactors convertSchemaToText into modular formatter helpers and a table document builder; and publicly re-exports aCheckConstraint from the schema package. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant convertSchemaToText
participant tableToDocument
participant formatColumns
participant formatPrimaryKey
participant formatConstraints
participant formatIndexes
Caller->>convertSchemaToText: convertSchemaToText(schema)
convertSchemaToText->>convertSchemaToText: Iterate schema.tables
loop For each table
convertSchemaToText->>tableToDocument: tableToDocument(name, table)
tableToDocument->>formatColumns: formatColumns(table.columns)
tableToDocument->>formatPrimaryKey: formatPrimaryKey(columns, constraints)
tableToDocument->>formatConstraints: formatConstraints(constraints)
tableToDocument->>formatIndexes: formatIndexes(indexes)
tableToDocument-->>convertSchemaToText: Table document string
end
convertSchemaToText-->>Caller: Combined text output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: REQUIRED Reason:
Changeset (copy & paste):---
"@liam-hq/schema": minor
---
- ✨ Export `aCheckConstraint` from `@liam-hq/schema`
- Adds a factory for creating CHECK constraints to the public API (useful for consumers and tests) |
🤖 Agent Deep Modeling ExecutionProcessing time: 9m 27s Command Output
RUN v3.2.4 /home/runner/work/liam/liam/frontend/internal-packages/agent (node:7653) ExperimentalWarning: WASI is an experimental feature and might change at any time ✅ [INFO] 2025-10-10T10:12:09.664Z 🧠 Thinking (pm)... Designing user management system requirements I'm acting as a PM Agent, focusing on producing structured requirements for a user management system. Since there's no schema provided, I'll generate entities, fields, constraints, and business rules. My task requires me to summarize this in a BRD format, starting with a concise checklist of key points. I'll then clarify any ambiguous instructions and structure the requirements accordingly. Once finalized, I’ll confirm saving the requirements using the appropriate tool. No web searches are necessary for this. Structuring test cases for user management I’m drafting test cases for areas like User Registration, Authentication, Profile Management, and more. Each category needs to reflect SQL operations, so I'll focus on actions such as INSERT for creating users, UPDATE for profile changes, DELETE for user removal, and SELECT for lookups. I'm also considering negative scenarios like rejecting duplicate emails. My goal is to provide 3-6 concise test cases per category. I'll outline a BRD with sections including objectives, assumptions, user stories, functional and non-functional requirements, and the data model with key entities like User and Role. Designing a simple yet comprehensive system I'm aiming for a user management system that remains simple but thorough. Let's outline essential components: For the User entity, I'll include fields like id, username, email, phone (optional), password_hash, status, and verification flags. Unique constraints will be needed for username and email as case-insensitive. The Role entity includes id, name (e.g., ADMIN, USER), and description. I'll create relationships like UserRole and establish Permissions for role-based access control, possibly simplifying it. I’ll also define authentication methods, token management for email verification and password resets, and log user actions with an AuditLog and LoginAttempt structure. Defining functional and non-functional requirements I'm working to keep the system simple while incorporating essential features, aiming for a user-friendly experience. For functional requirements, users can register with a unique email, verify their accounts via token, and sign in with their email or username along with a password. I’m contemplating a password policy that could start at 8 characters with complexity requirements. As for admin capabilities, they can manage user roles, creating and updating accounts while users manage their profiles. I'll also establish a search function for users and a soft deletion policy. On the non-functional side, security measures like password hashing with Argon2id or bcrypt are crucial, along with performance metrics for handling up to 10,000 users. Internationalization might be out of scope, and I’ll need to map test cases to SQL types for validation. Creating structured test cases for user management I'm outlining test cases for various categories, starting with Registration & Email Verification. Here, I'll include scenarios like successful registration with a unique email and token verification leading to account activation. For Authentication & Sessions, actions will include successful logins and rejections based on verification status. I'll maintain concise, meaningful test cases throughout. In Profile Management, users will update their profiles or be restricted from changing to an already taken email. Similarly, roles and permissions will be managed carefully by admins. I’ll structure the BRD with elements like project goals, stakeholder details, and data models, ensuring it's clear and to the point. Test cases will be crafted to minimize duplication while covering essential functionalities, leading to a comprehensive test plan. The function call for saving these requirements will ensure everything is documented accurately. Mapping SQL types for test cases I need to ensure that my test cases map correctly to SQL types for quality assurance. For successful logins, I can log an INSERT into the Session. If a login is rejected, I'll record it with an INSERT into LoginAttempt. For account lockouts after failed login attempts, I'll use UPDATE to reflect status changes. The logout action would require a DELETE operation to remove the session. In the Profile Management section, updating profile details will also use UPDATE, while role assignments by admins will involve INSERT or DELETE actions. As I create the BRD, I’ll keep it concise while ensuring all the essential details are included. After crafting the requirements and noting necessary constraints and indexes, I’ll call the tool to save everything and confirm the successful completion of the process. ⏺ (pm): Planned actions:
Business Requirements Document (BRD)
Test Scenarios (SQL-oriented, for QA coverage)
Proceeding to save the analyzed goal and testcases. 🔧 saveRequirementsToArtifactTool() ⎿ unknown: Requirements saved successfully to artifact 🧠 Thinking (db)... Planning database schema First, I’m focusing on the Planning Checklist. I’ll call the schema design tool to set up the necessary tables for the user management system, like users, email verification tokens, sessions, roles, user roles, password reset tokens, and audits. I need to enforce case-insensitive unique email addresses, possibly by storing a normalized version. There'll be primary keys and foreign keys for relationships, plus checks for username patterns using regex. Let’s get started on these operations! 🔧 schemaDesignTool() 🔧 schemaDesignTool() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase() ⏺ (qa): { 🔧 saveTestcase()(node:7653) Warning: The 'import-in-the-middle' hook has already been initialized 🔧 saveTestcase(), email, email_normalized, is_email_verified\n)\n, verif AS (\n -- 3) Create an email_verification_tokens row for the new user\n INSERT INTO email_verification_tokens (\n id,\n token,\n user_id,\n created_at,\n expires_at,\n consumed_at\n ) VALUES (\n gen_random_uuid(),\n substr(md5(gen_random_uuid()::text || clock_timestamp()::text), 0, 32),\n (SELECT id FROM ins_user),\n now(),\n now() + interval '24 hours',\n NULL\n )\n RETURNING user_id\n)\nSELECT\n u.id AS user_id,\n u.email,\n u.email_normalized,\n u.status,\n et.token AS verification_token\nFROM\n ins_user u\n JOIN verif ON verif.user_id = u.id\n JOIN email_verification_tokens et ON et.user_id = u.id;\n\nCOMMIT;\n" ⏺ (qa): {"sql":"-- Test Case: Logout revokes the active session\n-- Scenario: A user with an active session logs out; revoke the most recently created, non-expired session by setting revoked_at = now().\n\n-- Parameters to supply at runtime:\n-- user_id: UUID of the user performing logout\n\n-- Approach:\n-- 1. Find the latest active session for the user (not revoked, not expired).\n-- 2. Revoke it by setting revoked_at and updating last_activity_at.\n-- 3. Return basic details of the revoked session for auditing/verification.\n\nWITH target AS (\n SELECT s.id\n FROM sessions s\n WHERE s.user_id = user_id\n AND s.revoked_at IS NULL\n AND s.expires_at > NOW()\n ORDER BY s.created_at DESC\n LIMIT 1\n FOR UPDATE\n)\nUPDATE sessions s\nSET revoked_at = NOW(), last_activity_at = NOW()\nFROM target t\nWHERE s.id = t.id\nRETURNING s.id AS session_id,\n s.user_id,\n s.revoked_at,\n s.created_at,\n s.expires_at,\n s.last_activity_at;"} 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() ⏺ (qa): { ⏺ (qa):
I cannot output SQL text directly in this channel per your instruction to use the saveTestcase tool. If you want me to generate and save the SQL via the designated tool, please confirm and I will provide the exact JSON payload for the tool, ensuring a clean DO/BEGIN/COMMIT flow without syntax errors. Here is the corrected SQL you can run in PostgreSQL (as text). If you want it saved via the tool, I will send the tool call immediately after you confirm. BEGIN; -- Clean slate for test data -- Insert a test user and capture id -- Insert a test role and capture id -- Create the user_roles mapping -- Verify mapping exists -- Delete the role to cascade -- Verify cascade removal 🔧 saveTestcase() 🔧 saveTestcase() 🔧 saveTestcase() target_user_id)\n VALUES (\n 'UPDATE_PASSWORD',\n jsonb_build_object('user_id', v_user_id, 'change_type', 'PASSWORD_CHANGED'),\n NULL, -- system action; replace with actor_id if available\n v_now,\n NULL,\n v_user_id\n );\n\nEND;\n$$ LANGUAGE plpgsql;\n" COMMIT; If you want me to proceed to save this via the saveTestcase tool, I can provide the exact JSON payload wrapping this SQL (ensuring only the SQL string is sent).(node:7653) Warning: The 'import-in-the-middle' hook has already been initialized ⏺ (qa): { ❌ Test Case: Registration rejected for duplicate email (case-insensitive)Error:
|
MH4GF
requested review from
NoritakaIkeda,
Copilot,
junkisai and
sasamuku
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an infinite loop in the DB Agent by enhancing the convertSchemaToText function to include all schema elements that were previously missing. The Agent was stuck because the schema-to-text conversion was incomplete, causing the QA Agent to repeatedly flag the same missing constraints and indexes even after they were added.
- Enhanced schema-to-text conversion to include primary keys, constraints, and indexes
- Refactored the conversion logic into modular functions for better maintainability
- Added comprehensive test coverage with 18 test cases using inline snapshots
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/packages/schema/src/index.ts | Exports aCheckConstraint helper for test usage |
| frontend/internal-packages/agent/src/utils/convertSchemaToText.ts | Enhanced to include primary keys, constraints, and indexes in schema output |
| frontend/internal-packages/agent/src/utils/convertSchemaToText.test.ts | Added comprehensive test coverage for all schema conversion features |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
This issue can be resolved simultaneously with #3751, and since it also has significant value, I am closing it. |
1 participant
Issue
Why is this change needed?
The DB Agent was entering an infinite loop because the schema-to-text conversion was incomplete. When the DB Agent successfully added constraints and indexes to the schema, the QA Agent couldn't see these changes in the next iteration because
convertSchemaToTextwasn't including constraints and indexes in its output. This caused the QA Agent to repeatedly report the same missing constraints and indexes, leading to a recursion limit error.Root Cause
The
convertSchemaToTextfunction was only outputting:But it was missing:
Solution
Enhanced
convertSchemaToText.tsto include:Also added comprehensive test coverage (18 test cases) using
toMatchInlineSnapshot()per test-principles.md.Changes
aCheckConstraintfor test usageVerification
Deep modeling is now complete 👍🏻
ref: #3750 (comment)
Summary by CodeRabbit