Skip to content

Conversation

@hoshinotsuyoshi
Copy link
Member

@hoshinotsuyoshi hoshinotsuyoshi commented May 26, 2025

Just PoC. no need to merge.

poc(chat): improve prompt to guide LLM toward JSON Patch format

  • Replaced the previous agent-based approach with a LangGraph workflow (runChat) for the 'build' mode
  • Refined the system prompt to consistently guide the LLM toward proper RFC 6902 JSON Patch output, wrapped in ```json fences
  • Improved the prompt structure to allow informative commentary outside the JSON Patch block
  • Added fallback mechanism for non-schema-editing messages (i.e., omit JSON Patch)
  • Introduced unit tests for LangGraph flow to ensure format correctness and schema alignment
  • Updated dependencies to include @langchain/langgraph

Image

11.mov

🧭 Overview (Flow)

  • This change transitions from a sequential mastra.getAgent().generate(...)-based approach to a state-driven LLM processing pipeline using LangGraph.

  • The new process can be represented as the following DAG:

graph TD
  Start([START])
  BP[buildPrompt]
  DF[draft]
  CH[check]
  RM[remind]
  End([END])

  Start --> BP --> DF --> CH
  CH -->|valid: true| End
  CH -->|valid: false, retryCount < 3| RM --> CH
  CH -->|valid: false, retryCount >= 3| End
Loading

🧩 Role of Each Node

Node Name Description
buildPrompt Generates a system prompt based on user input, schema text, and chat history
draft Sends the prompt + message to the LLM and retrieves its response
check Validates whether the response contains a well-formed RFC 6902 JSON Patch
remind Sends a minimal fallback prompt to the LLM to retry generating a proper patch

Thanks to this design, the system can retry up to 3 times if a valid JSON Patch is not returned.


🏗️ Integration with Surrounding Code

route.ts

  • The /api/chat API endpoint calls runChat(...) if mode: 'build'.

  • It passes schemaText and chatHistory into LangGraph for stateful execution.

ChatbotDialog.tsx

  • The frontend explicitly sets mode: 'build', which activates the LangGraph flow.

mastra/agents/databaseSchemaBuildAgent.ts

  • The buildPrompt used in LangGraph has been enhanced with an action-oriented system prompt to ensure the LLM always returns a JSON Patch wrapped in a ```json code fence.


✅ Advantages of the DAG Approach

  • Clear State Management: With ChatState annotations, each step's expected inputs and outputs are type-safe and well-defined.

  • Self-Healing: The check → remind → check loop allows automatic retries when output is malformed or incomplete.

  • Extensibility: Adding a separate graph for ask mode or inserting new branches is straightforward and modular.


🧠 Summary

This commit represents a shift from "just calling the LLM" to a more robust architecture that controls, verifies, and retries LLM behavior.
By adopting LangGraph, we've built a reliable and extensible LLM integration pipeline grounded in DAG principles.


Issue

  • resolve:

Why is this change needed?

What would you like reviewers to focus on?

Testing Verification

What was done

🤖 Generated by PR Agent at deed8e5

  • Introduced LangGraph-based workflow for schema build chat mode
    • Replaced agent-based pipeline with runChat using LangGraph state graph
    • Enhanced system prompt to enforce RFC 6902 JSON Patch output in code fences
    • Added fallback for non-schema-editing messages (omit JSON Patch)
  • Added comprehensive unit tests for LangGraph chat flow
    • Validates JSON Patch extraction, schema alignment, and prompt handling
  • Updated documentation and prompt examples for output format clarity
  • Added and updated dependencies for LangGraph integration

Detailed Changes

Relevant files
Enhancement
3 files
langGraph.ts
Add LangGraph-based chat workflow for schema build mode   
+163/-0 
route.ts
Switch to LangGraph chat pipeline for build mode                 
ChatbotDialog.tsx
Ensure chat API uses build mode for LangGraph                       
+1/-0     
Tests
1 files
langGraph.test.ts
Add unit tests for LangGraph chat pipeline                             
+113/-0 
Documentation
1 files
databaseSchemaBuildAgent.ts
Update system prompt with strict JSON Patch output instructions
+27/-0   
Dependencies
2 files
package.json
Add @langchain/langgraph dependency                                           
+1/-0     
pnpm-lock.yaml
Update lockfile for LangGraph and related dependencies     
+57/-1   
Additional files
1 files
route.ts +11/-28 

Additional Notes


Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @hoshinotsuyoshi hoshinotsuyoshi self-assigned this May 26, 2025
    @changeset-bot
    Copy link

    changeset-bot bot commented May 26, 2025

    ⚠️ No Changeset found

    Latest commit: deed8e5

    Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

    This PR includes no changesets

    When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

    Click here to learn what changesets are, and how to add one.

    Click here if you're a maintainer who wants to add a changeset to this PR

    @supabase
    Copy link

    supabase bot commented May 26, 2025

    Updates to Preview Branch (hoshino-poc-chat-op) ↗︎

    Deployments Status Updated
    Database Tue, 27 May 2025 07:02:19 UTC
    Services Tue, 27 May 2025 07:02:19 UTC
    APIs Tue, 27 May 2025 07:02:19 UTC

    Tasks are run on every commit but only new migration files are pushed.
    Close and reopen this PR if you want to apply changes from existing seed or migration files.

    Tasks Status Updated
    Configurations Tue, 27 May 2025 07:02:20 UTC
    Migrations Tue, 27 May 2025 07:02:21 UTC
    Seeding Tue, 27 May 2025 07:02:21 UTC
    Edge Functions Tue, 27 May 2025 07:02:21 UTC

    View logs for this Workflow Run ↗︎.
    Learn more about Supabase for Git ↗︎.

    @vercel
    Copy link

    vercel bot commented May 26, 2025

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    liam-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 7:14am
    liam-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 7:14am
    liam-erd-sample ✅ Ready (Inspect) Visit Preview May 27, 2025 7:14am
    liam-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 7:14am

    @qodo-free-for-open-source-projects
    Copy link
    Contributor

    CI Feedback 🧐

    A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

    Action: _e2e-tests (Mobile Safari)

    Failed stage: Run e2e tests [❌]

    Failed test name: zoom in button should increase zoom level

    Failure summary:

    The action failed because the Playwright end-to-end test "zoom in button should increase zoom level"
    in the Mobile Safari project timed out after 10000ms (10 seconds). The test was trying to click on
    the "Zoom in" button in the toolbar, but the element was not becoming visible, enabled, or stable
    within the timeout period. The failure occurred at line 54 in the file
    /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts. The test was retried 5
    times but failed with the same timeout issue each time.

    Relevant error logs:
    1:  ##[group]Runner Image Provisioner
    2:  Hosted Compute Agent
    ...
    
    216:  CI: true
    217:  URL: https://liam-84levp3hy-route-06-core.vercel.app
    218:  ENVIRONMENT: Preview – liam-app
    219:  PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
    220:  ##[endgroup]
    221:  Scope: all 18 workspace projects
    222:  Lockfile is up to date, resolution step is skipped
    223:  Progress: resolved 1, reused 0, downloaded 0, added 0
    224:  Packages: +2251
    225:  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
    226:  Progress: resolved 2251, reused 1025, downloaded 0, added 0
    227:  Progress: resolved 2251, reused 2237, downloaded 0, added 198
    228:  Progress: resolved 2251, reused 2237, downloaded 0, added 841
    229:  Progress: resolved 2251, reused 2237, downloaded 0, added 1371
    230:  Progress: resolved 2251, reused 2237, downloaded 0, added 2251, done
    231:  WARN  Failed to create bin at /home/runner/work/liam/liam/frontend/apps/erd-sample/node_modules/.bin/liam. ENOENT: no such file or directory, open '/home/runner/work/liam/liam/frontend/packages/cli/dist-cli/bin/cli.js'
    232:  devDependencies:
    ...
    
    246:  │   Ignored build scripts: @biomejs/biome, @bundled-es-modules/glob,           │
    247:  │   @depot/cli, @prisma/client, @prisma/engines, @sentry/cli,                  │
    248:  │   @tailwindcss/oxide, core-js-pure, esbuild, onnxruntime-node, protobufjs,   │
    249:  │   sharp, style-dictionary.                                                   │
    250:  │   Run "pnpm approve-builds" to pick which dependencies should be allowed     │
    251:  │   to run scripts.                                                            │
    252:  │                                                                              │
    253:  ╰──────────────────────────────────────────────────────────────────────────────╯
    254:  frontend/packages/jobs postinstall$ cp ../db-structure/node_modules/@ruby/prism/src/prism.wasm prism.wasm
    255:  frontend/apps/docs postinstall$ fumadocs-mdx
    256:  frontend/packages/jobs postinstall: Done
    257:  frontend/apps/docs postinstall: [MDX] types generated
    258:  frontend/apps/docs postinstall: Done
    259:  frontend/apps/app postinstall$ cp ../../packages/db-structure/node_modules/@ruby/prism/src/prism.wasm prism.wasm
    260:  frontend/apps/app postinstall: Done
    261:  WARN  Failed to create bin at /home/runner/work/liam/liam/frontend/apps/erd-sample/node_modules/.bin/liam. ENOENT: no such file or directory, open '/home/runner/work/liam/liam/frontend/apps/erd-sample/node_modules/@liam-hq/cli/dist-cli/bin/cli.js'
    262:  Done in 7.4s using pnpm v10.10.0
    ...
    
    264:  with:
    265:  path: ~/.cache/ms-playwright
    266:  key: playwright-Linux-174e9c28f9490fb5ec7650a5a514cc6bc60d6f703c84a33b43dd4603fe6a2669
    267:  restore-keys: playwright-Linux-
    268:  
    269:  enableCrossOsArchive: false
    270:  fail-on-cache-miss: false
    271:  lookup-only: false
    272:  save-always: false
    273:  env:
    274:  CI: true
    275:  URL: https://liam-84levp3hy-route-06-core.vercel.app
    276:  ENVIRONMENT: Preview – liam-app
    277:  PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
    278:  ##[endgroup]
    279:  [warning]Event Validation Error: The event type deployment_status is not supported because it's not tied to a branch or tag ref.
    280:  ##[group]Run pnpm exec playwright install --with-deps
    ...
    
    1517:  |■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■| 100% of 2.3 MiB
    1518:  FFMPEG playwright build v1011 downloaded to /home/runner/.cache/ms-playwright/ffmpeg-1011
    1519:  ##[group]Run pnpm exec playwright test --project="Mobile Safari"
    1520:  �[36;1mpnpm exec playwright test --project="Mobile Safari"�[0m
    1521:  shell: /usr/bin/bash -e {0}
    1522:  env:
    1523:  CI: true
    1524:  URL: https://liam-84levp3hy-route-06-core.vercel.app
    1525:  ENVIRONMENT: Preview – liam-app
    1526:  PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
    1527:  ##[endgroup]
    1528:  Running 17 tests using 1 worker
    1529:  °°°·°×±···×××××T××±°°°°°·
    1530:  1) [Mobile Safari] › tests/e2e/toolbar.test.ts:47:5 › zoom in button should increase zoom level ──
    1531:  �[31mTest timeout of 10000ms exceeded.�[39m
    1532:  Error: locator.click: Test timeout of 10000ms exceeded.
    1533:  Call log:
    1534:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom in' })�[22m
    1535:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom in" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1536:  �[2m  - attempting click action�[22m
    1537:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1538:  52 |
    1539:  53 |   const zoomInButton = toolbar.getByRole('button', { name: 'Zoom in' })
    1540:  > 54 |   await zoomInButton.click()
    1541:  |                      ^
    1542:  55 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1543:  56 |
    1544:  57 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1545:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:54:22
    1546:  Error Context: test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari/error-context.md
    1547:  Retry #1 ───────────────────────────────────────────────────────────────────────────────────────
    1548:  �[31mTest timeout of 10000ms exceeded.�[39m
    1549:  Error: locator.click: Test timeout of 10000ms exceeded.
    1550:  Call log:
    1551:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom in' })�[22m
    1552:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom in" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1553:  �[2m  - attempting click action�[22m
    1554:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1555:  52 |
    1556:  53 |   const zoomInButton = toolbar.getByRole('button', { name: 'Zoom in' })
    1557:  > 54 |   await zoomInButton.click()
    1558:  |                      ^
    1559:  55 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1560:  56 |
    1561:  57 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1562:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:54:22
    1563:  Error Context: test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari-retry1/error-context.md
    1564:  attachment #2: trace (application/zip) ─────────────────────────────────────────────────────────
    1565:  test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari-retry1/trace.zip
    1566:  Usage:
    1567:  pnpm exec playwright show-trace test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari-retry1/trace.zip
    1568:  ────────────────────────────────────────────────────────────────────────────────────────────────
    1569:  Retry #2 ───────────────────────────────────────────────────────────────────────────────────────
    1570:  �[31mTest timeout of 10000ms exceeded.�[39m
    1571:  Error: locator.click: Test timeout of 10000ms exceeded.
    1572:  Call log:
    1573:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom in' })�[22m
    1574:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom in" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1575:  �[2m  - attempting click action�[22m
    1576:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1577:  52 |
    1578:  53 |   const zoomInButton = toolbar.getByRole('button', { name: 'Zoom in' })
    1579:  > 54 |   await zoomInButton.click()
    1580:  |                      ^
    1581:  55 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1582:  56 |
    1583:  57 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1584:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:54:22
    1585:  Error Context: test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari-retry2/error-context.md
    1586:  Retry #3 ───────────────────────────────────────────────────────────────────────────────────────
    1587:  �[31mTest timeout of 10000ms exceeded.�[39m
    1588:  Error: locator.click: Test timeout of 10000ms exceeded.
    1589:  Call log:
    1590:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom in' })�[22m
    1591:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom in" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1592:  �[2m  - attempting click action�[22m
    1593:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1594:  52 |
    1595:  53 |   const zoomInButton = toolbar.getByRole('button', { name: 'Zoom in' })
    1596:  > 54 |   await zoomInButton.click()
    1597:  |                      ^
    1598:  55 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1599:  56 |
    1600:  57 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1601:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:54:22
    1602:  Error Context: test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari-retry3/error-context.md
    1603:  Retry #4 ───────────────────────────────────────────────────────────────────────────────────────
    1604:  �[31mTest timeout of 10000ms exceeded.�[39m
    1605:  Error: locator.click: Test timeout of 10000ms exceeded.
    1606:  Call log:
    1607:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom in' })�[22m
    1608:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom in" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1609:  �[2m  - attempting click action�[22m
    1610:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1611:  52 |
    1612:  53 |   const zoomInButton = toolbar.getByRole('button', { name: 'Zoom in' })
    1613:  > 54 |   await zoomInButton.click()
    1614:  |                      ^
    1615:  55 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1616:  56 |
    1617:  57 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1618:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:54:22
    1619:  Error Context: test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari-retry4/error-context.md
    1620:  Retry #5 ───────────────────────────────────────────────────────────────────────────────────────
    1621:  �[31mTest timeout of 10000ms exceeded.�[39m
    1622:  Error: locator.click: Test timeout of 10000ms exceeded.
    1623:  Call log:
    1624:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom in' })�[22m
    1625:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom in" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1626:  �[2m  - attempting click action�[22m
    1627:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1628:  52 |
    1629:  53 |   const zoomInButton = toolbar.getByRole('button', { name: 'Zoom in' })
    1630:  > 54 |   await zoomInButton.click()
    1631:  |                      ^
    1632:  55 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1633:  56 |
    1634:  57 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1635:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:54:22
    1636:  Error Context: test-results/e2e-toolbar-zoom-in-button-should-increase-zoom-level-Mobile-Safari-retry5/error-context.md
    1637:  2) [Mobile Safari] › tests/e2e/page.test.ts:25:5 › Table node should be highlighted when clicked ─
    1638:  �[31mTest timeout of 10000ms exceeded.�[39m
    1639:  Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoHaveAttribute�[2m(�[22m�[32mexpected�[39m�[2m)�[22m
    1640:  Locator: getByRole('button', { name: 'accounts table', exact: true }).locator('div[class^="TableNode_wrapper"]')
    1641:  Expected string: �[32m"table-node-highlighted"�[39m
    1642:  Received: <element(s) not found>
    1643:  Call log:
    1644:  �[2m  - expect.toHaveAttribute with timeout 5000ms�[22m
    1645:  �[2m  - waiting for getByRole('button', { name: 'accounts table', exact: true }).locator('div[class^="TableNode_wrapper"]')�[22m
    1646:  35 |   await expect(
    1647:  36 |     tableNode.locator('div[class^="TableNode_wrapper"]'),
    1648:  > 37 |   ).toHaveAttribute('data-erd', 'table-node-highlighted')
    1649:  |     ^
    1650:  38 | })
    1651:  39 |
    1652:  40 | test('Edge animation should be triggered when table node is clicked', async ({
    1653:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/page.test.ts:37:5
    1654:  Error Context: test-results/e2e-page-Table-node-should-be-highlighted-when-clicked-Mobile-Safari/error-context.md
    1655:  3) [Mobile Safari] › tests/e2e/toolbar.test.ts:63:5 › zoom out button should decrease zoom level ─
    1656:  �[31mTest timeout of 10000ms exceeded.�[39m
    1657:  Error: locator.click: Test timeout of 10000ms exceeded.
    1658:  Call log:
    1659:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom out' })�[22m
    1660:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom out" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1661:  �[2m  - attempting click action�[22m
    1662:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1663:  68 |
    1664:  69 |   const zoomOutButton = toolbar.getByRole('button', { name: 'Zoom out' })
    1665:  > 70 |   await zoomOutButton.click()
    1666:  |                       ^
    1667:  71 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1668:  72 |
    1669:  73 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1670:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:70:23
    1671:  Error Context: test-results/e2e-toolbar-zoom-out-button-should-decrease-zoom-level-Mobile-Safari/error-context.md
    1672:  Retry #1 ───────────────────────────────────────────────────────────────────────────────────────
    1673:  �[31mTest timeout of 10000ms exceeded.�[39m
    1674:  Error: locator.click: Test timeout of 10000ms exceeded.
    1675:  Call log:
    1676:  �[2m  - waiting for getByRole('toolbar', { name: 'Toolbar' }).getByRole('button', { name: 'Zoom out' })�[22m
    1677:  �[2m    - locator resolved to <button type="button" tabindex="-1" data-state="closed" aria-label="Zoom out" data-orientation="horizontal" data-radix-collection-item="" data-sentry-element="IconButton" class="OpenedMobileToolbar_menuButton__Lwedt" data-sentry-source-file="OpenedMobileToolbar.tsx">…</button>�[22m
    1678:  �[2m  - attempting click action�[22m
    1679:  �[2m    - waiting for element to be visible, enabled and stable�[22m
    1680:  68 |
    1681:  69 |   const zoomOutButton = toolbar.getByRole('button', { name: 'Zoom out' })
    1682:  > 70 |   await zoomOutButton.click()
    1683:  |                       ^
    1684:  71 |   await expect(zoomLevelText).not.toHaveText(zoomLevelBefore)
    1685:  72 |
    1686:  73 |   const zoomLevelAfter = await zoomLevelText.textContent()
    1687:  at /home/runner/work/liam/liam/frontend/packages/e2e/tests/e2e/toolbar.test.ts:70:23
    1688:  Error Context: test-results/e2e-toolbar-zoom-out-button-should-decrease-zoom-level-Mobile-Safari-retry1/error-context.md
    1689:  attachment #2: trace (application/zip) ─────────────────────────────────────────────────────────
    1690:  test-results/e2e-toolbar-zoom-out-button-should-decrease-zoom-level-Mobile-Safari-retry1/trace.zip
    1691:  Usage:
    1692:  pnpm exec playwright show-trace test-results/e2e-toolbar-zoom-out-button-should-decrease-zoom-level-Mobile-Safari-retry1/trace.zip
    1693:  ────────────────────────────────────────────────────────────────────────────────────────────────
    1694:  1 failed
    1695:  [Mobile Safari] › tests/e2e/toolbar.test.ts:47:5 › zoom in button should increase zoom level ───
    1696:  2 flaky
    1697:  [Mobile Safari] › tests/e2e/page.test.ts:25:5 › Table node should be highlighted when clicked ──
    1698:  [Mobile Safari] › tests/e2e/toolbar.test.ts:63:5 › zoom out button should decrease zoom level ──
    1699:  9 skipped
    1700:  5 passed (3.3m)
    1701:  ##[error]Process completed with exit code 1.
    1702:  ##[group]Run actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02
    

    * Replaced the previous agent-based approach with a LangGraph workflow (`runChat`) for the `'build'` mode
    * Refined the system prompt to consistently guide the LLM toward proper RFC 6902 JSON Patch output, wrapped in \`\`\`json fences
    * Improved the prompt structure to allow informative commentary outside the JSON Patch block
    * Added fallback mechanism for non-schema-editing messages (i.e., omit JSON Patch)
    * Introduced unit tests for LangGraph flow to ensure format correctness and schema alignment
    * Updated dependencies to include `@langchain/langgraph`
    @qodo-free-for-open-source-projects
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The runChat function has a try/catch block that catches errors but doesn't properly handle the fallback case. The function ends without returning anything in the catch block.

    } catch (error) {
      console.error(
        'StateGraph execution failed, falling back to manual execution:',
        error,
      )
      // some fallback logic
    }

    Unimplemented Mode
    The code throws an error for 'ask' mode but doesn't provide a fallback implementation, which could cause runtime errors in production.

    @qodo-free-for-open-source-projects
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add missing graph edge

    The graph is missing an edge from 'drafted' to 'check', which means the initial
    draft is never validated. Add this edge to ensure the first draft goes through
    validation.

    frontend/apps/app/lib/chat/langGraph.ts [125-139]

     const graph = new StateGraph(ChatStateAnnotation)
     
     graph
       .addNode('buildPrompt', buildPrompt)
       .addNode('drafted', draft)
       .addNode('check', check)
       .addNode('remind', remind)
       .addEdge(START, 'buildPrompt')
       .addEdge('buildPrompt', 'drafted')
    +  .addEdge('drafted', 'check')
       .addEdge('remind', 'check')
     
       // conditional edges
       .addConditionalEdges('check', (s: ChatState) => {
         if (s.valid) return END
         if ((s.retryCount ?? 0) >= 3) return END // give up
         return 'remind'
       })

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 9

    __

    Why: This fixes a critical bug where the initial draft node never connects to the check node, preventing validation of the first response and breaking the entire graph flow.

    High
    Handle non-schema-changing queries

    The check function doesn't properly handle the case when the draft doesn't
    contain a JSON patch (which is valid for non-schema-changing queries). Add a
    condition to mark responses without JSON patches as valid when appropriate.

    frontend/apps/app/lib/chat/langGraph.ts [91-99]

     const check = async (s: ChatState): Promise<Partial<ChatState>> => {
       const m = s.draft?.match(/```json\s+([\s\S]+?)\s*```/i)
    -  if (!m) return { valid: false }
    +  if (!m) {
    +    // If no JSON patch is found but this is a non-schema-changing query, it's valid
    +    if (!s.draft?.includes('```json')) {
    +      return { valid: true }
    +    }
    +    return { valid: false }
    +  }
       try {
         return { valid: true, patch: JSON.parse(m[1]) }
       } catch {
         return { valid: false }
       }
     }
    • Apply / Chat
    Suggestion importance[1-10]: 8

    __

    Why: This addresses a critical logic flaw where valid non-schema-changing responses are incorrectly marked as invalid, breaking the intended functionality described in the system prompt.

    Medium
    Implement missing fallback logic

    The catch block logs an error but doesn't implement any fallback logic,
    potentially causing the function to return undefined. Implement proper fallback
    logic or rethrow the error.

    frontend/apps/app/lib/chat/langGraph.ts [156-163]

     try {
       const graph = new StateGraph(ChatStateAnnotation)
       // ... graph definition ...
       const compiled = graph.compile()
       const result = await compiled.invoke(
         {
           userMsg,
           schemaText,
           chatHistory,
           retryCount: 0,
         },
         {
           recursionLimit: 4, // for avoid deep recursion
         },
       )
     
       return result.draft ?? 'No response generated'
     } catch (error) {
       console.error(
         'StateGraph execution failed, falling back to manual execution:',
         error,
       )
    -  // some fallback logic
    +  // Fallback to direct agent call
    +  const agent = mastra.getAgent('databaseSchemaBuildAgent')
    +  if (!agent) {
    +    throw new Error('Failed to get agent for fallback')
    +  }
    +  const response = await agent.generate([
    +    { role: 'system', content: `Complete Schema Information:\n${schemaText}\n\nPrevious conversation:\n${chatHistory}` },
    +    { role: 'user', content: userMsg },
    +  ])
    +  return response.text
     }

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 7

    __

    Why: The catch block promises fallback logic but doesn't implement it, potentially causing the function to return undefined. The suggestion provides a reasonable fallback implementation.

    Medium
    Learned
    best practice
    Add input parameter validation

    Add input validation at the beginning of the runChat function to ensure all
    required parameters are provided and have the correct types. This prevents
    potential runtime errors when parameters are missing or malformed.

    frontend/apps/app/lib/chat/langGraph.ts [117-123]

     export const runChat = async (
       userMsg: string,
       schemaText: string,
       chatHistory: string,
     ) => {
    +  if (!userMsg || typeof userMsg !== 'string') {
    +    throw new Error('User message is required and must be a string')
    +  }
    +  if (!schemaText || typeof schemaText !== 'string') {
    +    throw new Error('Schema text is required and must be a string')
    +  }
    +  if (typeof chatHistory !== 'string') {
    +    throw new Error('Chat history must be a string')
    +  }
    +  
       try {
         const graph = new StateGraph(ChatStateAnnotation)
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why:
    Relevant best practice - Validate input parameters at the beginning of functions to prevent runtime errors from invalid, undefined, or improperly formatted data.

    Low
    • More

    Copy link
    Member

    @FunamaYukina FunamaYukina left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Sorry for the late review.
    frontend/apps/app/lib/chat/langGraph.ts and others were very helpful in considering the implementation of LangGraph.Thank you for creating the PoC!

    }

    const draft = async (s: ChatState): Promise<Partial<ChatState>> => {
    const agent = mastra.getAgent('databaseSchemaBuildAgent')
    Copy link
    Member

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    📝
    I found that I can use mastra as is! (I thought I had to go back to LangChain...)

    Comment on lines +91 to +99
    const check = async (s: ChatState): Promise<Partial<ChatState>> => {
    const m = s.draft?.match(/```json\s+([\s\S]+?)\s*```/i)
    if (!m) return { valid: false }
    try {
    return { valid: true, patch: JSON.parse(m[1]) }
    } catch {
    return { valid: false }
    }
    }
    Copy link
    Member

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    [Q]
    Is there a plan to enhance the check function to also verify the presence of specific properties, like 'op', similar to how the tests currently do it?

    Copy link
    Member Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    @FunamaYukina

    Good point! I believe validation is necessary from the following perspectives, increasing in complexity as you go:

    • Does it conform to the structure of a valid JSON Patch(RFC 6902)?
    • (If a JSON Schema or valibot schema is defined in schema.json) Does the resulting schema.json after applying the patch conform to that schema?
    • Does applying the patch result in any inconsistencies with the current schema state? (e.g., referencing a non-existent key)

    This is probably something we’ll implement eventually, but with low technical uncertainty, it’s a bit hard to justify doing it right away.

    Copy link
    Member

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Thank you for the details! It seems important to check until we can actually apply!
    Understood. Thank you very much.✨

    @hoshinotsuyoshi
    Copy link
    Member Author

    The materials created in this PoC might be used later, but I'll submit them in a separate PR. Closing this one for now.

    @hoshinotsuyoshi hoshinotsuyoshi deleted the hoshino-poc-chat-op branch July 10, 2025 06:56
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants