CLUE-610: rename the generic document type from "group" to "axes" by scytacki · Pull Request #2952 · concord-consortium/collaborative-learning

scytacki · 2026-08-08T13:25:54Z

Renames the generic document type from the stored value "group" to "axes", across the app, the Firestore rules, and the one-time sweep script.

Stacked on #2951 (CLUE-610-deferred-open-and-create-rules) — review that first. The diff shown against this base is only this work.

Why

type: "group" no longer means "this is a group document". Regular group documents and class-wide collaborative documents both store it, and they are told apart only by guards over their stored axis fields. What the value actually marks is a document whose behavior is read from its axes rather than from its type — but it is still named for the first such document. That costs us twice:

It blocks the next kind. Any new axis-native kind must put something in type. A fresh DocumentType enum value reintroduces the per-type branching this refactor exists to remove; storing "group" says something false about a document that is not a group's.
It hides what each reader means. Some of the sites comparing against GroupDocument mean "a group's document", some mean "a concurrent document", some mean "an axis-native document". The shared literal makes them look identical.

Why now, and the deadline

type is a stored value, so changing it needs an admin sweep, and a sweep needs a full client drain in front of it. CLUE-604 already schedules exactly that sweep — adding type is one more field in a write that is already happening to these documents.

This means the app change has to ship in 7.5.0. Landing it in 7.6.0 instead would make the sweep wait for 7.6.0 to drain, but 7.6.0's own content is the cleanup the sweep unblocks — so 7.6.0 would have to be split, or the sweep run twice. v7.4.0 is still the newest tag as of 2026-08-08, so the window is open; if 7.5.0 has been cut by the time this is picked up, this needs re-planning rather than merging.

The shape of it: a transitional accept-both window

Nothing rewrites stored data here. Until CLUE-604's sweep runs and old clients drain, both values exist, so readers accept both and GroupDocument stays exported.

Readers widen before writers flip, so no commit in the middle can write a value the app cannot read. The commits are task-sized and land in that order:

Commit	What
`1768c35c2`	`AxesDocument = "axes"` and the `isAxesType` predicate
`2fc531292`	nine readers accept both values
`58dbd1a6d`	writers create documents with `"axes"`; RTDB declarations widened
`9ed6aad23`, `cb8017666`	both Firestore rules clauses accept both values
`6d03bba57`, `0065dbfdf`	sweep script: one merged write per document

isAxesType is deliberately a type guard, not a boolean — db.ts:727 narrows type to a literal to build a discriminated-union member, and a plain boolean return breaks that. It also means the post-sweep cleanup is one edit to one function rather than one per call site.

Two things worth a careful look

The sweep script now makes one merged write per document rather than committing two disjoint write lists. This matters more than it looks: type is the script's own work-queue key — the driving query is where("type", "==", "group"), so once a document's type flips it stops matching and can never be returned by a re-run. Committing type separately from the axis fields would make write order load-bearing across the 400-document chunk boundary, and a document whose type landed while its axis-field write failed would be permanently half-migrated with no recovery path.

The rules widening is monotone — an added || term at each of the two clauses, no operand removed — so every document that reached a decision before reaches the identical decision now, and exactly one additional type literal is granted. isValidDocumentCreateRequest needed no change: it constrains only that type is present, never its value, because the create-time rules key on owner instead.

Review follow-ups

Added after @kswenson's review. Each is its own commit.

Three checks would have gone from a closed test to an open one. Widening them to the shared axis-native value binds them to a set this rename exists to let grow, so a future axis-native kind would inherit the behaviour with neither file appearing in its diff. Two are rebased; the third is marked.

Student read access (isDocumentAccessibleToUser) now reads concurrent, which is the field that answers the permissions question and what the Firestore rules key their own grant on (isConcurrentClassDocument) — they cannot enforce group membership either. The owner is deliberately not used: a group- or class-owned document that is not readable by every member is a shape we may want later.

It keeps the pre-sweep type as a transitional disjunct, and that disjunct is load-bearing. Group documents created before concurrent was stamped store neither it nor the new type, and this check runs on un-opened Firestore metadata — the thumbnail calls it to decide whether to suppress its own click, so the on-open backfill has not run and cannot. It tests the pre-sweep literal rather than isAxesType, so a future kind cannot inherit read access through it. The function also now states that it asks nothing about the class because it is only ever asked about the user's own class, which was previously unwritten.
The isGroup getter is deleted, not widened. It had one consumer — the group-switch reaction — which now asks hasGroupOwner(primary) directly. As a type test it excluded class-wide documents only by accident: they satisfy it, and the reaction returns early only because their groupId is undefined. With it gone, every remaining is<Type> getter really is a test on type.
isSortableType is marked, not rebased. It is a membership list keyed on a type string, so rebasing it means changing its signature to take a document — CLUE-611's pass. Its comment now says a later axis-native kind is listed in Sort Work by default, and directs a kind that should not be listed to check the axis fields rather than add another type test. Listing is a far weaker default than read access.

Also: the "axes" delete block gains the outside-class case its "group" sibling had (that block is what CLUE-604's cleanup deletes, so the coverage would otherwise vanish silently); handleOpenGroupDocument gains the .catch both its siblings already had; and the design doc gains a rollback paragraph, a dry-run re-check before the 7.6.0 reader removals, and the two testing exceptions noted above.

Deploy sequencing

This fits the release plan already agreed for the CLUE-550 line of work — 7.5.0 CLUE-610, 7.6.0 CLUE-604, 7.7.0 CLUE-612:

Rules deploy, ahead of the 7.5.0 release — npm run deploy:firestore:rules. Rules first, as usual, and here required rather than merely conventional. Step 7 goes the other way round, for the reason below.
7.5.0 — this PR. The app accepts both values and writes "axes" on creation. Deadline-bound, see above. Deploy outside class hours — see the caveat below.
Drain. The same drain CLUE-604's sweep already requires; no additional wait.
Run the sweep — not a release. CLUE-604's step 2: dry-run then APPLY=1, against each environment in turn. An operational step gated on step 3 having actually happened, deliberately separate from any version going out. Shipping the script is not running it — this PR ships it; the run is CLUE-604's.
7.6.0 — CLUE-604's code removals, including this PR's app-side cleanup. CLUE-604 already removes two of the accept-both readers; the rest belong in the same pass — every other accept-both reader, and "group" in isAxesType, isSortableType, and DocumentTypeEnumValues. They share CLUE-604's gate exactly (safe once the sweep has run everywhere, unsafe before), so there is no reason to leave them as unscheduled cleanup. Follows step 4 rather than accompanying it.
Drain again.
7.7.0 — CLUE-612, plus this PR's rules-side cleanup. CLUE-612 is a rules-only change, so here the rules deploy is the release — there is no app code to sequence ahead of. It can carry this PR's last leftover: dropping the "group" branch from the canonical-race delete. (concurrentChangeOk is deleted wholesale by CLUE-612, so its own branch needs no separate handling.)

Steps 1 and 4 are the two that are not a release going out.

Two things survive that cleanup rather than being dropped with it. The GroupDocument constant stays — it is still the group kind's registered name, and the kind axis is not renamed. And "group" stays in the RTDB type declarations, because RTDB is never swept and keeps a permanent mix of both values.

Why the rules move first here and last in 7.7.0

Both are rules changes and they sit on opposite sides of their releases. The rule is direction:

A widening rules change goes first. Step 1 accepts one more type value and takes nothing away, so deploying it against the currently-live app changes no decision. It must precede the app that starts writing the new value, or that app's writes hit rules that don't know the value yet.
A narrowing rules change goes last, after a drain. Step 7 stops accepting a write that older bundles still make. Rules apply to every client at once, so it can't ship until the app that made that write is not merely deployed over but gone from people's browsers.

For step 7 the write in question is the on-open concurrent backfill. Two independent things retire it: CLUE-604's step 3 deletes the code in 7.6.0, and the sweep (step 4) removes the unmigrated documents that would trigger it in the first place. Neither is enough alone, because a browser still running the 7.5.0 bundle contains the backfill — hence step 6's drain, and hence 7.6.0 and 7.7.0 being separate releases at all.

And why step 1's rules must precede the app rather than merely accompany it. The widening is monotone — an added || term at each clause, no operand removed — so deploying it against the currently-live app changes no decision, which is what makes rules-first safe. The reverse order is not: once the app writes "axes", two class members can race to create the same canonical document, and the loser's cleanup delete is governed by a rules clause that under un-widened rules matches only "group". That delete runs through deleteOrphanDocument, which is best-effort and swallows its own error — so the failure is silent, and the symptom is Firestore metadata quietly accumulating for documents nobody can reach.

Deploy 7.5.0 outside class hours. From the instant the writers ship, a student still on the old bundle who opens a brand-new shared document — one a classmate on the new bundle just created — fails to open it, and no drain is possible because the window opens when the release does. Only a group's or class's first document created in that window is exposed; everything already stored is untouched. Accepted rather than designed around, since deferring the writers forces the second sweep this work exists to prevent.

Deliberately not here

Almost no reader is rebased onto a real axis. The design records what each site actually asks, so a later pass (CLUE-611) can rebase each onto the axis that answers it. That is ten judgment calls and this release has a deadline. Three sites are exceptions, because in those the widening would have left a default behind rather than just a misleading name — see Review follow-ups below.
The RTDB type is not swept. Existing records keep "group" forever — only createdAt is read back from that tree, and the tree is slated for removal. The declarations widen so both values are representable; the data is not touched.
Nothing is deleted. The on-open concurrent backfill and getDocumentTitle are widened here and removed by CLUE-604, so the two stories do not both claim those sites.
The sweep is not run.

Testing

Full unit suite (3715) and the emulator-backed rules suite (435) both pass; check:types and lint:build clean. The rules tests cover both values at both clauses.

Each widened reader has a case for both values — the transitional window is the only time both occur, and exactly when a regression would ship — with two stated exceptions: document-title.tsx and document-workspace.tsx have no test file at all, so their one-line predicate swaps go in uncovered. (A third, db.ts's CREATE_GROUP_DOCUMENT log gate, is covered with the new value only, which is moot because after this PR no caller passes the old one.)

A literal-string sweep across src, shared, functions-v2, scripts, and firestore.rules found no reader left comparing against a bare "group"; the remaining hits are type declarations that legitimately admit both values, the owner axis (DocumentOwnerType), the kind registry, and unrelated uses of the word — a CSS class, drawing-object types, ARIA roles, a Sort Work section key, a sticky-note audience.

The sweep script's tests pin the query predicate (type == "group", not the value it writes) and the 400-document chunk boundary, including that the final partial batch is actually committed.

Jira

CLUE-610 and CLUE-604 have been updated: CLUE-610 gained the rename plus its release requirements, and CLUE-604's step 2 was restated around the merged write. CLUE-612 was re-read and needs nothing — it is written entirely in terms of the shared marker, never the type value.

🤖 Generated with Claude Code

…roup" [CLUE-610]

]

…ace rules [CLUE-610]

…LUE-610] Both tests reused whatever `db` the previous test left behind instead of opening their own studentAuth client, so the caller identity under test was an accident of declaration order and either test would throw undefined `db` if run in isolation.

…LUE-610]

…l ones [CLUE-610] The merged-write comments claimed a half-swept document (type:"axes" without kind:"group") matches neither getDocumentTitle branch and renders with no title. That branch checks isAxesType(type) && groupId, and isAxesType accepts both "axes" and "group", so a half-swept group document still matches and still renders correctly — the claimed hazard doesn't exist on this branch. Replace it with the two hazards that are real: a group document missing concurrent+kind loses its history manager and canonical-pointer slot label; a class-wide document missing curriculum scope is invisible to Sort Work's unit-scoped query. Also brings the function docstring in line with the three-part (not two-pass) behavior the header already describes.

…LUE-610] Ten no-behavior-change fixes from the final whole-branch review: - Replace the sweep script's merged-write rationale in all four places (design spec, script header, script body comment, test comment) with the one argument that survives verification: type is the script's own work-queue key (where("type", "==", "group")), so committing it separately from the axis fields would make write order load-bearing and could permanently strand a half-migrated document with no way to find it again. Two earlier rationales in the same spots were checked against the code and found false. - Name and accept the deploy-window collision in the design spec (§7) and the plan's deploy-sequencing section: old clients meeting a brand-new "axes"-typed document at the moment 7.5.0's writers ship, before any drain is possible. Mitigation: deploy outside class hours. - Strengthen the sweep script's test mock so it can catch a silent under-migration: pin the query predicate (collectionGroup + where args) instead of discarding them, and give db.batch() a fresh recorder per call so a 401-document case can assert the 400/1 commit split. - Reword the db.ts kind-lookup comment to state the actual constraint (no "axes" registry entry, so the group kind's `concurrent: true` is what's being borrowed) instead of a narrower and partly inaccurate one. - Delete a redundant db.test.ts case whose assertions are a strict subset of the amended create-path test just above it. - Note above both widened firestore.rules clauses that "group" and "axes" are both live until CLUE-604's sweep has run everywhere, so neither branch is dead code. - Correct "all ten call sites" to "every call site" in the isAxesType comment (an eleventh site was added after that comment was written). - Align the design spec with shipped code: DBGroupDocMetadata.type is "group" | "axes", not narrowed to "axes"; the isAxesType snippet now shows its type-guard return annotation. - Remove the design spec's link to an untracked session-handoff note so the committed doc doesn't 404 once that note is gone. - Note in the sweep script header that write volume now scales with every matched document, not just the ones needing a concurrent/scope backfill. Covering tests, npm run check:types, and npm run lint:build are clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ree comments [CLUE-610] The 401-document test counted batch allocations, so deleting the tail commit still passed it. The mock now records whether each batch was committed, and the test asserts on that — verified by deleting the tail commit and watching it fail. Also folds the plan-only deploy steps into the design spec, since the plan is transient: the rules deploy is now its own sequencing step with its command.

codecov · 2026-08-08T13:28:54Z

Codecov Report

❌ Patch coverage is 79.31034% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.98%. Comparing base (274886b) to head (9ba57e2).

Files with missing lines	Patch %	Lines
src/components/document/document-workspace.tsx	33.33%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2952      +/-   ##
==========================================
+ Coverage   85.67%   85.98%   +0.30%     
==========================================
  Files         987      988       +1     
  Lines       56591    56605      +14     
  Branches    14946    14950       +4     
==========================================
+ Hits        48487    48669     +182     
+ Misses       8084     7916     -168     
  Partials       20       20

Flag	Coverage Δ
cypress-regression	`70.88% <73.91%> (+0.78%)`	⬆️
cypress-smoke	`41.28% <69.56%> (-0.03%)`	⬇️
jest	`57.51% <65.51%> (+<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.

cypress · 2026-08-08T13:31:39Z

collaborative-learning Run #20066

Run Properties: Passed #20066 • 9ba57e2bfa: Merge branch 'master' into CLUE-610-generic-type-rename

Project	`collaborative-learning`
Branch Review	`CLUE-610-generic-type-rename`
Run status	`Passed #20066`
Run duration	`03m 44s`
Commit	`9ba57e2bfa: Merge branch 'master' into CLUE-610-generic-type-rename`
Committer	`Scott Cytacki`
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 ↗︎

…and schedule the cleanup [CLUE-610]

…generic-type-rename

…generic-type-rename The axis-profile backfill and the type rename both restructured the sweep script into one merged write per document, for different reasons. Unioned the passes: `type` for every matched document, then `axisProfile`, `concurrent`, and curriculum scope. `BackfillResult` carries both new counts. The create-shape fixtures the deferred-rules branch added spell `type: "axes"`, since they assert what the client sends. Group-document creation now takes its offering from the caller's token, and a teacher token carries no `offering_id`. The axes-typed canonical-race delete test had been setting up its document through a rules-bound teacher client, so it now writes that document with adminWriteDoc, as its siblings do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Renames the generic axis-driven document type from "group" to "axes" while supporting both values during migration.

Changes:

Adds AxesDocument and updates readers/writers for transitional compatibility.
Extends Firestore rules and tests to accept both values.
Updates the backfill script to atomically migrate type and axis fields.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/models/document/document.ts`	Recognizes both axis-type values.
`src/models/document/document.test.ts`	Tests axes-typed models.
`src/models/document/document-utils.ts`	Widens document accessibility handling.
`src/models/document/document-utils.test.ts`	Tests axes titles and access.
`src/models/document/document-types.ts`	Defines the new type and predicate.
`src/models/document/document-types.test.ts`	Tests type recognition and sorting.
`src/models/document/document-kinds.ts`	Supports axes-typed group titles.
`src/models/document/document-kinds.test.ts`	Tests transitional title behavior.
`src/lib/db.ts`	Writes and reads the new type.
`src/lib/db.test.ts`	Tests creation and backfill behavior.
`src/lib/db-types.ts`	Widens RTDB type declarations.
`src/components/tiles/tile-activity-badges.tsx`	Supports badges for axes documents.
`src/components/tiles/tile-activity-badges.test.tsx`	Tests axes activity badges.
`src/components/document/document-workspace.tsx`	Opens axes-typed primary documents.
`src/components/document/document-title.tsx`	Handles axes document titles.
`scripts/backfill-group-document-axes.ts`	Migrates types with merged writes.
`scripts/backfill-group-document-axes.test.ts`	Tests migration batching and queries.
`firestore.rules`	Widens transitional authorization clauses.
`firebase-test/src/documents-rules.test.ts`	Tests axes document permissions.
`firebase-test/src/canonical-pointers-rules.test.ts`	Tests axes document deletion rules.
`docs/superpowers/specs/2026-08-07-clue-610-generic-type-rename-design.md`	Documents design and rollout sequencing.

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

…e-rename ones [CLUE-610] The kind/concurrent gate accepts both type values, but the axis-profile gate beside it still tested the pre-rename one. Both creators now send "axes", so no newly created group or class-wide document was getting a profile — and the sweep could not repair them, since its query matches only the pre-rename value. The profile tests all passed the pre-rename type explicitly, so none of them exercised the value the client actually sends. The class-wide test now sends what the client sends, and the axes-typed stamping test asserts the profile too, giving the accept-both gate a case for each value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…LUE-610] The current-state docs explained the kind/profile stamp gate by quoting the single type value it used to test, which says the wrong thing now that the gate accepts both and new documents store "axes". The metadata field entry also called `type` written-once and immutable. The sweep rewrites it, so that entry now names the script and says why it gets past the rule that keeps the field read-only for clients. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

scytacki · 2026-08-21T02:42:47Z

The failing cypress test is one that was failing before.

… type [CLUE-610] The badges report who is co-editing a tile. That is what the `concurrent` axis says, and every neighbouring feature asking the same question already reads it: the concurrent history manager, the non-owner write-sync suppression, the concurrent title bar, the thumbnail. The badges were the last holdout testing the document type instead, and widening that test to accept both type values would have left the one site that already meant "concurrent" phrased as a type test. No behavior changes: every concurrent document is axes-typed today, and the on-open backfill derives `concurrent` from the kind registry for any axes-typed document, so documents predating the stored field carry it on the model by the time a tile can render. The two tests that stood for the type now stand for the axis, and each fails against a type test: a non-concurrent document typed like the ones that do get badges renders none, and a concurrent document of another type renders them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

kswenson

👍 Looks good -- the following review, developed in conjunction with Claude Code 🤖, identified a few issues worthy of consideration. Reviewed against #2951 as its base, which I have also approved.

PR Review Summary

Changes: 24 files, +925 / −150 lines. Stacked on #2951.

What it does

Renames the generic document type from the stored value "group" to "axes", across the app,
firestore.rules, and the one-time sweep script. The value no longer means "this is a group
document" — regular group documents and class-wide collaborative documents both store it, and they
are told apart by guards over their axis fields. What it actually marks is a document whose
behavior is read from its axes rather than its type, and it was still named for the first such
document. That blocks the next axis-native kind and hides what each reader means.

Nothing rewrites stored data here. This is a transitional accept-both window: readers accept
both values, writers now write only "axes", and CLUE-604's sweep flips stored data later. The
commits land readers-widen-before-writers-flip, so no commit in the middle can write a value the
app cannot read.

The approach

The ordering discipline is the good part. Readers widen (2fc531292) before writers flip
(58dbd1a6d) before the rules widen (9ed6aad23, cb8017666), and the PR is explicit that the
rules must deploy ahead of the app even though they were committed after it. The reasoning for
that — a widening rules change goes first because it changes no existing decision, a narrowing one
goes last because it must wait for a drain — is correct and worth keeping in the design doc.

isAxesType as a type guard rather than a boolean is the right call, and I verified the
stated reason: it narrows to exactly "axes" | "group", which is precisely what the runtime check
accepts, and db.ts needs that narrowing to build a discriminated-union member. A boolean return
genuinely would not compile there.

The sweep's one merged write per document is well-reasoned and I want to record why, because it
is subtle: type is the script's own work-queue key (where("type","==","group")), so committing
type separately from the axis fields would make write order load-bearing across the chunk
boundary, and a document whose type landed while its axis write failed would be permanently
half-migrated. The merged write makes every abort safe — each document is either fully migrated or
fully untouched, and a re-run selects exactly the remainder.

Assessment

This is careful, well-sequenced work and I'm approving it. The issues below are offered as input
to your plan rather than as gates — you have far more context on the CLUE-610/604/612 sequence than
I do, and I'd rather name the things that look like they could bite than guess at which of them are
already accounted for. Issue 1 is the one I'd genuinely weigh; the rest are minor.

The central risk on a rename like this is a missed reader, and a dedicated exhaustive sweep found
none — every site that decides on type either uses isAxesType or was deliberately rebased
onto an axis field, across src/, shared/, functions-v1/, functions-v2/, scripts/,
cypress/, MST enums, and the RTDB/Firestore type declarations. Two independent checks of the rules
confirm the widening is genuinely monotone (an added disjunct at each of two clauses, parenthesised
so precedence is unchanged), that no third type-testing clause was missed, and that both clauses
have both-values tests in both directions.

Two commits deserve credit rather than suspicion:

056985387 fixes a real bug this branch introduced and caught itself — profileName was still
gated on type === GroupDocument after the writers flipped, so no newly created document would
have had an axisProfile stamped at all, and the sweep (which queries for "group") could
never have repaired them. Good catch.
05ffb3cdd rebases the tile activity badges onto concurrent instead of the type. That is a
behavior change rather than a rename, and it is correct: every axes-typed document is concurrent
by profile, concurrent is populated synchronously on both the open and create paths, and the new
tests fail against the old type-based gate.

Verification I ran locally

Check	Result
`npm run check:types`	clean
jest — document models, db, tile badges, scripts	19 suites, 347 passed
firebase-test rules suite (emulator)	10 suites, 434 passed, 2 skipped
CI — Build, Jest, smoke, 14/15 regression shards	pass
CI — regression shard 09 (`tiles_copy_test_spec.js`)	fail — known-flaky spec

Two notes on that table. The PR body says 430 rules tests; it is now 434 — the body is slightly
stale, not a discrepancy. And shard 09's failure is in tiles_copy_test_spec.js, a spec already
known to be flaky; I confirmed independently that this PR touches no copy or canvas code, and that
the one tile-adjacent change (tile-activity-badges.tsx) is an early-return gate governing whether
a badge renders inside a tile, with no path to changing how many tile-rows a copy produces.

Issues

1. Three checks were widened from a closed test to an open one, and the open set is designed to
grow — one of them gates student read access. — medium-high
src/models/document/document-utils.ts:131, src/models/document/document-types.ts:67,
src/models/document/document.ts:124

Raising this for your judgment rather than as a gate — you know the CLUE-610/604/612 sequence far
better than I do, and it may be that the right home for this is CLUE-611's per-site rebase rather
than here. The observation is that the concern isn't the accept-both widening itself; it's that
three checks moved from a closed test to an open one.

// document-utils.ts:131
- const isGroupDoc = metadata.type === GroupDocument;   // matches exactly one literal
+ const isGroupDoc = isAxesType(metadata.type);         // matches any axis-native document

and that value gates student read access:

if (user.isStudent) {
  return ownDocument || isShared || isPublished || isGroupDoc || ...
}

Today the two forms cover the same documents, so nothing is broken now. The problem is what happens
next. After CLUE-604's sweep, every axis-native document stores type: "axes" — that is the
point of the rename — and this PR's own rationale is that the old name "blocks the next kind." So
the moment someone adds the next axis-native kind, isAxesType returns true for it, and every
student in the class silently gains read access to it. Nothing in that PR's diff would touch this
file or show up in its review. The check has been bound to a set that is designed to grow, and the
growth is the feature this PR exists to enable.

Your own design doc already classifies it this way. The §5b table's "Means" column reads, for
this row: "read access — a permissions question" — and the table's preamble says the column is
recorded "so a later pass can rebase each onto the right axis; it is not acted on here." So the
misalignment is identified and deliberately deferred, which is a defensible call; I'm flagging it
because this is the one row where what's deferred is an access decision rather than a naming tidy.

It is also unmarked. This PR is careful about transitional sites elsewhere — document-kinds.ts:275,
document-types.ts:39, db.ts:121 and :1137 all carry explicit TRANSITIONAL comments. Here the
comment was updated ("Group and class-wide documents are accessible to everyone") without a marker,
so nothing tells the next reader that the predicate is standing in for something narrower than what
it literally tests.

The owner axis already answers the real question, and both guards are already used on metadata
objects of exactly this shape in Sort Work (document-group.ts:213-279):

const isGroupDoc = hasGroupOwner(metadata) || hasClassOwner(metadata);

That is what the comment already claims, it stays correct when a fourth kind arrives with a
different owner, and — a bonus — it doesn't reference the transitional type at all, so it needs no
follow-up edit in CLUE-604's cleanup.

isSortableType (document-types.ts:67) is the same shape, and easy to miss. It widens through a
literal array rather than the predicate, so a grep for isAxesType does not find it:

export function isSortableType(type: string){
  return [ProblemDocument, PersonalDocument, LearningLogDocument, ExemplarDocument,
          GroupDocument, AxesDocument].indexOf(type) >= 0;
}

It gates sorted-documents.ts:87 — which documents appear in Sort Work — so the next axis-native
kind is listed automatically, with no decision taken. On its own that is a lower-stakes default than
read access. What makes it worth mentioning is that it compounds with the one above: a future
axis-native kind would, in a single stroke and with neither file appearing in its diff, be readable
by every student and listed in Sort Work.

document.ts:124 (isGroup) is the same pattern, much milder. Also widened to isAxesType,
also named for groups, but it gates a MobX reaction (document-workspace.tsx:58) that closes a
stale group document on a group switch, not an access decision. It is safe today only by accident:
a class-wide document satisfies isGroup, but getDocumentOwnerFields (document-kinds.ts:197)
returns {} unless the owner type is "group", so groupId is undefined and the reaction returns
early. Nothing declares that dependency and no test pins it. hasGroupOwner fixes this one too.

This cuts against your stated scope line — "no reader is rebased onto a real axis... ten
judgment calls and this release has a deadline" — and I think that line is right for the other eight.
The case for an exception is that these are the sites where the deferral leaves behind a default
rather than just a misleading name — and the fixes are one-liners against guards that already exist.
The cheapest alternative, if you'd rather hold the scope line to CLUE-611, is a TRANSITIONAL marker
on document-utils.ts:131 and isSortableType naming what they really mean, so that whoever adds
the next axes-typed kind trips over them. Either way it's your call — I'm not asking to re-review.

2. "Each widened reader has a case for both values" is not true for three of the nine. — medium
The PR's testing section makes this claim, and it is the right policy — the transitional window is
exactly when a regression would ship. But:

document.ts:124 (isGroup) — only the new value is tested. There is exactly one .isGroup
assertion in the entire test suite (document.test.ts:374) and it uses AxesDocument. If
isAxesType were narrowed back to accept only "axes", every test still passes. The untested
branch, "group", is the value every group document in the database has today. See issue 1
above — this getter has a problem larger than its coverage.
document-title.tsx:28 and document-workspace.tsx:209 have no test coverage at all — no test
file exists for either component. Both are one-line predicate swaps, so the risk is low, but the
claim doesn't hold for them.
db.ts's CREATE_GROUP_DOCUMENT log gate is tested only with the new value. Moot in practice —
after this PR no caller passes the old one — so I'd leave it.

An isGroup-with-GroupDocument case is three lines. If you take the issue-1 rebase, the test to
add alongside it is a class-wide-shaped document, which pins the part that is currently accidental.

3. The new "axes" delete tests are thinner than their "group" siblings. — low
firebase-test/src/canonical-pointers-rules.test.ts:86-99
The pre-existing "group" describe block has three cases; the new "axes" block duplicates two,
omitting "a user outside the class may not delete the group document." That case exercises the
class_hash conjunct rather than the type disjunct, so it doesn't undermine the monotonicity
claim — the "axes" branch just has slightly thinner regression coverage than "group".

4. The deploy plan has no rollback paragraph. — low, docs only
Rollback is cheap here (Release Production is four aws s3 cp calls driven by a tag input, and every
prior version stays in version/), which makes its absence more noticeable, not less. If 7.5.0 is
rolled back after "axes" documents exist, those documents are unreachable to students until
roll-forward. No data is lost — content lives in RTDB and only one Firestore string is unrecognised —
and teachers are unaffected (isDocumentAccessibleToUser returns early for
isTeacherOrResearcher). Worth one paragraph in §9 since the deploy plan is part of this PR's
deliverable. Suggested wording:

On rollback, re-dispatch Release Production with the prior tag; "axes" documents created in the
window become unreachable to students until roll-forward. If a rollback is expected to be
long-lived, reverse the sweep by flipping the query value and the written value in
backfill-group-document-axes.ts — it runs as a service account and is unaffected by
preservesReadOnlyDocumentFields.

For what it's worth, no prior CLUE migration has shipped a rollback story either — including
consolidate-metadata-docs.ts, which is considerably more destructive — so this is a suggestion,
not a house rule being broken.

5. Step 5 of the plan could name its own re-check. — low, belongs to CLUE-604
Step 5 (7.6.0 removes the accept-both readers) is gated on "the sweep having run everywhere," which
describes a past event rather than a re-checked condition — and 7.5.0 → sweep → 7.6.0 spans a
release cycle during which document creation continues. The detection and the repair are both one
command (the script is dry-run by default and prints group-typed docs: N total), so a clause like
"re-run the dry run immediately before cutting 7.6.0; expect 0" is nearly free. Mostly CLUE-604's
runbook rather than this PR's.

Adjacent gap worth a line while you're here

src/components/document/document-workspace.tsx:341 — handleOpenGroupDocument has no .catch,
while both sibling handlers in the same file end in .catch(error => ui.setError(error)). It is an
async arrow reached from an onClick, and there is no global unhandledrejection handler and no
error boundary on that path, so any throw is silent to the user. This is pre-existing and does
not mitigate the transition risk (the bundle that fails is the already-deployed one), but it is a
small inconsistency sitting directly next to code this PR edits.

Scheduling — not a code issue, but it's the PR's own precondition

The PR states this must ship in 7.5.0, and that if 7.5.0 has been cut it "needs re-planning
rather than merging." As of today (2026-08-21): v7.4.0 (2026-08-05) is still the newest tag, so the
window is open — but that is 16 days, against 12 days for v7.3.0→v7.4.0, and both #2951 and
#2952 are still unmerged. Releases are manually dispatched, so there is no scheduled cutoff to
check against. The window is a live race between merging this stack and whoever next cuts a 7.5.0
containing unrelated work. Worth confirming with Scott that the plan still holds.

Findings that did NOT survive verification

Recorded so they don't get re-litigated:

"A straggler's "group" document written after the sweep is permanently missed" — backwards. The
sweep queries for "group", so those are exactly what a re-run finds; and type is read-only
to clients, so a swept document can't revert. Only the runbook nit (5) survives.
"No try/catch around batch.commit() means one bad document aborts the run" — Firestore's
commit() issues an atomic Commit RPC with one status for the whole batch and no per-document
attribution, so the proposed "skip the bad one" fix is not implementable against this API. The
payload is four fixed field names over {"axes","group","classWide",true,null}, so a per-document
rejection isn't reachable; malformed input would throw at set(), not commit(). Fail-fast is
also the correct policy, since step 5 removes the readers on the assumption the sweep succeeded.
"The mixed-version window is longer than 'an instant' / the failure is silent" — the PR already
says "no drain is possible because the window opens when the release does," which is that exact
point, and the silent path is in the already-deployed bundle, so this PR cannot fix it. One fair
nuance survives: the affected document is the group's only shared document, so an affected group
loses group work for that session — an impact framing the body doesn't state.
"Missed readers" — a dedicated exhaustive sweep found none.

…ype [CLUE-610] isDocumentAccessibleToUser tested the document type, and widening that test to the shared axis-native value would have moved it from a closed test to an open one: every kind added later would be readable by every student in the class the moment it was declared, with neither this file nor that decision appearing in the new kind's diff. `concurrent` is the field that answers the permissions question, and it is what the Firestore rules key their own grant on (isConcurrentClassDocument), because they cannot enforce group membership either. The owner is not used: a group- or class-owned document that is not readable by every member is a shape we may want later. Keeps the pre-sweep type as a transitional disjunct. Group documents created before `concurrent` was stamped store neither it nor the new type, and this runs on Firestore metadata for documents that have not been opened -- the thumbnail calls it to decide whether to suppress its own click, so the on-open backfill has not run and cannot. It tests the pre-sweep literal rather than isAxesType so a future axis-native kind cannot inherit read access through it. Also records that the function asks nothing about the class because it is only ever asked about the user's own class, which was previously unwritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… site [CLUE-610] `isGroup` sat in a family of getters that are all tests on `type` (isProblem, isPlanning, isPersonal, isLearningLog, isSupport, isPublished), but the question it answers is about the owner axis: a class-wide document shares the axis-native type and belongs to no group. As a type test it excluded class-wide documents only by accident -- they satisfy it, and the group-switch reaction returns early only because getDocumentOwnerFields leaves their groupId undefined. Nothing declared that dependency. Deleted rather than renamed to hasGroupOwner. It had one consumer, and a model getter of that name calling the imported guard of the same name reads as recursion at a glance; it would also leave a second way to ask a question the guard already answers, forcing that choice on every axis question after it. With it gone, every remaining member of the family really is a test on `type`. Reactivity is unaffected: the guard reads `uid`, an observable prop, inside the reaction's data function. The tests that stood for the getter now assert hasGroupOwner on models of both type values, and the class-wide case additionally pins groupId as undefined -- the fact the reaction's early return depends on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…g it [CLUE-610] handleOpenGroupDocument is an async arrow reached from an onClick with nothing awaiting it, and there is no global unhandledrejection handler or error boundary on that path, so a throw left the user watching the click do nothing. Both sibling document-open handlers in this file already end in .catch(error => ui.setError(error)). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The "group" describe block has three cases and the "axes" block duplicated only two, omitting the one where a user outside the class attempts the delete. That case exercises the class_hash conjunct rather than the type disjunct, so it is not a gap in the monotonicity of the rules change -- but the block carrying it is the pre-sweep one, which CLUE-604's cleanup deletes. Without this the class check would lose its coverage at that point rather than at a deliberate decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…LUE-610] isSortableType is the third check that would have moved from a closed test to an open one, and it is the easiest to miss: it widens through a literal array, so a grep for isAxesType does not find it. It is not rebased -- it is a membership list keyed on a type string, and rebasing it means changing its signature to take a document, which belongs to CLUE-611's per-site pass. Instead it says so, says that a later axis-native kind is listed in Sort Work by default, and directs a kind that should not be listed to check the axis fields rather than add another type test. Listing is a far weaker default than read access. In the design doc: - 5b records the two sites taken off the type and why, including why the read-access check reads `concurrent` rather than the owner and why its transitional disjunct tests the pre-sweep literal rather than isAxesType. - 8 stops claiming every widened reader has a case for both values, and names the two components that have no test file at all. - 7 step 5 gates the reader removals on re-running the sweep's dry run immediately before cutting 7.6.0 rather than on the sweep having run, which is a past event rather than a re-checked condition. - 9 adds a rollback paragraph, and replaces the pre-sweep private-document risk with the one that remains: dropping the transitional disjunct before the sweep has run in an environment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

scytacki · 2026-08-25T15:16:08Z

Thanks for the review — pushed five commits addressing it. Summary of what changed and what I deliberately didn't do.

Issue 1 — closed test → open test

Took the rebase on the two where the deferral leaves a default behind, and marked the third. Your framing of that distinction is what decided it.

Read access (document-utils.ts) is rebased — but onto concurrent, not the owner. Scott's call, and I think it's the right one: a group- or class-owned document that is not readable by every member is a shape we may want later, so keying read access on the owner would rule it out. concurrent is also what the rules key their own grant on (isConcurrentClassDocument is concurrent == true && class_hash == context_id), for the reason you'd expect — they can't enforce group membership either.

There's a wrinkle worth flagging, because it bit us before. This exact rebase was tried on 2026-07-22 and reverted the same day, and the revert was right at the time: this check runs on un-opened Firestore metadata. SimpleDocumentItem takes an IDocumentMetadataModel and nothing else, and isDocumentAccessibleToUser prefers documentMetadata over document?.metadata — so a legacy group document that stores no concurrent reads as private, and since isPrivate sets onClick={isPrivate ? undefined : ...}, the thumbnail that would trigger the on-open backfill is the one refusing the click. Not merely stale; unrecoverable from that surface.

So the check keeps the pre-sweep type as a transitional disjunct:

const isConcurrentDoc = !!metadata.concurrent || metadata.type === GroupDocument;

It tests the pre-sweep literal, not isAxesType — everything written since stores concurrent alongside "axes", so the narrow literal covers every document that can be missing the field, while isAxesType would hand read access to a future axis-native kind, i.e. exactly what your issue was about. The disjunct is grouped with the other accept-both readers in §7 step 5, and step 5's new dry-run re-check is what confirms it's safe to drop.

This is also where your observation about the check being an access decision paid off twice: the function now records that it asks nothing about the class because it is only ever asked about the user's own class — sorted-documents.ts queries context_id == classHash, document-metadata-store.ts re-asserts it after a get-by-key, and firestore.rules refuses a student another class's metadata outright (resourceInUserClass). That invariant was load-bearing and entirely unwritten.

isGroup — deleted rather than widened or renamed. You're right that it was safe only by accident; the reaction returns early only because a class-wide document's groupId is undefined, and nothing declared that. It had exactly one consumer, which now calls hasGroupOwner(primary) directly. We considered renaming it to hasGroupOwner, but a model getter of that name calling the imported guard of the same name reads as recursion at a glance, and it would leave a second way to ask a question the guard already answers. With it gone, every remaining is<Type> getter really is a test on type. Its tests now assert hasGroupOwner on both type values, and the class-wide case additionally pins groupId === undefined — the fact the early return actually depends on.

isSortableType — took your cheaper alternative. Rebasing it means changing its signature to take a document rather than a type string, which is CLUE-611's pass. Its comment now says a later axis-native kind is listed by default, and directs a kind that should not be listed to check the axis fields rather than add another type test.

Issue 2 — the "both values" claim

Corrected rather than patched over. isGroup is moot now. document-title.tsx and document-workspace.tsx genuinely have no test file, and building component harnesses for two one-line predicate swaps isn't worth it — so the PR body and §8 now state those two exceptions instead of claiming coverage that doesn't exist. db.ts's log gate left as you suggested.

Issue 3 — thinner axes delete tests

Added. Your read is right that it exercises the class_hash conjunct rather than the type disjunct — but the block currently carrying that case is the pre-sweep one, which CLUE-604's cleanup deletes. Without the duplicate the class check would lose its coverage at that point rather than at a deliberate decision.

Issues 4 and 5 — deploy plan

Rollback paragraph added to §9, close to your wording. Step 5 now says to re-run the dry run immediately before cutting 7.6.0 and expect 0, with the note that the runbook belongs to CLUE-604.

Adjacent gap

.catch added to handleOpenGroupDocument, matching both siblings.

Verification

Check	Result
`npm run check:types`	clean
`npm run lint:build`	0 errors
Full jest suite	337 suites, 3715 passed (5 skipped)
`firebase-test` rules suite (emulator)	10 suites, 435 passed (2 skipped)

435, not 434 — that's your count plus the delete case above. The PR body's stale 430 and 3613 are updated.

Still open, and it's the one you flagged that I can't close

The 7.5.0 scheduling precondition. v7.4.0 (2026-08-05) is still the newest tag as of today, 2026-08-25 — so the window is open, but that's 20 days against the 12 between v7.3.0 and v7.4.0, and #2951 and #2952 are both still unmerged. Worth a decision soon.

Two conflicts, both in comments rather than logic. firestore.rules: master removed the Jira keys from this file, because the repo is public and the instance behind them is not. This branch's transitional notes on the two accept-both clauses named CLUE-604's sweep, so they are reworded the same way -- say what has to happen rather than which ticket says so. The second note did not conflict textually but had the same problem, so it is reworded too. document-utils.test.ts: master added a getDocumentLogParams suite immediately above the accessibility suite this branch rewrote. Both kept; the accessibility suite keeps this branch's title, since it no longer tests the document type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.

scytacki and others added 12 commits August 7, 2026 16:54

docs: design for renaming the generic document type to "axes" [CLUE-610]

420ba41

feat: add the "axes" document type and a predicate accepting it or "g…

1768c35

…roup" [CLUE-610]

feat: read the axes document type as well as the group one [CLUE-610]

2fc5312

feat: create group and class-wide documents with the axes type [CLUE-610

58dbd1a

]

feat: accept the axes document type in the concurrent and canonical-r…

9ed6aad

…ace rules [CLUE-610]

feat: rename the stored type in one merged write per swept document [C…

6d03bba

…LUE-610]

docs: correct the merged-write rationale in the rename design [CLUE-610]

b63ceee

docs: record that the rename is now on CLUE-610 [CLUE-610]

bd7b3ba

github-actions Bot temporarily deployed to development August 8, 2026 13:30 Inactive

docs: sequence rules by direction, split the sweep from the release, …

02158a3

…and schedule the cleanup [CLUE-610]

github-actions Bot temporarily deployed to development August 10, 2026 16:17 Inactive

Merge branch 'CLUE-610-deferred-open-and-create-rules' into CLUE-610-…

e1a6b2a

…generic-type-rename

github-actions Bot temporarily deployed to development August 12, 2026 00:19 Inactive

scytacki mentioned this pull request Aug 19, 2026

CLUE-610: name the axis profiles and record each document's profile #2965

Merged

scytacki requested a balanced review from Copilot August 20, 2026 22:52

Copilot started reviewing on behalf of scytacki August 20, 2026 22:53 View session

Copilot AI reviewed Aug 20, 2026

View reviewed changes

Comment thread docs/superpowers/specs/2026-08-07-clue-610-generic-type-rename-design.md Outdated

Comment thread src/lib/db.ts

Comment thread src/models/document/document-types.ts

github-actions Bot temporarily deployed to development August 20, 2026 22:57 Inactive

scytacki and others added 2 commits August 20, 2026 20:08

scytacki force-pushed the CLUE-610-generic-type-rename branch from 499e8a5 to 708e2fc Compare August 21, 2026 00:10

github-actions Bot temporarily deployed to development August 21, 2026 00:15 Inactive

github-actions Bot temporarily deployed to development August 21, 2026 00:17 Inactive

scytacki added the run regression label Aug 21, 2026

github-actions Bot temporarily deployed to development August 21, 2026 03:18 Inactive

scytacki assigned kswenson and unassigned kswenson Aug 21, 2026

scytacki requested a review from kswenson August 21, 2026 13:52

kswenson approved these changes Aug 21, 2026

View reviewed changes

Base automatically changed from CLUE-610-deferred-open-and-create-rules to master August 25, 2026 13:48

scytacki and others added 5 commits August 25, 2026 11:11

github-actions Bot temporarily deployed to development August 25, 2026 15:21 Inactive

github-actions Bot temporarily deployed to development August 25, 2026 15:54 Inactive

scytacki requested a balanced review from Copilot August 25, 2026 16:05

Copilot started reviewing on behalf of scytacki August 25, 2026 16:06 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

scytacki merged commit 7526494 into master Aug 25, 2026
25 of 29 checks passed

scytacki deleted the CLUE-610-generic-type-rename branch August 25, 2026 16:10

Navigation Menu

Uh oh!

CLUE-610: rename the generic document type from "group" to "axes" - #2952

CLUE-610: rename the generic document type from "group" to "axes"#2952
scytacki merged 24 commits into
masterfrom
CLUE-610-generic-type-rename

scytacki commented Aug 8, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 8, 2026 •

edited

Loading

Uh oh!

cypress Bot commented Aug 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scytacki commented Aug 21, 2026

Uh oh!

kswenson left a comment

Uh oh!

scytacki commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

scytacki commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Why now, and the deadline

The shape of it: a transitional accept-both window

Two things worth a careful look

Review follow-ups

Deploy sequencing

Why the rules move first here and last in 7.7.0

Deliberately not here

Testing

Jira

Uh oh!

codecov Bot commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cypress Bot commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

collaborative-learning Run #20066

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scytacki commented Aug 21, 2026

Uh oh!

kswenson left a comment

Choose a reason for hiding this comment

PR Review Summary

What it does

The approach

Assessment

Issues

Adjacent gap worth a line while you're here

Scheduling — not a code issue, but it's the PR's own precondition

Findings that did NOT survive verification

Uh oh!

scytacki commented Aug 25, 2026

Issue 1 — closed test → open test

Issue 2 — the "both values" claim

Issue 3 — thinner axes delete tests

Issues 4 and 5 — deploy plan

Adjacent gap

Verification

Still open, and it's the one you flagged that I can't close

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

scytacki commented Aug 8, 2026 •

edited

Loading

codecov Bot commented Aug 8, 2026 •

edited

Loading

cypress Bot commented Aug 8, 2026 •

edited

Loading