The multimodal codelab's starter is its own app, not a copy of another codelab's - #497
Open
DenisovAV wants to merge 3 commits into
Open
The multimodal codelab's starter is its own app, not a copy of another codelab's#497DenisovAV wants to merge 3 commits into
DenisovAV wants to merge 3 commits into
Conversation
step_01_starter was a byte-identical copy of Getting Started's finished app, so Step 1 downloaded 0.5 GB of licence-gated Gemma 3 1B — needing --dart-define=HF_TOKEN — that Step 2 then abandoned for SmolVLM2. Worse for a codelab whose Step 1 is titled "A modality is a session flag": Step 2 swapped the model AND set supportImage, so a learner could not tell which of the two made the picture work. Step 1 now opens the same SmolVLM2 weights as a text session. The model file does not change between Step 1 and Step 2, so Step 2 really is one boolean, and the codelab's own thesis is demonstrated rather than asserted. model.dart, main.dart and download_page.dart are byte-identical to step_02_vision's; chat_page.dart is that file minus the vision machinery, keeping maxTokens at 1024 so Step 2's "the call above it changes too — in two ways" stays true. Also drops the last Hugging Face plumbing from the codelab: the comment in step_02/step_03/complete main.dart still claimed Step 1 was gated. There is now no token and no --dart-define anywhere in this codelab. The getting-started -> multimodal MIRRORS row is removed from tool/check_codelabs.sh, and the smolVlm2 block, the ModelType.general paragraph and the main.dart `try` passage move from Step 2's text into Step 1's, where the code now lives. Step durations move with them (4/12 -> 7/9); the 46-minute total the catalogue carries is unchanged.
Step 2's text says model.dart, main.dart and download_page.dart are identical between step_01_starter and step_02_vision, and that the whole step lives in chat_page.dart. Nothing checked it. MIRRORS cannot: it diffs a lib/ directory whole, and here exactly one file has to differ — it is the step. So a comment added to the starter's main.dart would have made the text false with every other check still green. SHARED_STEPS declares both halves and asserts both: the files that must match, and the one that must not. Guarding only the first would let a step that teaches nothing pass — if chat_page.dart ever matched too, Step 2 would have no diff at all, which is as wrong as drift. Proved to fail closed three ways, each reverted and re-run clean: a comment appended to the starter's main.dart (drifted), the starter's chat_page.dart replaced with Step 2's (teaches nothing), and model.dart renamed away (cannot run). The text now says the paragraph is a CI check rather than a promise.
Three reviewers plus Codex went over this branch. Six findings, all confirmed against the tree before fixing. SHARED_STEPS could pass without checking, two ways, both mine. `diff` exits 0 identical, 1 different and >1 when it could not compare at all, and `elif diff -q …; then` reads every non-zero alike — so an unreadable file took the "they differ" branch and the must-differ half passed unchecked. Status 1 is now named in a `case`, and `|| status=$?` keeps `set -e` from aborting before it runs. A half-emptied row was the other: an empty file list ran the loop zero times and reported nothing. Both now fail closed; six perturbations tested, each red, each reverted. The ModelType.general paragraph was wrong in both halves. For a .litertlm file `extensions.dart:74-77` returns `raw` on every platform this codelab targets except iOS, and `raw` never consults modelType — so naming the wrong family does not "wrap the prompt in another model's turn markers"; it does nothing. On iOS, the one manual target, `general` and `gemmaIt` both emit Gemma's own <start_of_turn> markers, so `general` is not "the template inside the file" either. What the field does decide is post-processing: `cleanResponse` strips thinking tags for a fixed list of families that includes gemmaIt and not general. The comment and the prose now say that instead. Three claims the PR itself made checkable, and which failed on sight: "one line changes in _send" (five places do), "the one thing you edit" (contradicted by Step 2's own "in two ways" a page later), and a diff promise that omitted the five plugin registrants `pub add` regenerates.
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
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.
codelabs/multimodal-flutter-gemma/step_01_starter/was a byte-identical copy of Getting Started'scomplete/, enforced by aMIRRORSrow. Two things followed from that, and both are worth fixing.The learner downloaded half a gigabyte and then abandoned it
Getting Started ends on Gemma 3 1B — 0.5 GB, behind a Hugging Face licence gate, so Step 1 needed a token and
--dart-define=HF_TOKEN=hf_.... Step 2 then downloaded SmolVLM2 500M instead and never opened the first model again. Accept a licence, wait out 0.5 GB, throw it away.Worse: the code contradicted the step it was under
Step 1 is titled "A modality is a session flag". But Step 2 swapped the model and set
supportImage, so a learner had no way to tell which of the two made the picture work. The codelab asserted its thesis and then demonstrated something else.The change
step_01_starteris now Step 2 minus the vision flag: the same SmolVLM2 weights, opened as a text session.model.dart,main.dartanddownload_page.dartare byte-identical tostep_02_vision's.chat_page.dartis that file minus the vision machinery —image_picker,supportImageon bothgetActiveModelandcreateChat,Message.withImages, and the token budget, which stays at1024here so Step 2's "the call above it changes too — in two ways" stays true.So Step 2's diff is now the lesson and nothing else. The file on disk does not change between Step 1 and Step 2, which means "a modality is a session flag" is something the learner watches happen rather than something they take on trust.
It also removes the last Hugging Face plumbing from this codelab. The comment in
step_02/step_03/complete'smain.dartstill told the reader Step 1 was gated; it isn't, and there is now no token and no--dart-definein any step.Text
The
smolVlm2block, theModelType.generalparagraph and themain.darttrypassage move from Step 2 into Step 1, where the code now lives. Step 2 gains a short section saying what did not change, and why that is checkable only because Step 1 was already running these weights.Durations move with the material (Step 1 4 → 7, Step 2 12 → 9). The 46-minute total the catalogue carries is unchanged.
Gate
The
getting-started/complete → multimodal/step_01_starterrow leavesMIRRORS. One row remains — Inference Engines genuinely does start from Getting Started's finished app — so the fail-closed#MIRRORS -eq 0guard still has something to guard.Removing it would have traded one unenforced invariant for another, because Step 2's text now tells the learner to diff the two apps and says which files come back identical.
MIRRORScannot express that: it compares alib/directory whole, and here exactly one file has to differ — it is the step. A comment added to the starter'smain.dartwould have made the text quietly false with every other check still green.So the gate gains
SHARED_STEPS, for consecutive steps inside one codelab. Each row names the files that must match and the one that must not, and both halves are asserted: ifchat_page.dartever matched too, Step 2 would have no diff at all, which is as wrong as drift. Proved to fail closed three ways — a comment appended to the starter'smain.dart, the starter'schat_page.dartreplaced with Step 2's, andmodel.dartrenamed away — each naming the offender, each reverted and re-run clean.Review round
Three reviewers plus Codex read this branch. Six findings, every one confirmed against the tree before it was fixed.
Two were fail-opens in the
SHARED_STEPScheck this PR adds — both mine, both the exact class the check exists to prevent.diffexits 0 identical, 1 different, and >1 when it could not compare at all;elif diff -q …; thenreads every non-zero alike, so a file that exists but cannot be read took the "they differ" branch and the must-differ half of the invariant passed without being checked. Status 1 is now named in acase, with|| status=$?soset -edoes not abort before it runs. The other: a half-emptied row ran the shared-file loop zero times and reported nothing. Six perturbations now tested, each red, each reverted.The
ModelType.generalparagraph was wrong in both halves, and this PR had promoted it into Step 1. For a.litertlmfileextensions.dart:74-77returnsrawon every platform this codelab targets except iOS, andrawnever consultsmodelType— so naming the wrong family does not "wrap the prompt in another model's turn markers", it does nothing at all. On iOS, the one manual target,generalandgemmaItboth emit Gemma's own<start_of_turn>markers, sogeneralis not "the template inside the file" either. What the field does decide is post-processing:cleanResponsestrips thinking tags for a fixed list of families that includesgemmaItand notgeneral. The text says that now.The other three were claims this PR itself made checkable by telling the learner to diff the two apps, and which then failed on sight: "one line changes in
_send" (five places do), "the one thing you edit" (contradicted by Step 2's own "in two ways" a page later), and a diff promise that omitted the five plugin registrantsflutter pub addregenerates.