Repository navigation
test(agent): pin Note/MarkdownNote materialization from agent add_node ops #18363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
6fd84c1
test(agent): pin Note/MarkdownNote materialization from agent add_nod…
christian-byrne fa1f47a
test(agent): prove note nodes persist without host replay
christian-byrne 9db4164
Merge comfyorg/main into test/pm983-agent-virtual-note-nodes
christian-byrne ba7657f
test(agent): isolate note reopen from local state
christian-byrne deec281
test(agent): prove note reopen fetches the saved file
christian-byrne a487632
Merge comfyorg/main into test/pm983-agent-virtual-note-nodes
christian-byrne 091084f
test(agent): schema-validate the saved workflow in the note-nodes spec
christian-byrne fc0909e
Merge remote-tracking branch 'origin/main' into vnote
christian-byrne eb0cde2
Merge origin/main into test/pm983-agent-virtual-note-nodes
ampagent f17cacb
test: drop renderer imports from the virtual note materializer test
christian-byrne 8cbb35e
test: switch tabs through the Topbar helper in the virtual note spec
christian-byrne 306ad05
test: type the app mock in the virtual note materializer test
christian-byrne 5c5773b
test: move the virtual note scenario data into a fixture data module
christian-byrne 241558b
test: serve sentinel note text on reopen so cached state cannot pass
christian-byrne 86eb19c
test: drop the comments this PR added to the virtual note tests
christian-byrne e38e40e
test: fail the sentinel swap when the saved workflow lacks the text
christian-byrne 053202b
Merge origin/main into test/pm983-agent-virtual-note-nodes
christian-byrne 88a175b
test(agent): drop the unit half that the graph-API rewrite deleted th…
christian-byrne 61ffb37
test(agent): drop redundant savedWorkflowPath docblock
christian-byrne 4176a33
test(agent): drop redundant savedWorkflowContent docblock
christian-byrne 362b2de
test(agent): drop redundant replaceInSavedWorkflow docblock
christian-byrne fb13fad
test(agent): drop redundant refuseHostSubscribes docblock
christian-byrne e9be2ad
test(agent): drop redundant VIRTUAL_NODE_ADDS docblock
christian-byrne 8c5e390
test(agent): type the virtual-note builder as a semantic add_node op
christian-byrne 4f03ae4
test(agent): reopen the saved workflow through AgentPanel.selectWorkflow
christian-byrne af88496
test(agent): assert the Markdown note renders a level-1 heading
christian-byrne File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
45 changes: 45 additions & 0 deletions
45
browser_tests/fixtures/data/agent/agentVirtualNoteNodes.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import type { GraphOperation } from '@/workbench/extensions/agent/crdt/graphOperations' | ||
|
|
||
| import type { RecordedGraphOperation } from '@e2e/fixtures/data/agent/agentConversation' | ||
|
|
||
| export const NOTE_ID = 9001 | ||
| export const MARKDOWN_ID = 9002 | ||
| export const NOTE_TEXT = 'agent note text' | ||
| export const MARKDOWN_HEADING = 'Agent markdown heading' | ||
| export const MARKDOWN_BODY = 'markdown body line' | ||
| export const MARKDOWN_TEXT = `# ${MARKDOWN_HEADING}\n\n${MARKDOWN_BODY}` | ||
| export const SAVED_NAME = 'Virtual note nodes' | ||
| export const SERVED_NOTE_TEXT = 'served note text' | ||
| export const SERVED_MARKDOWN_HEADING = 'Served markdown heading' | ||
|
|
||
| function addVirtualNode( | ||
| id: number, | ||
| type: 'Note' | 'MarkdownNote', | ||
| pos: [number, number], | ||
| text: string | ||
| ): Extract<GraphOperation, { op: 'add_node' }> { | ||
| return { | ||
| op: 'add_node', | ||
| node_id: id, | ||
| class_type: type, | ||
| pos, | ||
| node: { | ||
| id, | ||
| type, | ||
| pos, | ||
| size: [320, 180], | ||
| mode: 0, | ||
| flags: {}, | ||
| order: 0, | ||
| inputs: [], | ||
| outputs: [], | ||
| properties: {}, | ||
| widgets_values: [text] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export const VIRTUAL_NODE_ADDS: RecordedGraphOperation[] = [ | ||
| addVirtualNode(NOTE_ID, 'Note', [1400, 100], NOTE_TEXT), | ||
| addVirtualNode(MARKDOWN_ID, 'MarkdownNote', [1400, 400], MARKDOWN_TEXT) | ||
| ] |
143 changes: 143 additions & 0 deletions
143
browser_tests/tests/agent/agentVirtualNoteNodes.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| import type { AgentRunMode, JobsListResponse } from '@comfyorg/ingest-types' | ||
| import { expect } from '@playwright/test' | ||
|
|
||
| import type { ModelFolderInfo } from '@/platform/assets/schemas/assetSchema' | ||
|
|
||
| import { agentConversationTest as test } from '@e2e/fixtures/agentConversationFixture' | ||
| import { AgentPanel } from '@e2e/fixtures/components/AgentPanel' | ||
| import { | ||
| MARKDOWN_BODY, | ||
| MARKDOWN_HEADING, | ||
| MARKDOWN_ID, | ||
| MARKDOWN_TEXT, | ||
| NOTE_ID, | ||
| NOTE_TEXT, | ||
| SAVED_NAME, | ||
| SERVED_MARKDOWN_HEADING, | ||
| SERVED_NOTE_TEXT, | ||
| VIRTUAL_NODE_ADDS | ||
| } from '@e2e/fixtures/data/agent/agentVirtualNoteNodes' | ||
| import { jsonRoute } from '@e2e/fixtures/utils/jsonRoute' | ||
|
|
||
| test.describe( | ||
| 'Agent adds frontend-only note nodes', | ||
| { tag: ['@cloud', '@agent', '@vue-nodes'] }, | ||
| () => { | ||
| test.use({ conversationCase: 'agent-rec-set-widget-existing' }) | ||
|
|
||
| test.beforeEach(async ({ page }) => { | ||
| const folders: ModelFolderInfo[] = [] | ||
| await page.route('**/api/experiment/models', (route) => | ||
| route.fulfill(jsonRoute(folders)) | ||
| ) | ||
| const jobs: JobsListResponse = { | ||
| jobs: [], | ||
| pagination: { offset: 0, limit: 200, total: 0, has_more: false } | ||
| } | ||
| await page.route('**/api/jobs?*', (route) => | ||
| route.fulfill(jsonRoute(jobs)) | ||
| ) | ||
| const runMode: AgentRunMode = { mode: 'ask_approval', credit_limit: null } | ||
| await page.route('**/api/agent/run-mode', (route) => { | ||
| if (route.request().method() !== 'GET') return route.fallback() | ||
| return route.fulfill(jsonRoute(runMode)) | ||
| }) | ||
| }) | ||
|
|
||
| test('renders Note and MarkdownNote with their own text, and keeps both across a tab switch and a reload', async ({ | ||
| agentConversation, | ||
| page | ||
| }, testInfo) => { | ||
| test.setTimeout(120_000) | ||
| const { topbar, vueNodes } = agentConversation | ||
| const noteText = () => | ||
| vueNodes.getNodeLocator(String(NOTE_ID)).getByLabel('text', { | ||
| exact: true | ||
| }) | ||
| const markdownText = () => | ||
| vueNodes.getNodeLocator(String(MARKDOWN_ID)).getByLabel('text', { | ||
| exact: true | ||
| }) | ||
|
|
||
| const expectBothNotes = async ( | ||
| note = NOTE_TEXT, | ||
| heading = MARKDOWN_HEADING | ||
| ) => { | ||
| await expect(noteText()).toHaveValue(note) | ||
| await expect( | ||
| markdownText().getByRole('heading', { | ||
| name: heading, | ||
| level: 1, | ||
| exact: true | ||
| }) | ||
| ).toBeVisible() | ||
| await expect(markdownText()).toContainText(MARKDOWN_BODY) | ||
| await expect(markdownText()).not.toContainText(note) | ||
| } | ||
|
|
||
| await test.step('host adds a Note and a MarkdownNote', async () => { | ||
| await agentConversation.persistSavedWorkflow() | ||
| await agentConversation.runTurns() | ||
| agentConversation.pushHostOps(VIRTUAL_NODE_ADDS) | ||
| await expectBothNotes() | ||
| }) | ||
|
|
||
| await test.step('both survive switching tabs away and back', async () => { | ||
| const before = agentConversation.subscribeCount() | ||
| await topbar.openBlankTabAndReturn() | ||
| await expect | ||
| .poll(() => agentConversation.subscribeCount()) | ||
| .toBe(before + 1) | ||
| await expectBothNotes() | ||
| await agentConversation.panel.hover() | ||
| }) | ||
|
|
||
| await test.step('the saved workflow file carries both notes', async () => { | ||
| await topbar.saveWorkflowAs(SAVED_NAME) | ||
| await expect(topbar.getActiveTab()).toContainText(SAVED_NAME) | ||
| await expect | ||
| .poll(() => agentConversation.savedWorkflowPath()) | ||
| .toContain(SAVED_NAME) | ||
| const savedNodes = (await agentConversation.savedWorkflowContent()) | ||
| .nodes | ||
| expect(savedNodes.find((node) => node.id === NOTE_ID)).toMatchObject({ | ||
| type: 'Note', | ||
| widgets_values: [NOTE_TEXT] | ||
| }) | ||
| expect( | ||
| savedNodes.find((node) => node.id === MARKDOWN_ID) | ||
| ).toMatchObject({ | ||
| type: 'MarkdownNote', | ||
| widgets_values: [MARKDOWN_TEXT] | ||
| }) | ||
| }) | ||
|
|
||
| await test.step('both survive reload and reopen without host replay', async () => { | ||
| agentConversation.refuseHostSubscribes() | ||
| agentConversation.replaceInSavedWorkflow(NOTE_TEXT, SERVED_NOTE_TEXT) | ||
| agentConversation.replaceInSavedWorkflow( | ||
| MARKDOWN_HEADING, | ||
| SERVED_MARKDOWN_HEADING | ||
| ) | ||
| const restoredContent = page.waitForResponse( | ||
| (response) => | ||
| response.request().method() === 'GET' && | ||
| decodeURIComponent(new URL(response.url()).pathname) === | ||
| `/api/userdata/workflows/${SAVED_NAME}.json`, | ||
| { timeout: 60_000 } | ||
| ) | ||
| await page.reload({ waitUntil: 'domcontentloaded' }) | ||
| await expect(agentConversation.panel).toBeVisible({ timeout: 30_000 }) | ||
| await new AgentPanel(page).selectWorkflow(SAVED_NAME) | ||
| expect((await restoredContent).ok()).toBe(true) | ||
| await expectBothNotes(SERVED_NOTE_TEXT, SERVED_MARKDOWN_HEADING) | ||
| const afterPath = testInfo.outputPath('after-reload.png') | ||
| await page.screenshot({ path: afterPath }) | ||
| await testInfo.attach('after-reload', { | ||
| path: afterPath, | ||
| contentType: 'image/png' | ||
| }) | ||
| }) | ||
| }) | ||
| } | ||
| ) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: This proves that the saved-file GET succeeded, but not that its body produced the graph. Browser-local workflow state and the mocked server file contain the same note values, so a loader that fetches this URL and then restores the cached workflow still passes
expectBothNotes(). Please make the sources distinguishable, for example by replacing the fake's served body with a schema-valid sentinel variant before reload and asserting those sentinel values after reopen, or by clearing only the cached payload while preserving the picker pointers. This is the remaining part of the earlier cached-workflow finding.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 241558b and e38e40e. Reopen now gets sentinel text; local state can no longer pass.
Full context for agent readers
replaceInSavedWorkflowmakes the persistence fake serve the saved file with the note text and markdown heading swapped forserved note text/Served markdown heading. The reopen asserts those values, and that the markdown does not contain the note text.Expected "served note text"/Received "agent note text". With it, the spec passes locally.