-
Notifications
You must be signed in to change notification settings - Fork 164
feat: route generate data CTA to developer chat #8505
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
base: main
Are you sure you want to change the base?
Conversation
e20b1a7 to
d15d162
Compare
ericpgreen2
left a comment
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.
Architecture Feedback
1. "Navigate on AI file write" should be part of the chat infrastructure
The FileWriteListener class and maybeNavigateToGeneratedFile function implement a behavior that belongs in the developer chat infrastructure, not in generate-sample-data.ts. This behavior—navigating to a file when the AI writes it—applies to all developer chat interactions (e.g., "create a new canvas YAML file"), not just sample data generation.
However, auto-navigating on every file write has a UX problem: what happens when the AI creates multiple files? Navigating on each write would be jarring; navigating only on the last write requires the frontend to guess when the AI is "done."
Recommended approach: Introduce a navigate tool that the AI explicitly invokes when it wants to direct the user's attention to a file. This gives the AI control over the UX—it can navigate immediately for single-file operations, or wait until it finishes a multi-file operation and navigate to the primary artifact.
The existing tool-registry.ts can be extended to handle side effects:
export interface ToolConfig {
renderMode: ToolRenderMode;
createBlock?: (...) => ToolBlockType | null;
/** Side effect to run when tool result is received */
onResult?: (callMessage: V1Message, resultMessage: V1Message) => void;
}
const TOOL_CONFIGS: Partial<Record<string, ToolConfig>> = {
// ... existing configs ...
[ToolName.NAVIGATE]: {
renderMode: "inline",
onResult: (_call, result) => {
const path = extractPath(result);
goto(`/files${path}`);
},
},
};This eliminates maybeNavigateToGeneratedFile, FileWriteListener, and the awkward binding pattern in DeveloperChat.svelte.
2. Remove sourceImportedPath.set() calls from sample data generation
The "source imported" modal should not be triggered for files created through the AI chat experience. The chat should propose the next step—we should not have a no-code UX that competes with the AI chat UX. For example, after generating sample data, the AI could prompt: "How does this look? Any adjustments to the data, or would you like me to create a dashboard from this?" This is a richer experience than a static modal.
The current code calls sourceImportedPath.set(path) in both generateSampleDataWithDevChat and generateSampleDataWithOverlay, which triggers the modal. These calls should be removed. FileAndResourceWatcher will continue to handle the modal for user-initiated imports (drag-drop, Add Data modal).
Minor Issues
3. createResource wrapper in CreateExploreDialog.svelte is redundant
The function just forwards to createResourceAndNavigate. The onClick handler can call createResourceAndNavigate directly.
4. Silent catch block in OnboardingWorkspace.svelte:56-58
} catch {
// no-op
}This swallows errors silently. Consider logging to console for debuggability.
5. waitUntil condition may be inverted
In generate-sample-data.ts:161-164 and :215-218:
await waitUntil(
() => !get(conversation.isStreaming) && !get(conversation.streamError),
-1,
);This waits until streaming stops AND there's no error. If streamError becomes truthy, the condition !get(conversation.streamError) is false, so the waitUntil never resolves. Should this be || instead of &&?
6. Feature flag default
developer_chat is set to true in runtime/feature_flags.go. The PR description notes this will be reverted before merge.
Developed in collaboration with Claude Code
3,4. Done |
a1f3f72 to
bd2ce30
Compare
if we remove this prompt box, would it be possible to create a CTA object in the chatbox experience? To start we can just replicate the same primary CTA button: "Generate Dashboard" cc @nishantmonu51 |
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.
Architecture Feedback
1. Remove FileWriteListener class; simplify the dev chat path
The FileWriteListener class introduces infrastructure that will be short-lived—it's only needed for the overlay path, which becomes dead code once the developer_chat flag is lifted.
Recommended approach:
- Dev chat path: Remove
FileWriteListenerentirely. The chat is visible, so users see the AI's response directly. Thenavigatetool handles navigation. No need forcreated/lastReadFiletracking or post-hoc notifications. - Overlay path: Keep the detection logic inline (don't extract it into a class). Mark it clearly as legacy code to remove when the feature flag is lifted.
The dev chat path simplifies to:
async function generateSampleDataWithDevChat(agentPrompt, conversationManager) {
conversationManager.enterNewConversationMode();
const conversation = get(conversationManager.getCurrentConversation());
conversation.cancelStream();
overlay.set(null);
sidebarActions.startChat(agentPrompt);
await waitUntil(() => get(conversation.isStreaming));
await waitUntil(() => !get(conversation.isStreaming), -1);
}2. Render the navigate tool as a top-level block
Currently renderMode: "hidden" means navigate calls aren't visible. Users should see that navigation occurred—it's a meaningful action.
Rather than creating a dedicated NavigateBlock component, introduce a generic tool-call block type. Consider adding a new ToolCall variant (e.g., variant="standalone") to distinguish from variant="block" which implies rich content below with a collapsible toggle.
3. Remove navigate-specific gating in conversation.ts
The current code special-cases navigate:
if (response.message.tool === ToolName.NAVIGATE && response.message.type === MessageType.CALL) {
const config = getToolConfig(response.message.tool);
config?.onResult?.(response.message);
}This should call onResult for any tool:
if (response.message.type === MessageType.CALL) {
const config = getToolConfig(response.message.tool);
config?.onResult?.(response.message);
}4. Use existing addLeadingSlash utility
maybePrependSlash in file-path-utils.ts duplicates addLeadingSlash from entity-mappers.ts. Use the existing utility.
5. Fix prompt inconsistency
In developer_agent.go:164, the prompt says "Use type 'file'" but the schema field is kind. These should match.
6. Rephrase prompt without caps
"Generate a NEW model..." — capitalizing "NEW" doesn't look clean. Rephrase: "Generate a new model with sample data based on the following user input: ..."
Developed in collaboration with Claude Code
|
LGTM except there isn't a clear CTA to a dashboard now. I would do two things:
|
da63404 to
c24f0cf
Compare
c24f0cf to
910de33
Compare
|
We will be adding CTAs in #8655 since we will have an aggregation block there. |



Note: This flips the developer flag for ease of testing. Will be disabled before merging.
Checklist: