Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 10
fix(chat): write the resolved model to chats.model_id at provision time#830
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
8dd9e3124d51889bb063bbb879e6File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -35,12 +35,15 @@ export async function createSessionWithInitialChat({ | ||
| title, | ||
| chatTitle, | ||
| artistId, | ||
| modelId, | ||
| }: { | ||
| accountId: string; | ||
| workspaceAccountId?: string; | ||
| title: string; | ||
| chatTitle: string; | ||
| artistId?: string; | ||
| /** Model recorded on the chat row — provenance, never left to a column default (chat#1956). */ | ||
| modelId: string; | ||
| }): Promise<CreateSessionWithChatResult> { | ||
| const cloneUrl = await ensurePersonalRepo({ accountId: workspaceAccountId ?? accountId }); | ||
| if (!cloneUrl) return { ok: false, reason: "repo" }; | ||
| @@ -50,7 +53,12 @@ export async function createSessionWithInitialChat({ | ||
| ); | ||
| if (!session) return { ok: false, reason: "insert" }; | ||
| const chat = await insertChat({ id: generateUUID(), session_id: session.id, title: chatTitle }); | ||
| const chat = await insertChat({ | ||
| id: generateUUID(), | ||
| session_id: session.id, | ||
| title: chatTitle, | ||
| model_id: modelId, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: This PR centralizes the invariant that "every chat writer persists a model explicitly" and the follow-up DB PR drops the Prompt for AI agentsContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid concern, deliberate non-fix: migrateRoom.ts backfills legacy rooms whose actual model is unknown. After recoupable/database#56, an omitted model_id inserts NULL — which for a legacy room is the honest value ("not recorded"), exactly the semantics the migration's column comment defines. Writing DEFAULT_CHAT_MODEL_ID there would fabricate provenance for historical rows, the precise failure chat#1956 exists to end. The invariant is better stated as: every writer of NEW chats records the model that will run; the backfill records what it knows, which is nothing. | ||
| }); | ||
| if (!chat) { | ||
| const rolledBack = await deleteSessionById(session.id); | ||
| if (!rolledBack) { | ||
Uh oh!
There was an error while loading. Please reload this page.
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: When a run request supplies
model: "", this line persists an emptymodel_idinstead of a usable default. Reject or normalize empty model IDs before provisioning so the persisted provenance and the model sent to the workflow are valid.Prompt for AI agents