feat: summaries for AI should include markdown + images (CLUE-371)

[emcelroy]

CLUE-371

What changes

Every AI analysis of a student document now sends a text summary and a screenshot in one request, instead of one or the other. Which of the two actually go is decided from the document's own content, not from a setting.

Per unit:

Unit	Before	After
MODS	screenshot only	both
cas	summary only	both
vibe	summary only	both

aiPrompt.summarizer is now ignored. Authored unit configs may still contain it; nothing reads it. It has been removed from the AIPrompt interface but not from the authoring UI, which is a separate follow-up.

How the decision is made

A new classifier walks the document's tiles and answers two questions: would the summary put student work in front of the model, and does anything here need a picture to be understood?

• The summary is sent when it carries student work. That is broader than "a tile holds typed text". A tile also counts when its summarizer describes it in detail (a full or partial handler) and it actually holds something to describe — a drawing with at least one object, a graph with at least one layer. This rule was measured before opening the PR; see "Validation run" below.
• The screenshot is sent when a picture is needed to understand the document, which means one of two things: some tile of the student's own work needs one, or the question does and there is student work for it to be context for. A question's authored prompt can itself be an image, which no summary carries, so an answer sent without a screenshot would be judged without the question it answers. A text-only document whose question needs no picture is not screenshotted at all, which removes a slow third-party call from those analyses.
• An empty document is not evaluated. Previously it was, and the model was paid to comment on a document with nothing in it. A document holding only an authored prompt is empty by this rule, so a picture of a question is never sent without an answer to go with it. - An empty document is evaluated anyway, for now. Not evaluating it is the better answer on its own terms — the summarizer emits only a preamble and headings for a blank document, so the model is paid to comment on nothing — but the client cannot yet say so, and the cost of silence falls on the student. See "Empty documents" below.

Three tile types never count toward the summary, each for a stated reason recorded in the code: Simulator, whose summary is the unit's own description of the simulation rather than anything the student did; Question, a container whose prompt and response rows are classified separately; and Placeholder, an empty slot. Table counts, but only because a Table tile is already treated as student-authored — its cases live in a shared data set, so there is nothing in the tile itself to check.

Empty documents: a decision for the team

This PR originally stopped evaluating empty documents, on the grounds that the summary is boilerplate and the answer is worthless. Manual testing against the emulator showed why that cannot ship on its own. Clicking Ideas? queues an "Ada is thinking about it…" placeholder in the comments panel, and nothing clears that placeholder except an arriving comment — there is no timeout. So a student who asks for ideas before doing any work waits for a comment that never comes, with nothing on screen explaining why.

So for now, empty documents go on being evaluated, exactly as they did before this branch. That is a holding position, not a preference. The recommendation, for discussion: stop sending empty documents to the AI, and have the client show a plain static message instead — something like "This document is empty. Add some work before asking for ideas." That gets both halves right: no wasted evaluation, and the student is told what happened.

That is a design decision rather than an implementation one, so it is not made here. Reviving it needs two changes together: restore the early return in on-analysis-document-pending.ts, and give the client a way to resolve a pending AI comment when no comment is coming. Doing the first without the second reintroduces the hang.

Validation run

The summary-sending rule changed because CLUE-646 turned a Drawing tile's summary into a table of its objects. Whether that table is worth sending was measured before opening this PR, using the evaluation harness: three image-only runs against three mixed runs, identical apart from the message shape, both arms on the prompt production now sends, with caching off so the repeats were real calls. 138 API calls, $0.34, no refusals and no errors.

The table earns its place, and nothing regressed. Corpus-wide, unknown fell from 47 of 69 evaluations to 39. Three documents changed answer, all in the same direction, with every repeat agreeing:

Document	image-only	mixed
drawing	unknown ×3, no key indicators	form ×3, 2–3 indicators each
graph	unknown, unknown, function	function ×3
adversarial-text	unknown ×3	function ×3

The other 20 documents answered identically in both arms. On drawing the image-only arm did not answer less confidently — it returned no usable feedback at all, three times out of three, saying the image held insufficient information to identify a focus area. That is the case the rule change exists for.

Two limits on what this shows:

1. The measurable population is two documents. Only drawing and graph are documents the rule actually changes — no typed text, but a summary that now carries student work. The 50-document teacher-workshop example corpus we had to work with contains none: 42 mixed, 7 text-only, and 1 visual-only that does not qualify. So the evidence comes from hand-built fixtures, and the change is close to unobservable on the real documents we hold today. That cuts both ways — low risk in production now, and unmeasurable there until a corpus contains drawing-only student work.
2. The comparison is broader than the rule. A mixed run sends text for every document that carries student work, which was 8 of the 23 sent, so adversarial-text moving is the mixed shape helping in general rather than anything the new rule did. Only drawing and graph isolate the rule itself.

The run is written up in the harness README's recorded-runs section, and its experiment file is committed as scripts/ai-harness/experiments/g5-drawing-mixed-vs-image.json. If review changes what gets sent, this is worth re-running before the deploy — the response cache makes an unchanged re-run nearly free.

A failed screenshot no longer costs the evaluation

Previously, if the screenshot service failed, the whole analysis failed and the student got no feedback. Now the failure is recorded and the analysis continues with the summary alone; the reverse holds too. Only having nothing at all to send is a failure.

The call to Shutterbug is also considerably stricter: a named 45-second timeout, a status-code check, the response body parsed inside a try, and the returned URL required to be a non-empty https: address. Text from outside is cut to 500 characters before it reaches a record.

The whole handler runs inside an error boundary. These triggers have no retry configured, so an exception escaping the handler used to leave the queue entry stranded with nothing recorded anywhere. Every failure now files a record, and that record keeps everything worked out before the failure — the classification, the render target, a summary that was produced but had nothing to go with it.

Two smaller guards sit behind that promise. A summary over 200,000 bytes is recorded as a summaryError rather than stored, because the queue record it travels in is capped at 1 MiB and a summary anywhere near that is past what gpt-4o-mini accepts anyway — the screenshot can still carry the work, so an outsized summary costs a representation rather than the whole document. And error() itself no longer throws: if the failure record is refused, it retries with the message and document id alone, carrying nothing over, and the pending entry is deleted either way. The record explaining a failure must not fail for the reason the work did.

What the queue records say

Omission and failure are separate fields, so the done queue can be counted without parsing text:

• summaryOmittedReason / imageOmittedReason — a fixed code, for something left out on purpose: no-student-work-in-summary, no-visual-content, images-disabled.
• summaryError / imageError — free text, for something that was meant to be there and could not be produced.

Exactly one of a pair is set when the matching send… is false, and neither is set when it is true. A test asserts this on every case in the suite rather than case by case.

Records also carry analysisVersion: 2, the classification (modality, hasStudentText, summaryCarriesStudentWork, needsImage, promptNeedsImage), the render target, and — from the consumer — messageShape (mixed, summary-only or image-only). The classification carries both halves of each decision so a record explains itself: hasStudentText and summaryCarriesStudentWork can disagree, and so can needsImage and promptNeedsImage, and which one drove the outcome should not need the classifier re-run over the document to work out. The summary is stored whenever it was produced, sent or not, so anyone investigating a piece of feedback can see what the model was not given.

One request builder

Production builds its OpenAI messages only through shared/ai-analysis-messages.ts. An image-only request is built with the mixed builder and a null summary, which produces exactly what the image-only builder would — so "image only because the text was omitted" stays on one code path. A test in functions-v2/test/ai-analysis-messages-integration.test.ts runs the real function with a fake OpenAI client and asserts the messages deep-equal the shared builders' output, including that the image-only case equals both builders. A message built inline anywhere in functions-v2 would fail it.

Related summaries are enrichment only: they are looked up just when a summary is being sent, and a failed lookup logs a warning and proceeds with none. The whole lookup is inside the try, because getEmbeddings returns undefined on error and FieldValue.vector(undefined) then throws from inside the lookup.

Shared code

Three modules that existed twice — once in production, once in the evaluation harness — now exist once, under shared/:

• shared/ai-analysis-classify.ts — the tile capability registry and document classifier.
• shared/render-page.ts — the page posted to Shutterbug. Production, the harness's render modes and scripts/shutterbug.ts all build it here, so the three cannot drift. This also closes the script-injection hole the harness reported: the document is escaped into the <script> element.
• shared/ai-analysis-messages.ts — the request builders, already shared.

The built-in prompt

defaultAiPrompt.mainPrompt opened "This is a picture of a student document", which is wrong whenever no picture is sent. It now opens "Below is a text summary of a student document and a picture of it. Either one may be absent." A missing article in the next sentence was fixed at the same time.

The harness keeps both copies as prompt files, because a prompt's hash is part of every request key and rewriting one in place would make past results claim to answer a question they were never asked. categorize-design-default is the prompt from when a request carried one representation; categorize-design-default-mixed is what production sends now.

Authored prompts were not touched. mods, cas and vibe prompts live in curriculum content, not this repository. They still describe a single representation, and the curriculum authors need telling.

A runtime switch for screenshots

Writing { imagesEnabled: false } to analysis/settings in Firestore stops screenshots immediately, with no deploy. Summaries keep flowing. A missing document, a missing field, or a setting that cannot be read all mean enabled — the switch can only turn things off, so a Firestore blip cannot cost a document its analysis. It is read only when a document actually needs a screenshot. Firestore rules already deny clients everything under analysis; no rules change was needed. Documented under "Runtime settings" in functions-v2/README.md and in docs/firestore-schema.md.

Deployment

1. Confirm the live release is v7.5.0 or later. Screenshots render against the released build's authoring-iframe/index.html. PR fix: page height from unwrapped document renderers (CLUE-371) #2975's height fix shipped in v7.5.0; an older release reports a height of 0 and clips screenshots at 500px.
2. Deploy the imaged function first, then the rest: firebase deploy --only functions:functions-v2:onAnalysisDocumentImaged. This shrinks the old-consumer window to nothing.

onAnalysisDocumentPending now declares timeoutSeconds: 120, above the 60-second default. The default was unsafe: the Shutterbug request alone could use all 60 seconds, so a hung request meant the platform killed the invocation before the internal timeout fired — nothing recorded, and the pending entry left behind with no retry to pick it up. The internal timeout is 45 seconds, which keeps the abort ahead of the platform with room to write the failure down.

The deploy window is covered in both directions and needs no drain. The new consumer recognizes records written by the old producer (keyed on a missing analysisVersion) and reads them the old way; the new producer still writes the old summarizer field, so an old consumer can read its records. Both are marked for removal in a later cleanup, once done shows no version-less records.

There is no AI_ANALYSIS_CLUE_IFRAME_URL parameter — making the CLUE URL configurable per environment was considered and skipped, so staging renders against the same released build as production.

Rollback and monitoring

Screenshots can be turned off at runtime with the switch above; summaries continue. A full rollback is redeploying the previous functions, which read the new records through the summarizer compatibility field, so nothing needs draining.

For the first week, watch:

• failedImaging and failedAnalyzing.
• The share of done records with imageError set. More than a few percent means Shutterbug is struggling and the switch should go off while that is looked at.
• The "No response from AI" rate. Prompts are larger than they were — see below — and spike finding 7d tied those failures to the largest prompts.

Re-evaluation triggering (spike finding 7e) is the real traffic multiplier and is unchanged here.

Note on CLUE-646

CLUE-646, already on master, changed what the summarizer emits: every tile carries a "This tile's id is …" line, and a Drawing tile is now a table of its objects rather than the sentence "This tile contains a drawing."

Two consequences for anyone reading this work's earlier results. Finding 8's measurements and the blinded review were done against the old summary, which the harness now calls text variant version 1 — future comparisons treat the new default as a new variant and should not be read against those numbers. And prompts are larger: an id line per tile, and a big drawing becomes a big table.

This change is also why the summary-sending rule is what it is. The old rule withheld the summary unless a tile held typed text, which was right when a drawing summarized as one sentence and wrong once it became a table of the student's own objects.

Testing

• functions-v2: 164 passing. The producer's suite went from 9 cases to 35 — what gets produced for mixed, text-only, drawing-only, empty-drawing, empty, image-prompt-question and mock documents; five Shutterbug failure modes; the switch on, off and unreadable; a summary too large to store; and the error boundary for malformed JSON, a throwing classifier, a throwing summarizer and a refused failure record.
• shared/: the classifier's tests and a snapshot of the render page now live beside the modules they cover; 454 passing under the root runner.
• Harness: 924 passing, and it reads the same classifier fields production does, so a harness run measures the request production would actually send.
• Root: 4027 passing; check:types and lint clean.

The pipeline was also driven end to end by hand, through the CLUE UI against the Firebase emulators, with real Shutterbug and OpenAI calls: a text-only document (summary-only, no screenshot), a drawing-only document (mixed — hasStudentText: false with summaryCarriesStudentWork: true, and the model's answer quoting the object table), the runtime switch off (images-disabled, prompt tokens dropping from 37,413 to 628 on the same document), an empty document, and the mock evaluator. The screenshot Shutterbug returned was checked and shows the student's actual document, so the render target works against the released build.

The live-Shutterbug test is skipped unless LIVE_SHUTTERBUG=1 is set; it was run against the real service to confirm it still passes.

Found in review and testing

The branch is five commits: first the change itself, then three fixes from automated review. Two of the three were regressions this branch introduced, so they are worth a reviewer's attention rather than being folded away. The fifth fixes an issue discovered during manual testing: empty documents left the student waiting. The queue behaved exactly as designed and the UI consequence was invisible from the record. See "Empty documents" above.

• A question whose prompt is a picture was sent summary-only. The classifier zeroes requiresVisualRepresentation for prompt-role tiles, which is right for "is this student work?" and wrong for "does this request need a picture?" — and an Image prompt's section of the summary is empty, because a prompt is summarized minimally and an Image tile emits nothing under minimal. The model was asked to judge an answer without the question it answers. This branch caused it: image-mode units previously screenshotted every analysis, so the prompt was always visible. Fixed by the second bullet in "How the decision is made".
• An oversized summary could strand a document. If the write handing a document on were refused for size, the boundary would call error() with the same summary, that write would be refused too, and the throw would escape with pending never deleted. Master's error() wrote only the pending record, which carries no summary, so this branch introduced it. Fixed by the two guards in "A failed screenshot no longer costs the evaluation".
• The harness README described the rule this branch replaced, in three places, and quoted call counts from it.

Known gaps and follow-ups

• An empty Dataflow canvas or an empty Table still counts as carrying student work, so it sends a summary describing little more than the tile's presence. This is pre-existing, not new here: both are marked containsStudentText at the type level, and unlike Text and Drawing that mark is never narrowed per instance, so the per-type check added by this PR never gets a say. Narrowing it would also move such documents between modalities, reclassifying any recorded harness result containing one — so it belongs with the item-12 thin-summary work rather than here. Recorded in comments beside both the check and its test.
• An Image used as a question prompt still summarizes to nothing. This PR gives the model the picture, which is the important half. The text half remains wrong: handleQuestionTile forces a minimal summary for a prompt and handleImageTile returns an empty string under minimal, so the "Question Prompt" section of a done record is a heading, a tile id and nothing else. Fixing it changes the summarizer's output for every document containing a Question — another variant version bump and another round of cache invalidation — so it belongs with the next summarizer change.
• onAnalysisDocumentImaged has no error boundary, and its done write is the larger of the two: the whole imaged record plus the model's full response. If that write is refused the throw escapes and the imaged entry strands, the way the pending entry could have before this PR. Pre-existing on master and left alone here, but the asymmetry is now worth closing — the producer has both a boundary and an unfailable recorder, and the consumer has neither.
• cypress/e2e/functional/document_tests/ai_evaluation_test_spec.js was not run. The installed Cypress binary is broken in this environment. It would not have tested this branch in any case: it visits with firebaseEnv=staging, so the evaluation is handled by the deployed staging functions. It is a post-deploy check, not a pre-merge one.
• One new lint warning. shared/ai-analysis-messages.ts:35 is 122 characters, over the 120 limit, because the prompt's second line gained a word. It is prompt text inside a template literal — the line breaks are part of what is sent to the model, so it cannot be wrapped, and the same file already carries a 130-character line on master for the same reason. lint and lint:build:shared both pass.
• Eight pre-existing shared/ lint warnings are untouched, in ai-summarizer/, seismic/, shared-utils.ts, slate-to-markdown.ts and two other lines of ai-analysis-messages.ts.

Out of scope

The authoring UI and its docs; the thin-summary threshold, measured capture height and other follow-ups; any prompt change beyond the opening sentence.

[Copilot]

Pull request overview

Updates AI document analysis to select and send summaries and screenshots based on document content, while improving failure handling and aligning the evaluation harness with production.

Changes:

• Adds shared document classification, rendering, and mixed-message infrastructure.
• Supports mixed, summary-only, and image-only analysis with runtime screenshot control.
• Expands queue metadata, resilience, documentation, and test coverage.

Reviewed changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
shared/render-page.ts	Centralizes secure render-page generation.
shared/render-page.test.ts	Tests shared rendering behavior.
shared/__snapshots__/render-page.test.ts.snap	Updates render-page snapshot.
shared/ai-analysis-messages.ts	Revises the default mixed-media prompt.
shared/ai-analysis-classify.ts	Adds shared content classification.
shared/ai-analysis-classify.test.ts	Tests summary-content classification.
scripts/survey-class-documents.ts	Uses the shared classifier.
scripts/shutterbug.ts	Uses the shared render generator.
scripts/ai-harness/test/tile-types.test.ts	Updates classifier imports.
scripts/ai-harness/test/smoke.test.ts	Updates summary eligibility assertions.
scripts/ai-harness/test/smoke-image.test.ts	Updates mixed/image smoke coverage.
scripts/ai-harness/test/shutterbug.test.ts	Updates render parity references.
scripts/ai-harness/test/prompt.test.ts	Validates the new production prompt copy.
scripts/ai-harness/test/image-messages.test.ts	Preserves historical prompt-based keys.
scripts/ai-harness/test/helpers.ts	Adds prompt and classification helpers.
scripts/ai-harness/test/fixtures.test.ts	Uses shared classification.
scripts/ai-harness/test/extras.test.ts	Tests drawing-only summary eligibility.
scripts/ai-harness/test/backends.test.ts	Uses shared rendering.
scripts/ai-harness/src/schemas.ts	Shares the modality type.
scripts/ai-harness/src/review.ts	Imports shared classification types.
scripts/ai-harness/src/report.ts	Imports the shared modality type.
scripts/ai-harness/src/execute.ts	Aligns harness send rules with production.
scripts/ai-harness/src/corpus.ts	Uses shared document classification.
scripts/ai-harness/src/backends/shutterbug.ts	Uses shared render HTML.
scripts/ai-harness/src/backends/puppeteer.ts	Uses shared render utilities.
scripts/ai-harness/README.md	Documents classifier and validation changes.
scripts/ai-harness/prompts/categorize-design-default.json	Marks the historical prompt provenance.
scripts/ai-harness/prompts/categorize-design-default-mixed.json	Adds the new production prompt copy.
scripts/ai-harness/experiments/g5-drawing-mixed-vs-image.json	Records the mixed-versus-image experiment.
scripts/ai-harness/debug-render.ts	Uses shared rendering.
functions-v2/test/on-analysis-document-pending.test.ts	Expands producer and failure-path coverage.
functions-v2/test/on-analysis-document-imaged.test.ts	Tests representation selection and consumption.
functions-v2/test/ai-analysis-messages-integration.test.ts	Verifies shared message-builder integration.
functions-v2/src/on-analyzable-doc-written.ts	Removes the obsolete summarizer setting.
functions-v2/src/on-analysis-document-pending.ts	Classifies, summarizes, and screenshots documents.
functions-v2/src/on-analysis-document-imaged.ts	Sends available representations together.
functions-v2/src/analysis-queue-types.ts	Defines versioned queue records.
functions-v2/README.md	Documents the screenshot switch.
functions-v2/lib/src/ai-categorize-document.ts	Adds unified representation categorization.
docs/unit-configuration.md	Documents content-driven representation selection.
docs/firestore-schema.md	Documents analysis runtime settings.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[emcelroy]

Addressed in a6928e4. We checked each case against the classifier:

Document	Counts as carrying student work?
Dataflow, empty program	yes
Dataflow, no program at all	yes
Table, no data set	yes
Graph, no layers	no
Drawing, no objects	no

So both claims in that paragraph were wrong. A Dataflow counts without a wired program, and "an empty tile of such a type does not count" is false for Dataflow and Table.

The cause is the same for both. Table and Dataflow are marked as holding student-authored text at the type level, and unlike Text and Drawing that mark is never narrowed for the individual tile. So the first half of the rule answers yes before the "does it actually hold anything" check is reached. Graph and Drawing behave exactly as the paragraph described.

The exception now has its own paragraph in "Which documents a run declines to send", naming both types, why they behave that way, that it is pre-existing rather than something this rule introduced, and that narrowing it would move documents between modalities and so reclassify recorded results.

We also corrected the capability registry section, which said classification "adds instance-level checks" and listed Text and Drawing in a way that implied the checks were general.

Narrowing the two types is logged as a follow-up alongside the thin-summary work, where the modality question can be settled deliberately.

[codecov]

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.28%. Comparing base (940076f) to head (3a5478e).

Files with missing lines	Patch %	Lines
shared/ai-analysis-classify.ts	80.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2988      +/-   ##
==========================================
+ Coverage   86.19%   86.28%   +0.09%     
==========================================
  Files         996      999       +3     
  Lines       56858    56960     +102     
  Branches    15060    15105      +45     
==========================================
+ Hits        49007    49147     +140     
+ Misses       7832     7793      -39     
- Partials       19       20       +1     

Flag	Coverage Δ
cypress-regression	70.70% <ø> (+0.20%)	⬆️
cypress-smoke	41.16% <ø> (-0.02%)	⬇️
jest	57.83% <80.95%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cypress]

collaborative-learning    Run #20216

Run Properties:   Passed #20216  •  3a5478e61c: Merge branch 'master' into CLUE-371-ai-feedback-text-and-images

Project	collaborative-learning
Branch Review	CLUE-371-ai-feedback-text-and-images
Run status	Passed #20216
Run duration	03m 45s
Commit	3a5478e61c: Merge branch 'master' into CLUE-371-ai-feedback-text-and-images
Committer	Ethan McElroy
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	4
View all changes introduced in this branch ↗︎

[scytacki]

Looks good to me. I just left one optional inline comment.

[scytacki]

Since this same logic is done above to compute messageShape. It would be more clear if this was an inline function with a switch statement using that messageShape.

[emcelroy]

@scytacki Thanks 👍 I've addressed that in 152db56.