Repository navigation
Sync AI updates and fix the Doctum deployment - #65
Merged
Merged
Conversation
Every Doctum deployment since the pnpm 12 upgrade has failed. The deploy step used wrangler-action, which installs Wrangler with `pnpm add` at the workspace root. pnpm 12 rejects that twice: the global ignore-workspace-root-check setting is no longer supported, and once it is bypassed the install fails on unapproved esbuild and workerd build scripts. Run Wrangler through `pnpm dlx wrangler@4 pages deploy`, passing the Cloudflare credentials and project name through the step environment. This installs only Wrangler, honors the workspace's minimum release age and leaves the checkout untouched. A local run of the command reaches Wrangler's credentials check.
Ask contributors reconciling upstream changes to add missing explanations for non-obvious Hypervel behavior they keep, so later syncs can see why it exists.
Port laravel/ai#1090 (laravel/ai 1.x at d44ae14d17fb). Embeddings, image, audio, transcription, reranking and classification requests, file uploads and store changes dispatched a starting and a success event but nothing on failure, so listeners could not close a failed call. Add AudioFailed, ImageFailed, TranscriptionFailed, RerankingFailed, StoringFileFailed, CreatingStoreFailed, AddingFileToStoreFailed and RemovingFileFromStoreFailed, dispatched through Provider::dispatchingFailure() for each failed attempt, including attempts failover recovers from. The existing EmbeddingsFailed and ClassificationFailed events move onto the same helper. Errors thrown by success listeners still do not produce failure events. The helper keeps Hypervel's hasListeners() guard, so passive observers alone do not trigger optional failure events. The new events type the provider as the contract, like their starting events. Classifies also carries laravel/ai#1134's state and attachment docblock types. Port upstream's operation, file and store event tests and remove the Hypervel embeddings tests they duplicate, keeping the check that a failed attempt dispatches no success event. Formatting, static analysis and the AI and Sentry suites pass.
Port laravel/ai#1133 (laravel/ai 1.x at d44ae14d17fb). A failed turn replays its unanswered tool calls with an interrupted placeholder result. Hypervel already kept the stored result ID, so OpenAI, Azure OpenAI and xAI receive the call_id their output is paired by. Match upstream's named arguments and port its regression test, which fails without the result ID.
Port laravel/ai#1137's test (laravel/ai 1.x at d44ae14d17fb). Its source change restores $incrementing for Laravel 12, which lacks the WithoutIncrementing attribute. Hypervel's conversation models use HasUuids and are already non-incrementing, so only the test applies: a conversation and message created through Eloquent keep their string keys.
Port laravel/ai#1134 (laravel/ai 1.x at d44ae14d17fb). TypeSafe and OpenRouter classification rejected image attachments. TypeSafe now sends images as referenced assets with its images header, OpenRouter appends data URL image parts to the state, and OpenAI and OpenAI-compatible classification label images given string keys, so questions can refer to them by name. ResolvesInlineImages shares the inline image checks. Fix two defects. Every classification state docblock said array<string, mixed>, although list states are supported, so list states failed static analysis; widen them to array<array-key, mixed>, pinned by a types fixture. TypeSafe also dropped an attachment named "images" from a structured state when unnamed attachments were given, while still uploading it; that combination is now rejected before any request. Port upstream's TypeSafe, OpenRouter, OpenAI, fake and integration tests with the updated fixture images and the text-only classifier helper. ClassificationFakeTest also drops the classification failure assertions that duplicate laravel/ai#1090's tests. Formatting, static analysis and the AI suite pass.
Port laravel/ai#1142 (laravel/ai 1.x at d44ae14d17fb), updating the workbench lockfile to source-map-js 1.2.2 with npm.
Port laravel/ai#1140 and laravel/ai#1146 (laravel/ai 1.x at d44ae14d17fb), which change the same file gateways. File, store and document IDs went into request paths unchanged, so an ID containing "/", "?", "#" or ".." could reach a different endpoint, such as a file ID that deletes a vector store. EncodesResourceIds now encodes each ID as one path segment and rejects empty and dot-segment IDs, in the OpenAI, Azure OpenAI, Anthropic, OpenRouter and Gemini gateways. Gemini still strips its own prefixes, but a document path from another store no longer names that document. Stored and retrieved file responses now keep the provider's raw HTTP response, which is dropped when they are serialized, so provider metadata such as a file's size is available. Port upstream's resource ID and gateway tests. Formatting, static analysis and the AI suite pass.
Port laravel/ai#1149 (laravel/ai 1.x at d44ae14d17fb). ImageResponse::toHtml() escaped only the alt text, so a MIME type or image payload containing quotes could break out of the src attribute. Escape both, with upstream's test.
Port laravel/ai#1145 (laravel/ai 1.x at d44ae14d17fb). Inlining a $ref shared by sibling properties at every use can expand a small MCP tool schema exponentially. Hypervel's normalizer already bounded each call; adopt upstream's 10,000-node limit and message, which bound the same work. McpTool::schema() still drops the parameters of a schema that exceeds the limit. Port upstream's normalizer and MCP tool tests, replacing the Hypervel test they duplicate.
Port laravel/ai#1143 (laravel/ai 1.x at d44ae14d17fb). Text an Anthropic response wrote before and after a server tool, such as a web search, ran together. Put a blank line between those remarks unless the line already breaks there, and keep citation-split text joined. Streaming adds the separator to the first delta after the tool, while replayed blocks keep their original text, so streamed and final text match. Port upstream's response and streaming tests.
Port laravel/ai#1138 (laravel/ai 1.x at d44ae14d17fb). prompt() was declared as returning AgentResponse, so static analysis rejected array access on a structured agent's response. Promptable::prompt() and agent() now carry conditional return types, pinned by a types fixture. Structured agents now always return a StructuredAgentResponse, with an empty payload when a run produces none, such as a stopped run, a string fake or a Bedrock approval pause. Unlike upstream, the response type is chosen by HasStructuredOutput rather than by a filled schema, so agents with an empty schema also match the declared type; their text remains available and the response casts to an empty payload. Port upstream's tests and fixture agent, adapting the empty-schema test to the corrected behavior. Formatting, static analysis and the AI suite pass.
Port laravel/ai#1144 (laravel/ai 1.x at d44ae14d17fb). Gemini filter keys were interpolated unchecked, and conditions were not grouped although AIP-160 gives OR higher precedence than AND. Hypervel already escaped values and supported whereNotIn(); adopt upstream's formatter, which also validates keys and parenthesizes every condition. Also fix empty whereIn() and whereNotIn() lists, which produced an invalid empty group. An empty exclusion list is now omitted, and an empty inclusion list throws before the request, since Gemini has no portable condition that matches nothing. Port upstream's tests, folding Hypervel's duplicate filter tests into them.
Document laravel/ai#1090, #1134, #1138, #1144 and #1146 in the AI SDK docs, porting laravel/docs 13.x at 12080c72699b where it covers them: - list the new failure events and when they are dispatched - port the Classifying Images section, adapted for the providers that now accept images and for named images, with its table of contents entry and upstream's missing Reranking Collections and Using Stored Files in Conversations entries - note that a structured run without a payload returns an empty structured value - note Gemini's metadata filter key and empty-list rules - show a stored file's raw HTTP response
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoSync AI SDK improvements and restore Doctum deployment
AI Description
Diagram
High-Level Assessment
Files changed (88)
|
Code Review by Qodo
1. Gemini imports target the wrong file
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| }; | ||
|
|
||
| return "{$storeId}/documents/{$documentId}"; | ||
| return "{$storeId}/documents/" . $this->resourceId(Str::chopStart($documentId, ["{$storeId}/documents/", 'documents/', 'files/'])); |
There was a problem hiding this comment.
2. Gemini cannot remove full-path documents 🐞 Bug ≡ Correctness
normalizeDocumentId() compares a supplied full document path against a store prefix whose ID has already been URL-encoded. If the store ID contains a reserved character, the raw full path does not match that prefix and the entire path is encoded and appended as though it were the document ID.
Agent Prompt
## Issue description
Full Gemini document paths fail to normalize when the store ID contains characters that are encoded for the request URL.
## Fix Focus Areas
- src/ai/src/Gateway/Gemini/GeminiStoreGateway.php[162-172]
- src/ai/src/Gateway/Gemini/GeminiStoreGateway.php[229-232]
## Recommended Fix
Compare and strip the document's full-path prefix using the unencoded store resource name before encoding the store and document IDs independently for the URL. Test a full document path with a reserved character in its store ID.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Member
Author
There was a problem hiding this comment.
Keeping this. Gemini file search store IDs only contain lowercase letters, digits and dashes, per the FileSearchStore resource docs (https://ai.google.dev/api/file-search/file-search-stores#FileSearchStore), so encoding never changes a real store ID and the full-path prefix always matches.
 [High impact] The PR appears safe to merge; both earlier findings are fixed. SummaryUpdates the AI SDK and switches Doctum deployment to The latest changes address both earlier findings:
Both fixes have regression tests. No new actionable issues were found. Tests were inspected, not run. Reviews (2) · Last reviewed commit: "Ignore empty Anthropic text blocks after..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
laravel/ai#1134 (laravel/ai 1.x at d44ae14d17fb) merged image references into a structured TypeSafe state with an array spread, which renumbers integer keys. PHP stores numeric string keys such as record IDs as integers, so a state keyed by IDs lost them once images were attached. Merge with a union instead; the existing duplicate-key check already rejects overlaps, so no state value is replaced. Add an integer-key case to the image reference states. Formatting, static analysis and the AI suite pass.
laravel/ai#1143 (laravel/ai 1.x at d44ae14d17fb) added a blank line before an empty text block that followed a server tool, so the final text gained a separator the model never wrote. The block also used up the pending separator, so a following line break was doubled. Streaming skips empty deltas, so streamed and final text differed. Skip empty blocks before applying the separator, as streaming does. Cover an empty block at the end, before text and before text that starts a new line. Formatting, static analysis and the AI suite pass.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Laravel Updates
/,?,#or..could otherwise reach a different endpoint, such as a file ID that deletes a vector store.ImageResponse::toHtml(). Only the alt text was escaped, so quotes in either could break out of thesrcattribute.$refexpansion in the schema normalizer. Hypervel already capped each call at 20,000 nodes, and the lower limit bounds the same work. MCP tools still drop the parameters of a schema over the limit.prompt()andagent()conditional return types, so static analysis accepts array access on a structured agent's response. Structured agents always return aStructuredAgentResponse, with an empty payload when a run produces none, such as a stopped run or a Bedrock approval pause. Document the empty structured value.whereNotIn(). Document Gemini's key rules.Additional Hypervel Fixes
pnpm dlx wrangler@4. The Cloudflare action installed Wrangler withpnpm addat the workspace root, which pnpm 12 rejects, so every deployment since the pnpm 12 upgrade failed.imagesalongside unnamed attachments. Upstream replaced that attachment's reference with the unnamed images but still uploaded it.AgentResponsefor them, contradicting the declared return type. Their text remains available.whereIn()andwhereNotIn()lists, which produced an invalid empty group. An empty exclusion list is omitted, and an empty inclusion list throws before the request, since Gemini has no portable condition that matches nothing.The affected suites, formatting and static analysis pass. CI will run the full suite and supported service matrix.
Summary by cubic
Syncs upstream Laravel AI updates to Hypervel's AI SDK and fixes the broken Doctum deployment.
The deployment failure: the Cloudflare action installed Wrangler with
pnpm addat the workspace root, which pnpm 12 rejects, so every deployment has failed since the pnpm 12 upgrade. It now runspnpm dlx wrangler@4instead.AI changes
ImageResponse::toHtml().$refexpansion.prompt()andagent()conditional return types so structured agents always return aStructuredAgentResponse.whereIn()andwhereNotIn()lists.imagesalongside unnamed attachments, and asks contributors reconciling upstream changes to explain non-obvious Hypervel behavior.source-map-jsto 1.2.2 in the AI workbench and documents the new events and behaviors.Written for commit 403427f. Summary will update on new commits.
Note
Add image classification attachments, failure events, and resource ID encoding to AI gateways
ResolvesInlineImagesconcern.Provider::dispatchingFailurehelper.EncodesResourceIdsconcern, so IDs with path or query characters stay within one URL segment.FileResponseandStoredFileResponseretain the raw provider HTTP response.StructuredAgentResponse, with empty structured data when no structured output was produced. Anthropic text after tool blocks gets a two-newline separator.SchemaNormalizernode budget drops from 20,000 to 10,000, so schemas that previously normalized now throw;GeminiProvider::formatMetadataConditionis removed and Gemini metadata filters now reject empty inclusion lists and invalid keys;ImageResponse::toHtmlnow escapes MIME type and image data.Macroscope summarized b44851e.