Send Dataflow grouping to ForeverLearning and use their highlight captions

[kswenson]

ForeverLearning's v0.13 release gives grouping a home in their schema and adds a caption to highlight directives. This takes both, so the tutor can see how a student organized their program and can label what it points at in its own words.

Grouping

A Dataflow tile's content now carries an optional groups array, omitted when a program has none, as their catalog asks. Until now grouping reached ForeverLearning only through the Graphviz rendering we send alongside the schema form, where it appeared as labeled clusters — the one thing the drawing carried that the schema form structurally could not.

CLUE's model is flatter than theirs, and the projection says so positively rather than by omission: a group holds nodes and never other groups, and a node carries a single optional groupId, so group_ids is always empty and no node id appears twice. Sent rather than omitted so a reader can tell our flat model from missing data. If CLUE ever nests groups, that field is where they go with nothing else moving.

collapsed does not travel — it is whether the group is folded away in the editor, which says nothing about the program and has no home in their shape. An over-long label is truncated to their cap rather than dropped, because a group the diagnostic cannot name is worse than one named imprecisely. The cap is 60 code points, which is what their maxLength counts, so the label is split by code point rather than by UTF-16 unit: a label of 59 characters plus an emoji already fits and comes through intact instead of being cut through the emoji.

Highlight captions

op_highlight may now carry a 1–60 character caption naming the block in the words the prose just used — on 375 of 376 highlights in ForeverLearning's latest corpus. We use it when present and fall back to orderedDisplayName otherwise, since an absent caption is a legitimate packet rather than an error.

The caption supplies the words and not the license to point somewhere. The target id is still resolved against the packet we sent, and that check runs before the caption is read, so a highlight aimed at a node we never described is dropped however well it is worded. That property mattered before captions existed and matters more now that the button will read convincingly. A blank caption falls back rather than dropping the highlight.

label lives on highlight alone — focus and annotate are additionalProperties: false and reject it. Nothing on our side validates a directive against their schema, so the caption is read only when the op is highlight: a focus directive uses our name even if it arrives carrying a label.

Verified against real documents, not fixtures

Seven context packets were built from real CLUE documents in one class and validated against the v0.13 context schema. Three carry real groups. That exercise found a bug this PR fixes: their catalog requires a group id with minLength: 1, and the first cut emitted an empty string for a group lacking one. Their packet schema leaves tile content open, so that would not have been rejected, but it would have sent a group outside their catalog's shape that nothing could reference. Such a group is now dropped, as are empty member ids.

It also corrected an assumption. Group labels are sometimes the author's own words ("Gripper Control Based on EMG") and sometimes our auto-generated "Group 1" — so a label is always present and only sometimes meaningful, which is what ForeverLearning needs to know before quoting one back.

Not in this PR

Dropping the Graphviz rendering is the other half of CLUE-688 and waits on v0.13 being live for us. It is roughly a third of each packet, so the largest sample goes from 11,873 bytes to about 8,000 once it goes.

Two comments were also corrected. The file header claimed sending both program representations let the DOT form "be evaluated on live turns" — it never was, since no rule on their side reads it. And response.ts said op_highlight has no label field, which v0.13 changed.

The schemas are not vendored, so the conformance suite stays opt-in behind FL_SCHEMA_DIR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MdXthUWvaXQw51Tq43v48p

[cypress]

collaborative-learning    Run #20454

Run Properties:   Passed #20454  •  5d73a9c44c: fix: prefer a captioned highlight and type-check group fields

Project	collaborative-learning
Branch Review	CLUE-688-fl-dataflow-groups
Run status	Passed #20454
Run duration	03m 44s
Commit	5d73a9c44c: fix: prefer a captioned highlight and type-check group fields
Committer	Kirk Swenson
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 ↗︎

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.34%. Comparing base (9a951a5) to head (5d73a9c).
⚠️ Report is 59 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (9a951a5) and HEAD (5d73a9c). Click for more details.

HEAD has 22 uploads less than BASE

Flag	BASE (9a951a5)	HEAD (5d73a9c)
cypress-regression	15	0
cypress	7	0

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #3006       +/-   ##
===========================================
- Coverage   86.82%   72.34%   -14.49%     
===========================================
  Files        1031     1030        -1     
  Lines       59032    59039        +7     
  Branches    15733    15740        +7     
===========================================
- Hits        51253    42709     -8544     
- Misses       7757    16294     +8537     
- Partials       22       36       +14     

Flag	Coverage Δ
cypress	?
cypress-regression	?
cypress-smoke	40.81% <ø> (+<0.01%)	⬆️
jest	60.47% <100.00%> (+0.01%)	⬆️

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.

[Copilot]

Copilot review overview

🟡 Changes recommended

Focus directives can incorrectly use captions, and UTF-16 truncation can corrupt valid group labels.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

• Truncate labels by Unicode code points, not UTF-16 units · New
• Only use directive labels for highlight operations · New

What changed in this PR

Adds ForeverLearning v0.13 support for Dataflow groups and captioned highlights.

Changes:

• Projects Dataflow groups into context packets.
• Uses highlight captions with node-name fallback.
• Adds unit and schema-conformance coverage.

File	Description
shared/​fl-packet/​response.ts	Handles highlight captions.
shared/​fl-packet/​response.test.ts	Tests caption behavior.
shared/​fl-packet/​dataflow-tile.ts	Projects Dataflow groups.
shared/​fl-packet/​dataflow-tile.test.ts	Tests group projection.

---

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

[emcelroy]

Looks good 👍 A review generated with Claude raised the issues below to consider addressing before merge.

Findings (most severe first)

1. Highlight caption can be silently dropped depending on directive order

File: shared/fl-packet/response.ts:186

The dedup key that keeps one highlight per target is ${tileId}/${objectId}. It does not include the directive's op. The caption added in this PR is only readable off a highlight directive — focus directives never carry one.

Failure scenario: A response's components list has a focus directive for node n-logic (no caption possible), followed later by a highlight directive for the same node that carries a real caption from ForeverLearning. The focus entry is processed first, falls back to sentName, and claims the dedup key at line 188. The later highlight directive is skipped by seen.has(key) at line 187, so its caption is discarded — the rendered button shows the generic name instead of the words the prose just used, purely because of array order. response.test.ts has no test with two different ops targeting the same node, so this is unverified. Before this PR, label content didn't depend on which duplicate won; now it does.

Suggested fix: Keep one button per target, but when duplicates exist, prefer an entry with an explicit caption over one without, rather than first-in-array-wins. Adding op to the dedup key is not the fix — it would produce two buttons for one target. Also add a test with a focus directive followed by a highlight directive for the same node (and the reverse order), asserting the captioned one wins either way — the current suite has no case with two different ops on the same target.

---

2. Group node_ids sent to ForeverLearning aren't checked against the sent node list

File: shared/fl-packet/dataflow-tile.ts:111

projectGroups copies Object.keys(raw.nodeIds ?? {}) straight into node_ids with no check that each id actually appears in program.nodes (and therefore in the tile's own nodes array sent alongside it).

Failure scenario: The dataflow model's comment (dataflow-program-model.ts:329-333) documents a past bug where node deletion left groups referencing deleted nodes — but git history shows the fix (758247310) landed in the same PR (#2910) that introduced groups (b80327e65), before either commit ever reached master. So this is not a case of already-corrupted documents sitting in production. That framing was
wrong.

The gap is still real for a different reason: functions-v2/src/chat-tutor.ts:121 shows this content is read from a Firestore document a client wrote directly (msg.rightContent, JSON.parse'd with no MST model reconstruction) — not a document guaranteed to have passed through the model's removeNode action. Nothing requires the JSON reaching projectGroups to have been produced by the app's own editing actions at all. So the dangling-reference risk is a trust-boundary gap, not a legacy-data one: a written document whose groups and nodes disagree (by any means) will have that disagreement sent straight to ForeverLearning as a group referencing a node id absent from that tile's own nodes array — the kind of dangling reference packet.ts and response.ts filter out everywhere else in this module family.

Suggested fix: Filter node_ids against the projected nodes set before sending.

---

3. Group label isn't guarded against non-string values before an unguarded spread

File: shared/fl-packet/dataflow-tile.ts:114

if (raw.label) group.label = [...raw.label].slice(0, kMaxGroupLabel).join("");

This spreads raw.label with only a truthiness check.

Failure scenario: GroupModel.label is types.string in the MST model, so normal UI editing can't produce a non-string label — but that guarantee only holds for data that went through the model. functions-v2/src/chat-tutor.ts:121 confirms the server reads this content by parsing a Firestore document's raw JSON string directly, with no MST model in between. A numeric (or otherwise non-string) label in that JSON reaches [...raw.label] unchanged and throws (numbers/objects aren't iterable). Nothing between projectGroups and buildContextPacket (packet.ts:133) catches it — only renderProgram, two functions below, is wrapped in try/catch, specifically so one malformed program doesn't fail the whole turn's packet build. Blast radius is one conversation's turn, not a wider outage, but the input path is real, not hypothetical.

Suggested fix: Guard with typeof raw.label === "string" before spreading, matching the type-checking already done elsewhere in this projection.

---

4. Group id isn't validated as a non-empty string before being sent

File: shared/fl-packet/dataflow-tile.ts:110

id: raw.id is copied as-is. The drop check if (!raw.id) continue only catches falsy values, not non-string truthy ones (e.g. a number), so such a group is not dropped and its id is sent unchanged.

Failure scenario: Same trust-boundary path as findings 2 and 3: a document whose JSON is read without going through the MST model could carry a group with a non-string, truthy id. It survives the drop check and is sent as id: <that value> — violating ForeverLearning's declared string/minLength: 1 group-id schema that this code otherwise exists specifically to guarantee ("Ids fail closed here, as they do throughout this projection").

Suggested fix: Don't coerce with String(...) — that would manufacture a value that never existed in the source data. Since groups already fail closed on a missing id, extend the same check: require typeof raw.id === "string" && raw.id, and drop the group otherwise.

All four findings are reachable through real input paths, not just synthetic or legacy edge cases: finding 1 through ordinary directive ordering, and findings 2–4 through the fact that this content is read from a client-written Firestore document via direct JSON parsing, with no MST model standing between the wire and the projection code.

[kswenson]

Addressed all four in 5d73a9c:

1. A later directive now replaces an earlier one for the same node when it supplies a caption the earlier one lacked, so the result no longer depends on order. The highlight keeps its first position. Tests cover focus→highlight and highlight→focus, plus two captions (first wins).
2. node_ids is filtered to the nodes the tile sends.
3. label is used only when it's a string.
4. A group whose id isn't a non-empty string is dropped, not coerced.