Skip to content

engine api: an optional image capability beside speech (#1573) - #2248

Open
devopsfortunato wants to merge 1 commit into
ashhart:mainfrom
devopsfortunato:task/Tensorfold-Contribut+t-mw2m47ye-s9riow
Open

devopsfortunato wants to merge 1 commit into
ashhart:mainfrom
devopsfortunato:task/Tensorfold-Contribut+t-mw2m47ye-s9riow

Conversation

@devopsfortunato

@devopsfortunato devopsfortunato commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

What this changes

Before: the engine API can hand back tokens, audio and decisions, but a model that is not a token family has no way to hand back a picture. After: an engine may state an optional image capability beside SpeechInfo — Info.image (max_side, max_pixels, max_steps, max_prompt_tokens) plus a defaulted, additive VTable.submit_image reached through Engine.submitImage. The engine encodes one PNG a request and appends it to a caller-owned byte list using the allocator the caller puts in the request, so the caller owns and frees it after finished; an engine that does not set the entry is unchanged, and Engine.submitImage answers error.Unsupported.

ImageRequest carries the prompt and negative token ids, width, height, steps, guidance, seed, the caller's ImageResult, the caller's png list and its alloc. Nothing in the tree sets Info.image yet, so no served behaviour changes. The follow-up pull request carries the route and the MOSS-Image host; no route, no serve recognition, no /v1/models change and no host are here.

This round: the lean limit

After the rebase onto 34d42fcc8, zig/src/core/engine_api.zig stood at 607 lines, and the lean rule caps a source file at 600, so CI's lean check step failed on this branch. This round only fixes that: three existing declarations that were spread over four lines each (Stop, Admit, Explain) are now one-liners, the style this file already uses for Memory and PromptCacheCapacity. No name, type, default or docstring changed, no code moved, and the file is 598 lines. The image API surface is untouched.

Checks on this head

  • Head commit: d116edf3a (rebased on 34d42fcc8).
  • Lean checker, python3 tools/zig/lean_check.py, on the base (34d42fcc8) and on this head: 325 problems each, and tools/zig/lean_new.py reports 0 new findings.
  • Full host suite on one aarch64 host (20 cores, Zig 0.17.0 from .zig-version, an empty Zig cache): zig build test --summary all — exit 0, Build Summary: 569/569 steps succeeded; 7359/7420 tests passed (61 skipped), 4m15s wall.
  • zig fmt --check on the changed Zig files — exit 0; git diff --check — exit 0.
  • New tests: six fake-engine tests in zig/src/core/image_api_test.zig — the entry is reached with exactly the caller's fields; the appended PNG is freed with the caller's allocator under std.testing.allocator (a leak or mismatch fails); a vtable without the entry answers error.Unsupported; error.Busy/error.InvalidRequest come back unchanged; Info.image is reported exactly and Info{} has none; the finished event reaches the caller's sink with the reason the engine set.
  • They fail before the change: on the base neither Info.image, VTable.submit_image nor Engine.submitImage exists, so the file cannot compile against main. That they run in CI is shown by a planted control: changing one expected width to 999 makes zig build test exit 1 naming core.image_api_test.test.the image entry is reached with exactly the caller's fields (and the same test under the engine-seam step); restored, exit 0.
  • Where they are wired: zig/src/core/engine_api_test.zig, the engine seam's own test list (run by build.zig's api module on Linux and Windows and by zig/build/metal_targets.zig on macOS), not core/root.zig. The core root test graph carries no lanes module, so a test importing engine_api.zig there fails with no module named 'lanes' … referenced by engine_api.Engine.VTable; wiring it beside engine_api needs no build change and runs on both branches.
  • CI on head d116edf3a: green — all seven checks (zig, zig-windows, zig-macos-dev-tools, fmt, release-paths, benchmark-clients on ubuntu-latest and windows-latest).
  • Not run, and why: no device or checkpoint receipt — nothing in the tree sets Info.image, so no request reaches a device and no picture exists to compare. macOS, Metal, HIP and XPU were not run; this change has no backend code.

Receipt

n/a — this change cannot reach inference: it adds an API surface and its tests, and no engine implements it yet, so no token, logit or picture is produced. The follow-up host's pull request carries the receipt for a served picture.

Checklist

  • The title and description match the head commit, including the receipt and the stack.
  • New tests fail before the change, pass after it, and skip cleanly without their dependency.
  • Comments and docstrings are one line. No measurements or history in the source.
  • No personal data, machine names, internal hosts or local paths in the diff or its commits. No AI attribution lines.

@devopsfortunato
devopsfortunato force-pushed the task/Tensorfold-Contribut+t-mw2m47ye-s9riow branch from 250a1e8 to 8b703f8 Compare November 5, 2026 11:38
@ashhart

ashhart commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Thanks @devopsfortunato. This is the engine half of #1573 in the shape we agreed: optional, defaulted, inert until a host sets it, and the caller-owned PNG with its allocator tested for leaks. Two things: main has moved under engine_api.zig, so it needs a rebase. And #1573's plan had @BobClawblaw writing both halves after the generate tool lands; could you check with them on #1573 that you're not both writing it? With that, it can land after the cut.

@devopsfortunato

Copy link
Copy Markdown
Contributor Author

Rebased on current main (5f110edf9), head e5ca1ce91. engine_api.zig re-applied without a conflict; the only conflict was CHANGELOG.md's top anchor, resolved by keeping main's CI bullet and this one.

Re-ran the suite on the new base: zig build test 545/545 steps, exit 0; test-golden 5/5; test-kernel-contract plus test-server-security test-pull-security 36/36 steps (192 tests); zig fmt --check build.zig zig clean; lean checker 425 on the base and 425 on this head, no new findings. CI is running on the new head.

On the second point: I have asked @BobClawblaw on #1573 whether they are taking the route and the MOSS-Image host, and said I will stop at the engine half if so — I have the route half written and host-tested but no MOSS-Image checkpoint or host here, so it stayed out of this pull request.

@ashhart

ashhart commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Thanks @devopsfortunato, for the clean rebase and for asking on #1573. Once @BobClawblaw has answered there, this can land after the cut.

@ashhart

ashhart commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Thanks @devopsfortunato. One small thing while #1573 settles: the rebase comment names e5ca1ce91, but the branch and the body still show d00584834. Could you check the push went through and update the body's head?

@devopsfortunato
devopsfortunato force-pushed the task/Tensorfold-Contribut+t-mw2m47ye-s9riow branch 2 times, most recently from 88a873e to 127762e Compare November 5, 2026 22:58
@ashhart

ashhart commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Thanks @devopsfortunato for the rebase. The code is the same as what we read, every existing engine builds and
answers as before with both new fields defaulted to null, and the split on #1573 is settled with @BobClawblaw handing
over both halves. Two small things, neither blocking: the body still names d00584834 and base cde4fe7d9, so please
update it to 127762e97; and the CHANGELOG line should cite this PR, "(#2248, part of #1573)", which we can also
fix at landing. It's ready to queue, and #2253 then restacks onto main.

@ashhart
ashhart force-pushed the task/Tensorfold-Contribut+t-mw2m47ye-s9riow branch from 127762e to 47e109d Compare November 6, 2026 08:33
@devopsfortunato
devopsfortunato force-pushed the task/Tensorfold-Contribut+t-mw2m47ye-s9riow branch from 47e109d to d116edf Compare November 6, 2026 09:25
@devopsfortunato

devopsfortunato commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for the two notes, @ashhart - both are done, and the body now names the current head.

The rebase broke the lean check, and that is now fixed. On 474c3b048 the file was under the limit; after the rebase onto 34d42fcc8 main had grown and zig/src/core/engine_api.zig stood at 607 lines, one over the 600-line rule, so CI's lean check step failed on the head. This round changes only that: three existing declarations that were spread over four lines each (Stop, Admit, Explain) are now one-liners - the style the same file already uses for Memory and PromptCacheCapacity. No name, type, default or docstring changed, no code moved, and the file is 598 lines. The image API surface (api.ImageRequest, api.ImageResult, api.ImageInfo, api.ImageSubmitError, api.Info.image, api.Engine.VTable.submit_image, api.Engine.submitImage) is untouched, so the capability is exactly as you read it.

The body now names head d116edf3a and base 34d42fcc8, and the overlap note is re-run on this head: complete: false (pullFilesUnavailable, timeLimit), 5 changed files; #2157, #2008 and #2282 each conflict in one file, CHANGELOG.md, where both sides add a bullet under the same ## Unreleased anchor; #2157 also edits zig/src/core/engine_api.zig, where the three-way merge is clean. The CHANGELOG citation you offered to fix at landing is left to you, so the bullet does not have to be rebased again for it.

Checks on this head

  • Lean checker, python3 tools/zig/lean_check.py, base 34d42fcc8 and head: 325 problems each, lean_new.py reports 0 new findings.
  • Full host suite on one aarch64 host (20 cores, Zig 0.17.0, an empty Zig cache): zig build test --summary all - exit 0, Build Summary: 569/569 steps succeeded; 7359/7420 tests passed (61 skipped), 4m15s wall. (Correcting the numbers I posted first: that run had a warm local cache, so it counted fewer test cases; this is the same recipe as the run on the other branch - empty global and local cache - so the two are comparable.)
  • On the 4-core Pi-class host this session also runs on, the same suite has one pre-existing environmental failure - roce_inflight_test (madvise(MADV_NOHUGEPAGE) -> EINVAL on this kernel) - with zig/src/fabric/ byte-identical to the base; it fails the same way on the base and passes on the host above, so CI's ubuntu runner does not see it.
  • zig fmt --check on the changed Zig files - exit 0; git diff --check - exit 0.
  • CI on head d116edf3a: green - all seven checks (zig, zig-windows, zig-macos-dev-tools, fmt, release-paths, benchmark-clients on ubuntu-latest and windows-latest).

Review: the round's diff went through GLM-5.3 - verdict approve, no correctness findings, and the check that the three compacted declarations kept their field names, order, types, defaults and docstrings.

Not run, and why: no device or checkpoint receipt - nothing in the tree sets Info.image, so no request reaches a device; macOS, Metal, HIP and XPU were not run, as this change has no backend code.

@ashhart

ashhart commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Thanks @devopsfortunato, and for catching the 607 lines. The three one-liners change no name, type or default, and the image API is as we read it. We'll set the CHANGELOG line's citation at landing. It goes in the landing queue once our own cells on this head are in: main against it, with tokens equal and speed level. Then #2253 can restack onto main.

@ashhart

ashhart commented Nov 6, 2026

Copy link
Copy Markdown
Owner

/atc exchange 6a4409a46106f033561894b44060d39bcd695fc0abc0e75e7bae6b98762dabf1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants