feat(backend-native, server-core): support the chatCompletion config hook - #11733
feat(backend-native, server-core): support the chatCompletion config hook#11733paveltiunov wants to merge 4 commits into
Conversation
Documents the `chatCompletion` config hook the LLM Gateway provider calls, with the LangChain and plain async-iterable forms, the request fields the hook receives, and the troubleshooting cases specific to it. Also corrects the Network configuration section, which said Cube always connects to the model provider from its control plane. That is now true of every provider except this one — the LLM Gateway request is made by the customer's own deployment, so a gateway with no public ingress needs no allowlisting or peering. That inversion is the reason the provider exists, so the old wording would have sent readers looking for an allowlist that does not apply. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016hUrmyUJnJ9ta9aGM6oi7H
…hook
Lets a deployment declare the `chat_completion` hook that Cube Cloud's LLM
Gateway model provider calls, and stops Core rejecting its camelCase
counterpart outright.
Three changes, each needed for a different reason:
- `Configuration.chat_completion` plus `"chat_completion"` in the
`cube_config.rs` allow-list. That list is what decides whether a
`cube.py` attribute reaches JavaScript at all — an unlisted name is
dropped in silence, with no error pointing at the omission. Both halves
ship in one native artifact, since the Python module is embedded with
`include_str!`.
- `chatCompletion` in `optionsValidate`. The schema rejects unknown keys,
so without an entry every deployment declaring the hook fails to start
with "Invalid cube-server-core options". Core does not read the option;
it accepts and ignores it.
- `CoreCreateOptions.chatCompletion`, so a `cube.ts` config that sets it
type-checks. Typed as `unknown` deliberately: the value may be a chat
model instance, a factory, or a stream of chunks, and Core has no
dependency on the model library that would let it name any of those.
The Python contract is narrower than the JavaScript one and the tests
pin why: the bridge carries scalars, lists, dicts and plain functions, so
a hook returns a list of chunks or a `{"next": fn}` closure — a model
object throws, which the third test asserts rather than leaves as folklore.
Docs cover both languages, including the chunk format a hand-written hook
needs. Agents call tools every turn, so a hook that only emits `content`
cannot serve one; that was worth stating outright.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016hUrmyUJnJ9ta9aGM6oi7H
Running the chat_completion pull-stream case against a real build of the native module turned up an asymmetry worth a test of its own: `CLRepr::Null` converts to `cx.undefined()`, so the `return None` that ends a Python stream arrives in JavaScript as `undefined` and never as `null`. The first version of this test looped until `null` and hung, which is exactly what a consumer written from the Python side would do — nothing about writing `return None` suggests the other side sees anything else. Pinned directly so the conversion cannot drift, and so the next reader finds the answer instead of a timeout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016hUrmyUJnJ9ta9aGM6oi7H
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
github-actions
Bot
added
rust
|
Claude finished @paveltiunov's task in 2m 26s —— View job Review: 1 medium, 5 low — solid, additive change; main asks are a doc/behaviour mismatch and comment volumeFull review (6 inline comments)Todos
What holds upThe wiring is complete and I traced it end to end: The No security concerns: the hook is customer code in the customer's own config file, credentials stay in deployment env vars, and the change adds no new network path. No performance concerns. Findings
The docs page itself reads well and the chunk-format section is the part a customer will actually need. Two claims I could verify are correct: I could not run the native tests here — reproducing them needs |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Two real defects and three trims. **The JSDoc in `js/index.ts` said the stream terminator was `null`** — the exact mistake the `a Python None crosses as undefined, not null` test exists to prevent, left standing in the one file a consumer types against. Anyone writing `if (chunk === null) break` from it would hang. **The docs pointed at an error users will never see.** A `cube.py` hook returning a model object throws `Unable to represent PyObject in JS` at the bridge, before the return value is ever inspected for a stream, so the "Must return a LangChain chat model, …" entry could not match it. That failure now has its own entry with the message it actually produces. `preserves the hook by reference` only covered the function branch, which Joi cannot rewrite anyway. The branch worth pinning is the object one: `validateOptions` returns Joi's `value`, so a clone would strip a model instance's prototype and with it `bindTools` — reported downstream as "does not support tool calling". Now asserted with a class instance, which an object literal could not have shown. Confirms Joi preserves it. The same explanation had been written out in five places and would have drifted independently. Each now keeps only the fact unique to it and points at the docs page for the rest. `chatCompletion` stays `unknown` rather than `((request: any) => any) | object`: nothing in Core reads it, so no consumer has to narrow it, and an author assigning to it type-checks either way. `object` already admits functions in TypeScript, so the wider type buys little and costs an `any`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016hUrmyUJnJ9ta9aGM6oi7H
|
Claude finished @paveltiunov's task in 1m 7s —— View job Re-review: all 6 prior findings addressed, no new blocking issues — 1 low (comment length in tests)Full re-reviewTodos
What
|
| Prior finding | Status |
|---|---|
1 — docs pointed at an error cube.py never produces |
Fixed. Unable to represent PyObject in JS now has its own bullet naming the message the bridge actually raises, and the LangChain-model bullet is scoped to cube.js. This matches what config-chat-completion-object.py pins |
2 — 7-line comment in optionsValidate.ts |
Trimmed to 4 lines; keeps only "unknown keys are rejected" + why an object is allowed, and links the docs page |
| 3 — reference-preservation only covered the function branch | Fixed properly. preserves a model instance by reference, prototype intact uses a class instance and asserts toBeInstanceOf plus a live bindTools — that's the assertion an object literal could not have made |
4 — types.ts JSDoc length / unknown typing |
Comment trimmed to 4 lines. unknown kept, with a rationale I agree with on reflection: object admits functions in TypeScript, so the union buys almost nothing and costs an any, and no Core code reads the field so nobody is forced to narrow |
| 5 — same paragraph in five places | Each site now carries only its unique fact: the allow-list comment says silent-drop, the Python one says bridge restriction, the Joi one says unknown-key rejection |
6 — js/index.ts JSDoc said the terminator was null |
Fixed, and the replacement is better than my suggestion — it names the cause (None → undefined) and the symptom (if (chunk === null) break never terminates) in the same breath |
Remaining, low
The test prefaces in python.test.ts are still 4–5 lines each (the allow-list one at the top of the suite, and the None/undefined one). Two of the three earn it — silent attribute drop and the undefined terminator are both facts a reader cannot get from the code below them. The chat_completion returning an arbitrary Python object is rejected preface mostly restates its own assertion and could lose a line or two. Not worth another round on its own.
Verification
Re-traced the wiring after the edits: allow-list entry still in its alphabetical slot, chatCompletion: expect.any(Function) still asserted in the full-config shape test, Joi entry unchanged in behaviour (only the comment above it moved). expect(terminator).not.toBeNull() is a real assertion here rather than a tautology — Jest's toBeNull is ===, so it would fail if the bridge ever started emitting null.
As before, I could not run the native suite in this environment — it needs cargo build --features python. The PR reports 14 passed / 1 skipped and 12 passed for optionsValidate; the diff is consistent with that and I took the counts at face value.
No security or performance concerns: still customer code in the customer's own config file, no new network path, and Core reads the option nowhere.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11733 +/- ##
===========================================
+ Coverage 59.08% 84.64% +25.55%
===========================================
Files 237 261 +24
Lines 19022 86101 +67079
Branches 3865 0 -3865
===========================================
+ Hits 11239 72876 +61637
- Misses 7222 13225 +6003
+ Partials 561 0 -561
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
3 participants
Check List
Description of Changes Made
Lets a deployment declare a
chatCompletion/chat_completionconfig hook, which Cube Cloud's LLM Gateway BYOM provider calls to route agent inference through the deployment instead of dialling a model vendor from the control plane. Cube Core accepts the option and ignores it.Four registries define a config option, and each needed an entry for a different reason:
python/cube/src/__init__.pyConfiguration.chat_completion, so@configbinds itsrc/python/cube_config.rscube.pyattribute reaches JS at all — an unlisted name is dropped silently, with no error pointing at the omissioncore/optionsValidate.tsInvalid cube-server-core optionscore/types.tscube.tsconfig that sets it type-checks. Typedunknownon purpose: the value may be a chat model instance, a factory or a stream of chunks, and Core has no dependency on the model library that would let it name any of thoseThe Python and JavaScript contracts differ, and the tests pin why rather than leaving it as folklore. The bridge carries scalars, lists, dicts and plain functions, so a Python hook returns a list of chunks or a
{"next": fn}closure; a model object throwsUnable to represent PyObject in JS, which the third test asserts.One finding worth a reviewer's attention.
CLRepr::Nullconverts tocx.undefined(), so thereturn Nonethat ends a Python stream arrives in JavaScript asundefined, never asnull. The first draft of the pull-stream test looped untilnulland hung — which is exactly what anyone writing the consumer from the Python side would do.a Python None crosses as undefined, not nullpins it.Testing
Ran against a debug build of the native module with
--features python(cargo build --features python), since the Rust allow-list and the embedded Python module ship in one artifact and neither is exercised without it:packages/cubejs-backend-native—jest dist/test/python.test.js: 14 passed, 1 skipped (the darwin-only suite). Includes the three newchat_completioncases and theNoneconversion case.packages/cubejs-server-core—jest dist/test/unit/optionsValidate.test.js: 12 passed,optionsValidate.tsat 100% statement coverage.yarn tscfrom the repo root: clean.Consumers
The runtime and Cloud halves are on the same branch name in
cubedevinc/cube-runtimeandcubedevinc/cubejs-enterprise. Nothing in this PR depends on them — Core accepts the option and ignores it either way.🤖 Generated with Claude Code
https://claude.ai/code/session_016hUrmyUJnJ9ta9aGM6oi7H
Generated by Claude Code