refactor(llm): Improving syntax, prompt, types and bug fixes - #367
refactor(llm): Improving syntax, prompt, types and bug fixes#367szanata wants to merge 38 commits into
Conversation
…o only have prompt
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const { steps, sources: sourcesFromResponse } = response; | ||
| const sourcesFromTools = extractSourcesFromSteps( steps ); | ||
| const allSources = sourcesFromTools.concat( asArray( sourcesFromResponse ) ); | ||
| return new Map( allSources.map( s => [ s.url, s ] ) ).values().toArray(); |
There was a problem hiding this comment.
Must-fix (Correctness): dedup key is s.url, but AI SDK sources can contain { type: 'source', sourceType: 'document', id, mediaType, title, filename } entries with no url. Every document source maps to key undefined, so only the last one survives.
This is newly reachable: wrapGeneration/wrapStream now call extractSources unconditionally, whereas v0.11 only overrode sources when sourcesFromTools.length > 0 and otherwise passed response.sources through untouched. Consider keying on s.url ?? s.id.
This comment was marked as outdated.
This comment was marked as outdated.
…tion with fixed heuristics and validations
szanata
changed the title
| * @param {string[]} paths | ||
| * @returns {Skills[]} | ||
| */ | ||
| export const recursiveLoadSkillFile = paths => recursiveLoadSkillPaths( paths, false ); |
There was a problem hiding this comment.
Nice-to-have (Quality): recursiveLoadSkillFile is exported but its only production caller is loadSkills two lines below. Inline the recursiveLoadSkillPaths( paths, false ) call and drop the export.
| export type PromptMessage = { | ||
| /** The role of the message. Examples include 'system', 'user', and 'assistant'. */ | ||
| /** The message role. Authored prompt blocks support 'system', 'user', and 'assistant'. */ | ||
| role: string; |
There was a problem hiding this comment.
Nice-to-have (Quality): after this PR parseContent guarantees the role is one of system / user / assistant (promptRoleSet throws otherwise), so role: string is looser than the runtime contract. role: 'system' | 'user' | 'assistant' would match the tightening this PR is about.
This comment was marked as outdated.
This comment was marked as outdated.
| const stats = lstatSync( path ); | ||
| if ( stats.isSymbolicLink() ) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Nice-to-have (Correctness): the symlink skip also applies to paths listed explicitly in config.skills, so a symlinked skill file is dropped silently — no error, no log, and existsSync above already passed. Consider only skipping symlinks during directory recursion, or logging when an explicitly listed path is skipped.
PR reviewVerdict✅ PASS Findings
Categories
|
1 participant
Summary
Comprehensive refactor of the LLM module in order to fix many bugs and tight the types, signatures, validations, heuristics:
Skills
skillsargument fromgenerateText(),streamText(),generateTextWithStreaming(), andAgent.skills/auto-discovery.skill()helper and theSkillandSkillsArgtypes.Call signatures and validation
Fixed unrestricted native AI SDK call arguments bypassing validation and overriding prompt-owned model/configuration. Generation and Agent APIs now reject unsupported or misplaced arguments before provider I/O. Removed native AI SDK call arguments from
generateText(),generateTextWithStreaming(),streamText(),generateImage(), andAgent, as well asskillsandmaxSteps. These are the supported arguments:generateTextgenerateTextWithStreamingstreamTextgenerateImagepromptpromptDirvariablestoolsoutputtoolChoicestopWhenabortSignalonChunkonFinishonErrorimagesmasknew Agent.generate.generateWithStreaming.streampromptpromptDirvariablestoolsoutputstopWhenmessageStoremessagesabortSignaltoolChoiceonChunkonFinishonErrormaxRetries: 0.GenerateImageInputtype forgenerateImage()imagesandmaskvalues.Fixed invalid Agent constructor and method arguments being forwarded for late failure by validating messages, callbacks, tool choices, and
MessageStoreimplementations at the public boundary.Tools and tool loops
load_skillis last).stopWhen: stepCountIs(maxSteps)from the prompt (default 10).TypeError. They now fail provider-tool validation.Prompt files
Prompt.promptFileDirtoPrompt.fileDir.PromptVariables(Record<string, unknown>, including nested objects and arrays) andPrompt.variables(default{}). LLM APIvariablesarguments use the same type.Prompt.config.skillsvarying between missing, a string, and a string array. It is always astring[]after load.Prompt.config.maxSteps(positive integer, default 10). It replaces the old argument and is required on the public type.Prompt.instructionsallowingundefinedin its public contract. It is alwaysstring | nullafter load (chat prompts arenull).options="<name>"is resolved duringloadPromptagainstconfig.messageOptionsinto optionalPromptMessage.providerOptions;PromptMessage.attributesis removed.generateImage()requires instruction mode.system,user, andassistantblocks, with no root text between blocks.<tool>blocks as string messages even though AI SDK requires structured tool-result parts. They now fail at load; structured tool messages remain supported through AgentmessagesandmessageStore.<tag>escape hint; different-name tags remain message content.=and>inside quoted values. Bareoptions, unknown option names, and invalid quote pairs throw at load.optionsvalues invalidating templated prompts. They now behave as no per-message options.configaccepting ignored or invalid values:modelmust be a non-empty string.maxTokensmust be a positive integer.Streaming and Agent message store
streamText()andAgent.stream()onErrorandonFinishcallbacks are fire-and-forget observers. Output maps and forwards provider errors, and logs and ignores observer exceptions and rejected promises.conversationStoretomessageStoreandConversationStoretoMessageStore.createMemoryConversationStore(). The caller supplies aMessageStore(getMessages/addMessages).Agent.stream()to persist tomessageStorewhenfinishReasonis not'error'.onFinish. Failures are logged and stream finalization continues.Tracing, sources, and response types
generateText(),streamText(),generateTextWithStreaming(),generateImage(), andAgent:inputto{ prompt }(the loaded object). Removed the v0.11 filenameprompt, siblingvariables, andloadedPrompt.coston endoutput(also still a trace attribute andcost:llm:requestwhen present).sourcesFromToolstosources(merged tool + provider sources).idbut nourl.sourcesas an array.ExtractedSourceto the AI SDKgenerateTextsources item type (url and document).LLMCallCostandLLMUsageEventare theTracing.Attribute.LLMUsageinstance type from@outputai/core;response.costand streamonFinishcostare that instance, ornullwhen pricing is missing. StreamonFinishtypes also include wrappedresultand mergedsources.AI SDK exports and public parameter types
aitoaiSdk(both code and types).tool,Output,smoothStream,stepCountIs,hasToolCall,jsonSchema) and AI SDK type re-exports (ToolSet,FinishReason,ModelMessage, and others). Use theaiSdknamespace (or import fromai) for those.GenerateTextAiSdkOptions,StreamTextAiSdkOptions, andGenerateImageAiSdkOptionsaliases. UseGenerateTextParameters,StreamTextParameters, andGenerateImageParameters.OutputAgentGenerateWithStreamingParametersto no longer accept an output type argument.