feat(gemma-4-trimodal): tri-modal training scaffolding for Gemma 4 12B Unified - #63
marksverdhei wants to merge 5 commits into
Conversation
Pre-merge constraint audit (read-only, spare-cycles work)Used idle time to audit the 4 pre-merge constraints I logged in the PR body. Scope is smaller than feared: only 1 of 4 actually needs refactor work, 2 already handle the both-True case. Follow-up PR planning can scope down accordingly. 1. Worker mutually-exclusive logic — ✗ hard mutex (refactor required)
vision = is_vision_model(...) if not self.is_audio else False # short-circuits
self.is_vlm = not self.is_audio_vlm and vision and is_dataset_imageWhen Refactor scope: allow 2. Collator label masking — ✓ already handles both
3. Dataset standardization — ✓ already preserves all three
4. Frontend upload exclusivity —
|
22cd7f2 to
547b816
Compare
CI red — investigated + partial fixPR is currently red across Backend CI (Py 3.10–3.13), Core CI (all 3 HF/TRL matrix cells), Frontend build, Wheel, Tauri, and the UI test workflows. Investigated during idle cycles. Two distinct root causes, neither caused by this PR's commits: Class A —
|
| Test | Pattern expected | What rebase ate |
|---|---|---|
test_cancel_id_wiring.py (4 tests) |
cancelId const + cancel_id: cancelId in payload + onAbortCancel handler |
HT cancel-id wiring in chat-adapter.ts |
test_composer_rtl_bidi_attribute.py (5 tests) |
dir="auto" in composers + compositionend watchdog (issue unslothai#5546) + onKeyDown IME gate |
HT RTL + IME composer hardening |
test_studio_text_descender_clipping.py |
sidebar account-block leading-tight |
HT sidebar tweak |
These are HT-only UI hardening features that the upstream rebase silently regressed. The right fix is to restore them in the source files, not delete the tests. That's substantive frontend work that overlaps with PR #65 (chat-runtime-store rebase fix) — probably belongs as an extension of #65 or a sibling PR, not in this trimodal scaffolding PR.
Meta-finding
ht direct-push CI only triggers the lightweight Studio Tests workflow. The full matrix (Backend CI, Core CI, Repo tests, etc.) only runs on PRs. The Class B failures have been silently broken since 2026-06-01 — visible only now that I opened this PR. Worth tightening the on-push policy so rebase debt doesn't compound.
Proposed disposition for this PR
- Class A is fixed; no further action.
- Class B is out of scope for trimodal. Suggest landing it as an extension of PR fix(frontend): sync missing store properties and fix build errors from upstream rebase #65 (which is already restoring rebase-regressed frontend code) OR a dedicated
chore: restore HT-only UI hardening after 2026-06-01 rebasePR. This PR should not be gated by Class B — it's pre-existing breakage we just happen to be the first to surface.
Open to alternative dispositions. cc reviewer agent on %33.
…signature The /api/models/config route signature was split from a single `hf_token: Optional[str] = Query(None)` parameter into two — a Query parameter aliased "hf_token" and a Header parameter aliased "X-HF-Token" — as part of the X-HF-Token header / token-leak fix on ht. The unit test that direct-invokes get_model_config was not updated, so since that signature change the test has failed on all 4 Backend CI Python versions with: TypeError: get_model_config() got an unexpected keyword argument 'hf_token' The failure has been silent on ht direct pushes (Backend CI only runs on pull_request events), and was first surfaced by PR #63's CI run. Fix: pass `hf_token_query=None, hf_token_header=None` to match the new signature. Function body resolves them via `hf_token = hf_token_header or hf_token_query`, so the resulting behavior is unchanged. Verified locally by overlaying ht's routes/models.py (which has the new signature) onto the trimodal branch and running pytest → PASS in 2.12s.
|
Second Class A fix shipped — commit `e1f921776`. `test_get_model_config_resolves_cached_case_before_model_checks` was calling `get_model_config(hf_token=None, ...)` but `ht` split that param into `hf_token_query` + `hf_token_header` for the X-HF-Token leak fix. Same silently-broken-on-ht pattern as the lile_router stub — Backend CI only runs on pull_request events, so the regression sat unnoticed on `ht` since the signature change. Updated to pass both new params as `None`. Verified locally by overlaying ht's routes/models.py onto the trimodal worktree and running pytest → PASS. Expected CI delta after this lands: Backend CI Py 3.10/3.11/3.12/3.13 should drop from FAIL to PASS (assuming no other test in that batch is broken). Class B (pinned-content) failures remain out of scope. |
|
Heads up — two unblocking notes:
Suggested recipe: git fetch origin
git checkout feat/gemma4-trimodal
git filter-branch --force --msg-filter \
'sed -E "/^[Cc]o-[Aa]uthored-[Bb]y: ?[Cc]laude/d"' \
origin/ht..HEAD
git rebase origin/ht
git push --force-with-lease origin feat/gemma4-trimodalPosted by ht-unsloth maintainer doing a §10 trailer-hygiene sweep. |
…get_model_config signature) (#74) * fix(test): add lile_router to test_desktop_auth router stub test_health_response_reports_desktop_capability_fields constructs a SimpleNamespace as a stand-in for studio/backend/routes, listing 12 routers. The stub predates PR #8 (lile addition to studio); after that PR landed, studio/backend/main.py started importing lile_router from routes, but this test stub was never updated. Result: ImportError on all 4 Python versions in Backend CI ("cannot import name 'lile_router' from <unknown module name>"). Verified locally: pytest studio/backend/tests/test_desktop_auth.py:: test_health_response_reports_desktop_capability_fields → PASS in 0.30s. * fix(test): update get_model_config call to new hf_token_query/header signature The /api/models/config route signature was split from a single `hf_token: Optional[str] = Query(None)` parameter into two — a Query parameter aliased "hf_token" and a Header parameter aliased "X-HF-Token" — as part of the X-HF-Token header / token-leak fix on ht. The unit test that direct-invokes get_model_config was not updated, so since that signature change the test has failed on all 4 Backend CI Python versions with: TypeError: get_model_config() got an unexpected keyword argument 'hf_token' The failure has been silent on ht direct pushes (Backend CI only runs on pull_request events), and was first surfaced by PR #63's CI run. Fix: pass `hf_token_query=None, hf_token_header=None` to match the new signature. Function body resolves them via `hf_token = hf_token_header or hf_token_query`, so the resulting behavior is unchanged. Verified locally by overlaying ht's routes/models.py (which has the new signature) onto the trimodal branch and running pytest → PASS in 2.12s. --------- Co-authored-by: Mark's synthetic twin <249650165+marksverdhai@users.noreply.github.com>
e1f9217 to
9c1cbfd
Compare
|
Ran the filter-branch to remove the Co-Authored-By trailers, rebased on ht, and force-pushed as requested. The commit history is now clean. |
Opens the tri-modal (text + image + audio) training track for Gemma 4
12B Unified. The model arch supports it natively (single encoder-free
projection over all modalities; Gemma4UnifiedProcessor exposes
image_processor + feature_extractor + video_processor + tokenizer);
Studio's training plumbing was shaped for Gemma 3N and didn't route
gemma4_unified through the audio path. This commit ships only the
scaffolding so the follow-up collator + dataset work can be reviewed
separately.
Changes:
- docs/gemma4-trimodal-training.md — full gap analysis: where audio
detection fires, why the existing audio_vlm collator (Gemma 3N-shaped)
doesn't fit, what the tri-modal dataset row schema should look like,
follow-up PR breakdown.
- studio/backend/utils/models/model_config.py — new _AUDIO_CONFIG_PATTERNS
dict that matches against the full tokenizer_config.json (not just
added_tokens_decoder). First entry: processor_class ==
"Gemma4UnifiedProcessor" -> audio_vlm. _check_token_patterns runs
the added-vocab patterns first (existing behavior) then config
patterns as a fallback. Verified locally:
detect_audio_type("unsloth/gemma-4-12B-it") -> "audio_vlm"
negative control (processor_class="Qwen2TokenizerFast") -> False
- tests/test_gemma4_12b_smoke.py — new --mode trimodal that does NOT
load the 12B model. Loads Gemma4UnifiedProcessor + apply_chat_template
with a 1-sample image+audio+text message, asserts:
* processor class is Gemma4UnifiedProcessor
* all four sub-processors are present
* input_ids contains audio_token_id AND image_token_id
* pixel_values + input_features payloads both populated
CPU-only, ~1s. Verified PASS: 256 image tokens + 25 audio tokens +
payloads + mm_token_type_ids all present in the batch.
Out-of-scope (separate PRs):
- Audio collator branch for gemma4_unified in trainer.py:3098
- Tri-modal dataset format detector/converter
- Studio frontend recipe-studio support for mixed-modality rows
- End-to-end LoRA SFT smoke on a tri-modal dataset
Address review feedback from reviewer agent:
1. _AUDIO_CONFIG_PATTERNS now duck-types on capability instead of being
coupled to a concrete processor_class string. processor_class names
get renamed during the first weeks of upstream HF integration; the
structural tokenizer_config keys (audio_token, boa_token) are
stickier.
audio_vlm: lambda c: "audio_token" in c or "boa_token" in c
Negative-control verified locally against Gemma-3 (image-only),
LLaVA, Qwen2, Whisper, and Gemma 4 12B-it (positive). The
added-vocab patterns still run first so Whisper/CSM/BiCodec/DAC/SNAC/
Gemma 3N never reach the fallback.
2. Trimodal smoke assertions tightened to close two silent-drop holes:
(a) Text modality survival — assert the literal probe substring
("Describe" + "see and hear") appears in the decoded batch, not
just audio/image tokens.
(b) Payload validity — every pixel/audio key now has .numel() > 0
asserted, with the actual shape + numel printed. Catches the
"key exists, tensor is an empty shell" failure mode.
Smoke re-run PASS:
audio_token count: 25, image_token count: 256, text-probe hits: 2
pixel_values: shape=(1, 280, 6912), numel=1935360
input_features: shape=(1, 25, 640), numel=16000
(25 audio tokens * 640 features = 1s of 16kHz audio — correct)
Reviewer's pre-merge constraints recorded in the PR body (worker
mutually-exclusive logic, collator label masking, dataset
standardization, frontend upload exclusivity) — addressed in
follow-up PRs, not this scaffolding commit.
…lper
Adds 18 unit tests (CPU-only, 0.18s) that lock in the contract introduced
by this PR so future _AUDIO_TOKEN_PATTERNS / _AUDIO_CONFIG_PATTERNS edits
can't silently regress:
- _AUDIO_CONFIG_PATTERNS positive: audio_token-only, boa_token-only,
both keys present.
- _AUDIO_CONFIG_PATTERNS negative controls: LLaVA, Qwen2, Gemma-3
(vision-only), Whisper (audio-only via the added-vocab path),
empty dict.
- _AUDIO_TOKEN_PATTERNS regression guards: csm, whisper, audio_vlm
(Gemma 3N's <audio_soft_token>), dac.
- Two-pass ordering: added-vocab wins over config-fallback when both
could match (synthetic Whisper added-vocab + audio_token at root
resolves to "whisper"); config fallback runs when added-vocab is
missing or empty; both-empty returns None.
- Taxonomy: VALID_AUDIO_TYPES contains "audio_vlm";
is_audio_input_type("audio_vlm") is True.
Production refactor (no behavior change): _check_token_patterns
promoted from a nested closure inside _detect_audio_from_tokenizer
to a module-level function. Necessary so the test imports the real
production code instead of a mirror that could silently drift. Two
existing call sites at lines 926 + 951 unchanged.
9c1cbfd to
a0a8d4e
Compare
Summary
Opens the tri-modal (text + image + audio) training track for
google/gemma-4-12B-it(gemma4_unifiedarch, the only Any-to-Any member of the Gemma 4 family). Architecture supports it natively; Studio's plumbing was shaped for Gemma 3N. This PR ships scaffolding only — audio detection extension + processor-side smoke + scope doc — so the follow-up collator + dataset work can be reviewed separately.Builds on
ht-2026-06-07Gemma 4 12B wiring which added 12B to the mapper + Studio registries and proved text/vision SFT works on RTX 3090.What's in this PR
docs/gemma4-trimodal-training.md— full gap analysis: where current audio detection fires, why the existingaudio_vlmcollator (Gemma 3N-shaped) doesn't fit Gemma 4 Unified's encoder-free architecture, what the tri-modal dataset row schema should look like, and the breakdown into follow-up PRs.studio/backend/utils/models/model_config.py— new_AUDIO_CONFIG_PATTERNSdict that matches against the fulltokenizer_config.json(not justadded_tokens_decoder). Duck-typed on capability:"audio_token" in tok_config or "boa_token" in tok_config→audio_vlm. The existing 6 added-vocab patterns run first; this is the structural fallback for Any-to-Any models that declare modalities as top-level keys.tests/test_gemma4_12b_smoke.py— new--mode trimodalthat does not load the 12B model. LoadsGemma4UnifiedProcessor+apply_chat_templatewith a 1-sample image + audio + text message; asserts text + audio + image all survive into the batch and that every payload tensor hasnumel() > 0. CPU-only, ~1s.Verification
Review applied (round 1, reviewer agent on %33)
processor_class == "Gemma4UnifiedProcessor"→ duck-typed"audio_token" in c or "boa_token" in c. Class names get renamed during early HF integration; structural keys are stickier..numel() > 0on every payload tensor, closing the "key exists, tensor is empty shell" silent-drop hole.Pre-merge downstream constraints (must verify before non-draft)
These are review-surfaced concerns that this scaffolding PR doesn't address but should NOT regress further down the stack:
studio/backend/core/training/worker.py(andtrainer.pymodel-routing): does the codepath currently doif is_vision: ... elif is_audio: ...? Gemma 4 Unified is genuinely both — needs a path that hydrates image_processor + feature_extractor in the same run.DataCollatormust setlabels = -100for both audio and image tokens in the same sequence. Today's Gemma 3N collator masks one modality at a time.standardize_chat_formatmust handle a singlecontentarray containing{"type":"image"}+{"type":"audio"}+{"type":"text"}without dropping or reordering.Out of scope (follow-up PRs)
gemma4_unifiedintrainer.py:3098(current path is Gemma 3N-shaped).studio/backend/utils/datasets/.is_vision_model/detect_audio_typesimultaneous-True case — Studio currently treats the two as mutually exclusive routing keys; Gemma 4 Unified is genuinely both.Test plan
--mode trimodal) PASS — processor contract verified, text + image + audio all survive into batch with populated payloads.detect_audio_typesubprocess returnsaudio_vlmfor Gemma 4 12B-it from a fresh process (out-of-band; needs Studio venv).🤖 Generated with Claude Code