CLUE-610: Sort Work + workspace UI for class-wide documents

[scytacki]

Sort Work + workspace UI for class-wide documents

The groundwork already merged to master creates the Driving Question Board but surfaces it nowhere; this PR makes it visible, sectioned, titled, and editable.

The presentation half (#2940) has merged, so this PR targets master and holds the remaining half. That PR was split out of this branch and carried offeringId onto both metadata types, the thumbnail and title bar reading concurrent, and the split of a document's owning group (groupId) from its owning user's group (groupIdOfUserOwner). This PR's container guard builds on the first of those, and its owner guard on the last. Together the two replace the earlier single 47-file PR #2935, which can be closed.

The design spec is committed at docs/superpowers/specs/2026-07-27-clue-610-sort-work-ui-design.md and is the document to review alongside the code. Note that the spec records the design as it stood when written — several names changed during implementation (see "The axis-modeling checkpoint" below), and docs/document-axes/ is the current-state record. The roadmap at docs/document-axes/README.md flips kind to done and records the read side of owner, container, and curriculum.

⚠️ This supersedes the edit predicate that shipped in PR #2930

PR #2930 (CLUE-525) landed canUserEditDocument on master while this work was in flight, and the base branch has since merged it. This PR replaces its semantics, and the two disagree on a real case, so it is flagged here rather than left to be discovered in review.

	CLUE-525, now on master	this PR
collaborative test	type === GroupDocument	the stored concurrent axis
group membership	groupId === user.currentGroupId	the document's owner uid vs user.userIdForGroupDocuments
metadata vs. document	read field by field	one source chosen outright
published documents	editable by the publisher	never editable by anyone
researchers	editable where owned	never given an edit affordance
class-wide documents	not addressed	editable by any member of the class

The group-membership row is the one that changes behavior. A group id is unique only within an offering, so groupId === user.currentGroupId matches across offerings — group 3 of a past assignment is a different set of students, and its document still satisfies the test. That reaches real documents, because Sort Work's "All" filter lists everything a class has produced, including documents from offerings it has moved on from. Comparing owner uids is exact instead: group_<offeringId>_<groupId> carries the offering, so two same-numbered groups can never be confused.

The merge resolution therefore removed CLUE-525's describe("canUserEditDocument") block from document-utils.test.ts: two of its cases assert the superseded contract, most directly "prefers the metadata groupId over the document's" → true, against this PR's "does not treat a document model's groupId as evidence of group ownership" → false. Every other case that block covered is covered here. The one case it uniquely had — a group document reached through the document rather than its metadata — was ported to the new contract rather than dropped.

The axis-modeling checkpoint, resolved

The project deferred "does scope need a helper, and if so what shape?" to its richest consumer. Working through this PR's five consumers produced two findings.

First, "scope" was three questions wearing one name. It has been split into owner (who the document belongs to), container (where it is kept: class → classUnit → offering), and curriculum (what it is about: nothing → unit → investigation → problem). Every consumer turned out to want exactly one of them, and picking the wrong one is a live defect rather than a style question:

Consumer	Branched on before	What it actually asks
Workspace title bar	document.isGroup	kind → registry presentation
Thumbnail treatment	type === GroupDocument	concurrent → collaborative styling
Edit gate	uid === user.id	concurrent, then the document's owner and container
Sort Work listener	—	curriculum breadth: a whole-unit document must survive a problem filter
byGroup / byName sectioning	type === GroupDocument	which owner the document belongs to

The listener is the sharpest illustration. It fetches documents about a whole unit, and it must not be shaped as a container question: an exemplar is kept in the classUnit container but is about a problem, so a container-shaped query would surface exemplars from every problem in a unit whenever a student was on any one of them.

Second, the answer is narrow named guards — no level enum and no unified struct. src/models/document/document-axes.ts holds hasGroupOwner, hasClassOwner, isInClassUnitContainer, and getCurriculumLabel, each reading only its own axis's stored fields — no type, no kind, no registry. That last point is load-bearing: Sort Work's "All" filter lists documents from other units, whose kinds were never registered this session, so a registry lookup would silently misfile them. The decision record, the field-by-shape table, and what is not covered yet are in docs/document-axes/reading-axes-in-code.md.

On the creation side the kind registry declares ownerType (user / group / class) and containerType (class / classUnit / offering). There is deliberately no group container level — a group document is kept in the offering alongside the problem documents its members write, and what makes it the group's is its owner.

What's in the PR

Axes

1. Axis guards and the owner/container/curriculum split (document-axes.ts, document-kinds.ts), per the checkpoint above. On the creation side the kind registry declares ownerType and containerType; on the read side, narrow guards over stored fields.
2. Explicit-null curriculum fields — the classUnit container stamps investigation: null, problem: null rather than omitting them, following the convention the rules already encode (hasPresentField treats a null field as absent). Firestore cannot match a missing field, so this is what makes "about a unit but not a problem" queryable.
3. A group owner is read off the uid, testing the group_ prefix its minter used, symmetric with hasClassOwner and its class_. That leaves the uid the single authority on who a document belongs to; the stored groupId now carries only Sort Work's group label and the two queries that retire with the canonical-pointer migration.

Sort Work

4. A whole-unit listener — a sibling of the existing unit-less listener, active only under the Investigation/Problem filters, querying unit in <variants> && investigation == null. It names no type and no kind. Equality-only, so no composite index. It is a curriculum query, not a container one: an exemplar is kept in the classUnit container but is about a problem, so a container-shaped query would surface exemplars from every problem in a unit whenever a student was on any one of them.
5. Sectioning by owner — byGroup gets a "Whole Class" section ahead of the numbered groups; byName files class-wide documents under "No Name". Ordering comes from structured sort keys instead of parseInt-ing the display label — which mattered because the group term is translatable per unit and "Whole Class" contains no digits.
6. A cross-offering defect fixed. Sort Work's "All" filter spans every offering a class has worked through, and byName resolved a group document's members by bare group id. Group 3 of a past assignment is a different set of students, so its document was attributed to this offering's group 3 — or dropped from the sort when no group of that number existed here. Resolution now goes through getGroupByOwnerId, which builds the whole owner id and compares; a document from another offering gets its own section, labelled by where the work came from (Groups from sas-2.4).

Workspace

7. Titling a class-wide document from another unit — the kind registry resolves slot titles, with a fallback naming a document whose declaring unit's config is not loaded (Driving Question Board (sas-2.4)), and a unit-declared title is scoped to its own unit so two units declaring the same kind don't borrow each other's wording.
8. One shared edit predicate — canUserEditDocument in document-utils.ts, beside its sibling isDocumentAccessibleToUser, replacing two divergent inline gates (Sort Work's ownership-only check and the resources pane's tab check).

Firestore rules

9. Canonical slots addressed as container + owner + label. A pointer path is now canonical/v1/classes/<classHash>/(offerings/<offeringId>|units/<unit>)/owners/<uid>/slots/<label>, built segment-by-segment from the document's own fields on both sides (getCanonicalPointerPath and the rules' canonicalPointerPath). Each container names only itself, so a new container level can be added later without moving pointers that already exist — they are immutable by rule. Taking the owner segment straight from uid removed the last reference to groupId anywhere in firestore.rules.
10. History-write rules for synthetic owners + a guard on the concurrent field's write path (see below).

Security: two defects found and fixed during review

Both were defects in the plan, caught before merge, and both have regression tests.

The edit predicate initially granted Edit on the user's own published documents. A publication's uid is the publisher, so the ownership arm matched in the Class Work tab where the old tab-based gate had withheld it. Not a write hole (published documents are forced read-only downstream) but a dead button, and it contradicted a live Cypress assertion in nav_panel_test_spec.js. Now excluded via isPublishedType, with researchers denied explicitly.

The history-write rule was forgeable. Gating on concurrent was unsound because concurrent was not a read-only field: any class member could update a classmate's document with {concurrent: true} and thereby grant themselves — and the whole class — read and append on that document's private history. Fixed at the write path rather than the read path: concurrentChangeOk() permits concurrent to change only on type == "group" documents, and type is itself read-only so it cannot be flipped first.

That write allowance is transitional — it exists only because two backfill paths merge-update concurrent onto existing group documents. Once the migration is complete it should be removed so concurrent becomes settable only at creation, and the create path should be constrained too (isValidDocumentCreateRequest currently checks neither concurrent nor uid). Breadcrumbs to that effect are in firestore.rules, the backfill script, and both design specs.

Separately, getDocumentOwner now throws for an unregistered kind rather than defaulting to the creating user. Defaulting would hand a group's or a class's document to whoever created it and — because a canonical slot is addressed by its owner — file it in that user's slot instead of the shared one, both silently. Two existing tests were relying on the default; they now register the kind, as production does.

Migration

scripts/backfill-group-concurrent.ts → scripts/backfill-group-document-axes.ts, now normalizing the stored axes of every type: "group" document in two passes selected so they cover disjoint sets:

pass	selects	stamps
existing	has a groupId, missing concurrent	{ concurrent: true, kind: "group" }
new	no groupId, missing curriculum fields	{ investigation: null, problem: null }

Selecting on groupId rather than on the value being written matters: a class-wide document stamped kind: "group" would break both its title resolution and its canonical-pointer slot, since the slot label is the kind.

Firestore rules deployment sequencing

The rules can be deployed before the code, in the usual order. Two parts of this change look at first like they might not allow that. Neither does, and the reasons are worth recording because both are non-obvious from the diff.

1. The history rule is purely additive.

The new arm is isConcurrentClassDocument(), gated on concurrent (not on canonical). Pre-existing group documents don't carry concurrent until a backfill runs, and the backfill only ships with this code — which reads like a catch-22 against a rules-first deploy: the rules would depend on a field nothing in production writes yet.

It isn't one. Under the rules on production today, history create and read are gated by isAuthed() && userOwnsDocument(), and userOwnsDocument() compares the document's uid to the caller's platform_user_id. Every group document's uid is the synthetic group_<offeringId>_<groupId>, which never equals a real user id — so no user can create or read group-document history in the authed domain today. The new arm can only widen that, and widens it to nothing while concurrent is absent. Deploying it early takes nothing away and enables nothing that is not already inert.

2. The canonical-pointer path move denies a path production code still uses — and that is fine.

The path already moved once, in 7.4.0, which is now on production: pointers live there at canonical/v1/…/classes/<context_id>/(offerings/<offeringId>/groups/<groupId> | units/<unit>)/slots/<label>. This PR moves it a second time, inserting an owners/<uid> segment. So unlike the first move, this one denies a path that production code is running against today.

These rules delete the 7.4.0 match blocks (offerings/{offeringId}/groups/{groupId}/slots/{label} and units/{unit}/slots/{label}) and grant only the canonical/v1/…/owners/<uid>/slots/<label> layout above; anything unmatched falls through to the top-level default-deny. Confirmed against the emulator — under these rules a class member's read and create at the legacy path are both denied. So between the rules deploy and the code release, a client on current code would take a permission-denied on the first step of getOrCreateCanonicalDocument, the pointerRef.get(). Nothing catches it, so getOrCreateGroupDocument() would reject.

That costs nothing, because group documents are not released in the authed domains. They are opt-in per unit via groupDocumentsEnabled, which defaults off; the only unit enabling it is demo/units/qa, and the demo partition has its own recursive-wildcard allow read, write: if isAuthed(), so it is unaffected by the authed pointer paths in either deploy order — as are dev, qa, and test. The authed-domain group-document path is already non-functional, per the history rule above. Denying the legacy pointer path breaks it further, not something that works.

So no dual-path rules release is needed. The rules do not have to carry both the legacy and the canonical/v1 pointer blocks through a transition window: deploy the rules, release the code after.

Existing legacy-path pointers need no migration either. The new code never consults them — with no v1 pointer present, getOrCreateCanonicalDocument falls through to findLegacyGroupDocument, which queries the documents collection by context_id/offeringId/groupId, finds the same group document, and claims a v1 pointer for it. The document's existing canonical label is rewritten to the same value, which canonicalFieldOk permits as a no-op change. The old pointers are simply abandoned.

Testing

Full suite green: 3591 Jest tests plus the emulator rules suite. check:types and lint:build clean.

New coverage includes the axis guards across every stored document shape, sectioning and sort-key ordering, the cross-offering group cases (attribution, the missing-group-number case, and section ordering), the whole-unit listener (registration per filter, dedupe, disposal, clearing on filter change), the edit predicate across all arms including negative cases, and rules tests for the owner segment of a canonical slot plus synthetic-owner history writes and the escalation that a class member cannot flip a classmate's document to concurrent.

Three verification notes, since none is visible in the diff:

• The emulator suite must be run with --runInBand. The eight suites share one emulator and reset state per test, so running them in parallel produces ~51 spurious failures. firebase-test/package.json's test script does not pass the flag — pre-existing, and worth its own fix.
• The new rules tests were checked for discrimination, not just for passing: hardcoding the owner segment in canonicalPointerPath fails all three claim tests, and the guards' behavior tests fail when a guard is stubbed out.
• The split from PR CLUE-550 Stage 3: Sort Work + workspace UI for class-wide documents #2935 was verified by tree equality at the time it was made: the two branches' tips were byte-identical (git diff between them empty), so nothing changed behaviorally in the reorganization. They have since diverged, as this branch has merged master several times.

Not done — for the reviewer

• Manual two-student end-to-end check has not been run. The checklist is in the design spec; the two items no automated test covers are the two-student concurrent edit round-trip and the workspace title bar.
• Eager-open cost is ~670–730ms per unit load on the fast path, measured against a live dev server. Only demo/units/qa declares a slot today, so no production unit pays it. The fix is specified in the spec (defer the open, not the get-or-create) and wants its own ticket.
• Whether to drop the stored groupId is deliberately deferred. Nothing asks it the owner question anymore, but byGroup still uses it for a section label, and it does so in a way that intentionally ignores the offering: a group-3 document from a past assignment shares "Group 3" with this one's, which matches students carrying groups forward between offerings. Removing the field forces a behavior decision there, which is the real gate — not any remaining code cleanup.
• Deliberate deviation from PR CLUE-550: auto-generate a class-wide Driving Question Board per unit #2890: its absolutely-positioned centered DQB title bar is not ported — it is cosmetic and would hardcode a kind in CSS, which cuts against making "add another class-wide document" a configuration change.

🤖 Generated with Claude Code

---

Replaces #2942, which was opened from the previous branch name; its review discussion stays readable there.

🤖 Generated with Claude Code

[scytacki]

@tealefristoe thank you for another thorough review. I added one small change to this PR but haven't addressed your inline comments yet. I'm curious what you think of the draft PRs and new axis profile concept described below.

Storing Titles

I was torn by the question of storing the titles in the class-wide documents or not. Since you wanted it, I had Claude draft what that would look like: #2962

The first thing this exploration turned up was that I should have made a getKindDefinitionFor method in the current PR, so I added that. For the case of dynamic kinds this method takes care of finding the kind definition in a generic way, not just some code specific to titles. If we keep dynamic kinds I'm pretty sure there will be other reasons we need to find the kind definition, for example when copying a document. If we allow a user to copy a dynamic kind document from another unit we'll need to know which axis values to copy. This kind of copying definition hasn't been implemented in any of these PRs but it is described in the docs.

Another thing this exposed more clearly was how we handle documents from other units. Before any of this work we already had to deal with this with problem documents. These do not have a title in the document. Their title is the name of the problem. In the sort work tab when looking at all documents the user can see these problem documents from other units. What we've done is name them by their curriculum position instead, like "Problem doc from sas-1.2". Since we don't have that unit's curriculum loaded we don't know the real title to give them. Along these same lines, even if we store the title of class-wide documents in the document itself, we should differentiate a Driving Question Board from the current unit and the Driving Question Board from another unit. So Claude came up with a rule for this:

A title identifies a document only within the scope its author controlled. Displayed outside that scope, it needs the coordinate that restores identification.

Supporting this means there is actually a net increase in the number of production code lines (+20/−12). So it doesn't help with complexity. It does make it more user friendly though.

The other thing this points out is that storing the title for class-wide documents would be inconsistent with what we do with problem documents. I think it makes sense to not store the title for problem documents.

Given all of this my inclination is to leave it as is and not store the title. If we get complaints about not being able to show the correct title, then I think the best fix would be to load the unit that defines the titles for both the problem and class-wide documents. This way we solve both problems and keep everything consistent.

Replacing Dynamic Kinds with a classWide kind and variants

I wanted to see what this would look like so Claude built a PR on top of the title removal PR to see what it would look like to add a new field to track the variant and remove dynamic kinds: #2963

Before looking at these changes my argument against this was that this goes against the point of this document axes refactoring. By making the kind "classWide" it seems we are treating the kind as if it is a type again. By having this as a kind it is really tempting to simplify places in the code that are currently using axis values and have them just look at the kind. For example in Sort Work when it is deciding how to organize the sections it could just check the kind instead of using the generic "hasClassOwner". And when deciding if someone can edit a document it would be easy to start just looking at the kind instead of what it currently does.

After looking at the draft PR, adding the variant field doesn't really simplify the code. Just looking at production code it is +42/−41, a wash. It requires adding this field to several type definitions and then it needs to be used in several places instead of, or in addition to kind: the workspace title bar stylesheet, the last-resort display label getDocumentKindLabel(kind).

It does reduce the comments though. Those comments explained how a dynamic kind's definition is scoped to its config.

One place that is simplified is registerClassWideDocumentKind, but really this is just replaced with a static registerDocumentKind which doesn't really look much different than what registerClassWideDocumentKind did. Another place is getKindDefinitionFor, which goes away, but it isn't much code.

The main simplification might be a conceptual one. However I find that supporting variants of kinds just shifts this concept to a new field. The actual documentation (written by Claude) actually got bigger by 3 lines.

Looking at this the other way, one of the things you wanted was to enumerate every kind in one place. This is the reason that registerClassWideDocumentKind is in document-kinds.ts. It does keep the axis values for this profile (see below) in the same place as the static kinds.

Axis Profiles (a new concept)

What your comments prompted me to come up with was a label for something that is already happening in the code and documentation. And it addresses your strongest argument against dynamic kinds: migrations.

If we keep dynamic kinds, we are referring to class-wide documents but really those don't exist as something in the state of documents. So what do we call this class-wide thing? Likewise in the code we have personalLikeKindInfo, problemLikeKindInfo, and group documents. These 4 bundles of axis values have four things we need from them:

• Assignment at creation — personalLikeKindInfo, problemLikeKindInfo in document-kinds.ts and the class-wide axis values in registerClassWideDocumentKind.
• Recognition of existing documents — the shorthand the comments and tests already use.
• Many kinds to one bundle — four kinds share personalLike, four share problemLike, and in the future 2 kinds of class-wide documents
• Migrations — if we want to change the axis value assignments we probably want to change them for all kinds of this bundle.

So what I came up with was calling this concept a "document axis profile" or "axis profile" or just "profile". So we have a class-wide axis profile, and we perhaps say we have a problem axis profile and personal axis profile.

Then taking this further we can use it to address two of your other concerns: migrations of dynamic kinds, and seeing all axis values in one place so we can make sure all known combinations are tested.

By storing the axisProfile on the document itself it supports the migrations. Without it we'd have to search each unit for its dynamic kinds, then find documents with that unit and kind and then update those to the new axis values. With it we can just search for the axisProfile. Additionally by defining the axis profiles in code and only allowing kinds to use specific profiles we are now limiting the number of combinations.

This is similar to the kind - variant approach you came up with, but now it is axisProfile - kind. Both store two strings and carry the same information; the difference is which one holds the cohort and which holds the per-document identity.

That difference is what makes the advantage possible. Under kind - variant the cohort key is kind, which the runtime already leans on: it's the registry key, it's the canonical slot label, and it's the CSS hook on the title bar. It has to be readable, so it can't be kept away from the code that shouldn't use it. Under axisProfile - kind the cohort key does nothing at runtime at all, so we can leave it off IDocumentMetadata, DocumentMetadataModel, and DocumentModel entirely — which is what the axis profile PR does. Reading it would require widening a type first, so it shows up as a deliberate change in review rather than as a convenient one-liner. To be clear, this is a compile-time barrier rather than a real one: the value is still on the object and a cast would reach it.

Because this sounded good I made a draft PR of it: #2965

It does have a net gain in production code lines, +67/−43, but to my mind it seems to reduce the complexity of the code since it is formalizing something that was already there in the form of problemLike and personalLike.

Comparing it to the PR adding the variants, it doesn't need to have the variant passed into the CSS; that stays just the kind.

The one thing it doesn't solve is having one place that shows all possible kinds. As long as we have dynamic kinds this isn't possible. However my argument is that the usage of the kind should be limited enough that we don't need it. The axisProfile covers creation and migration. The one thing that isn't addressed is how we will handle copying documents. Since we haven't implemented this yet it seems fine to punt that. We have multiple options to handle it, and with axis profiles we could support copying dynamic kind documents from other units.

Where I land

Against your three recommendations:

• Save the titles when they're created. I'd rather not, for now. Storing them turns out to be a small net increase in production code rather than a decrease, and it makes class-wide documents inconsistent with problem documents, which resolve their titles the same way. If this becomes a real problem for users, loading the defining unit is the better fix because it solves both at once.
• Make classWide the kind and tell them apart with a variant. I'd like to counter with axisProfile + kind. It gives you the same migration story and a better version of the central list — every axis combination in one file, rather than every kind name — while keeping the cohort key out of the runtime types.
• Drop dynamic kinds. I'd like to keep them. Your migration argument was the strongest one against them and I think axis profiles answer it. The part I can't answer is enumerating every kind in one place; that isn't possible while kinds are dynamic, and I'm betting a kind with a small enough job doesn't need it.

I'm not trying to close this off. If the draft PRs and axis profiles don't convince you, please let me know. Your thoroughness and critique is making this code better.

[tealefristoe]

I'm glad to hear the reviews are helpful. They take a ton of time and are honestly pretty draining for me to provide, so it's good to know they're making a difference.

There's a lot going on, so I'll try to respond in sections here. First, a general thought about where I'm coming from with these reviews:

• Fewer lines of code is fantastic, and generally indicates things are simpler. But what I'm really trying to get at is a system that will be grokable by everyone working on CLUE, so we can all be on the same page when talking about and developing CLUE. This is a somewhat amorphous idea that I think claude has no concept of, but it will have measurable outcomes like miscommunications and bugs.

About saving classWide titles vs looking them up dynamically:

• Don't we need the unit disambiguating code even if we look up titles dynamically? Stored or looked up, titles have the same chance of colliding. I don't think it's fair to include those extra lines only with the stored titles.
• When I suggested storing titles on the document itself, I was thinking it would be easy to follow the pattern of documents that can be renamed (personal documents? Let's assume personal documents, even if that's wrong). So my thought is that when you're looking up a document's title, no matter what kind, you'd first look for a saved title, and only fallback to the kind specific variant if no title is saved. Maybe a new axis would be necessary to specify if a document's title can be changed via the UI or not.
• I don't think you should have to change the way problem documents handle titles based on classWide documents. They seem independent to me, in the same way that personal documents and problem documents are currently independent.
• I still think storing the title will be easier to understand, and when things go wrong it will be more obvious what's going on.

classWide kind vs dynamic kinds:

• I guess a big red flag for me in reading your response about this is that I seem to still not understand what kind is supposed to be. Maybe that's a me problem, but if I don't get it after trying hard to understand all these PRs, who will understand it?
• The slot/${kind} address rule isn't set in stone, and I think we should reconsider it. To me, it adds an extra meaning to kind, and it doesn't have to be that way. Plus, I believe the only canonical documents that exist right now (group) explicitly already break this rule!
• Ultimately what I'm proposing is a separate field to specify the slot. So maybe it should be slot instead of variant (though I do feel like slot is a pretty awkward term in general here).
• Honestly, what would be best would be to use slot/${title} for this, but that has the downside of making it way harder to change the title in the future. Though do note that in the current system, we use kind for both the slot and the fallback title when we're in a different unit, so we're almost already using slot/${title}.
• I guess part of what makes me think this is ok in general is a bit of a loose interpretation of canonical. Strictly, canonical is supposed to mean there's a single instantiation of that kind of document. But the way I'm thinking about it, there could be an arbitrary finite number of that kind of document. So instead of a single one, we have a set N, which can vary by unit. To me this makes sense, but I can now see how it's a bit of a leap that maybe wasn't obvious to you, and maybe you won't agree with it being a good idea.

Axis profiles:

• I'm happy to call these things whatever you want 😄 I just want to make sure this is understandable, it's only as complex as it needs to be, and hopefully the names are intuitive so anyone working on CLUE can have a pretty good idea of what they mean without reading any documents.
• That said, I do want to break this proposal down a bit. From my understanding, it splits kind's responsibilities:
  • Before, kind determined the axes, and stuck around for migrations, to determine the title, to determine some other visuals, and I guess was used to address the canonical slot.
  • Now, kind determines profile, and sticks around for titles, visuals, and canonical slot. profile is now in charge of the axes and sticks around for migrations.
• Is that a fair assessment? I think the biggest difference with the profile - kind split vs the kind - variant split I was originally proposing is how the responsibilities are distributed. With kind - variant, I basically assumed variant would only be used for the canonical slot, while kind would keep all its other responsibilities (so variant was probably never a very good name). It still seems better to me to separate that single responsibility into a different field, but I can get behind profile as proposed.
• I guess my only other concern to bring up is if this setup will lock different kinds together. Is there ever a situation where we'd want personal docs and learning logs to have different axes? Would this system allow us to do that?
• Enumerating every kind in one place isn't as important to me as having a centralized list of all the axis combinations, which profile supports.

Only tangentially related thoughts:

• I haven't thought carefully about copying documents, but why would you need to look up anything about the kind? Shouldn't you already have access to all the axes in the document itself? Is there a wrinkle I'm not seeing here?
• Production line changes vs total line changes in these PRs:
  • +20/-12 vs +163/-129
  • +42/-41 vs +194/-238
  • +67/-43 vs +381/-95
    How many camp fires are we burning for even simple code changes these days? 😢

[scytacki]

@tealefristoe thank you again for sticking with this.

I'm going to go with axis profiles. I see you reviewed that PR so I think I'll merge this one first so we can move on and then update the axis profile PR so there is only a brief period of class wide docs without axis profiles.

classWide titles

It is interesting you are asking all of the same questions I did when generating the draft PR for this.

The unit disambiguation code already exists in the current PR. It just changes shape when we start storing the titles. This is one reason to generate the draft PRs so I could see what it would really look like. So the +20/−12 is not the cost of disambiguation; it is the difference between the two shapes of it.

The simple "if the document has a title, then show it, otherwise fallback to the kind based title", doesn't work well because of the unit disambiguation. A stored title still has to be qualified when the document is from another unit: a "Driving Question Board" stored by sas needs to read as something like "Driving Question Board (sas)" when it shows up in a list next to this unit's own board. So the lookup doesn't end at "is there a stored title?". It still has to know which unit the document belongs to and compare that against the current one, which is the part that makes it more than the simple rule. I asked the same question about needing an axis for editable titles. The current reason that it isn't needed is that concurrent documents show a non-editable title bar. If we did need to customize it in the future we could build that into the permission axis.

The connection with problem documents is that the title of the problem document is specified in the unit. If we use variants or dynamic kinds we still need to specify the title of the class wide document in the unit. So if an author changes the unit the title shown to users ought to change. So the argument is about consistent behavior or implementation. If an author changes the problem name and they see the problem document titles update for their users, they would expect changing the title of the class wide document to also cause the titles to update for their users.

At this point I think we'll just have to agree to disagree. You've definitely pushed me to pay more attention to this, and I will try to get the "non-current unit" loading code in at some point so both problem and class wide titles are displayed properly even when in a different unit.

classWide kind vs dynamic kinds

I continue to hope that kind will make more sense after the migration is more complete. With the addition of the axis profiles, the stored kind should be just for the "presentation" of the document and its title when it is a computed title. The part that still makes this confusing (I think) is the use of "kind" as the name we use when discussing and defining documents. It is a name for the combination of axis values that make up that type of document. But I hope once we start discussing document kinds this will feel more natural.

I don't see using kind for the slot label as a rule. It is just a shorthand so when creating the document, we don't have to separately define the slot label. When the document is created there is a separate canonicalLabel creation parameter that is used for this. So if we had the case where we wanted the same kind to be used in multiple canonical slots, we'd just pass a different value for the canonicalLabel, but they'd have the same kind. Regarding your last point in this section: "Strictly, canonical is supposed to mean there's a single instantiation of that kind of document." That is not what the code does, the code supports any number of slots with the same kind of document. This is why the canonical field is a string label not just a boolean. The Firestore rules need to know which slot a document is supposed to be in; they use the canonical field for that.

The unit defined class wide documents make this blurry though. In that case the author is providing both the kind identifier, and the slot label with a single value. These unit configurations are defining both the kind and also the template for making a canonical instance of that kind. We could make this more clear and have the author provide a kind identifier, a canonical slot label, and a title. While that makes it more clear that the same kind can be in multiple slots, it means we are requiring the author to add another identifier that is currently unnecessary. So I think it is better not to require this until it is needed.

Looking through the code I found a few places that make it seem like this is a rule and that canonical is just a boolean. I've tried to update all of those places to make this more clear.

As you wrote using slot/${title} means an author can't change the title for the kind without us doing a migration. So this makes the title not really authorable. If we do that, I don't think the title should be in the unit configuration anymore. Also it does have to work as a Firebase compatible path segment, so that limits what characters can be used.

Axis Profiles

Your assessment of them is mostly correct. kind is mainly stored for titles and visuals (presentation). As discussed above, the slot label is set when the document is created, so in the code it is independent from the kind. As discussed below, if we need to split an axis profile then the kind can be used for profile splitting migrations. The profile defines the combinations of axis values, and is stored to help with migrations. In the future the profile might help with copying too (see below).

I misunderstood your variant proposal. My interpretation of it was that variant was needed to remove dynamic kinds. The part I focused on was the document titles and possibly presentation more than the slots. I think that was because I knew about the canonical field value defining the slot label. So that was already a separate field.

Regarding locking different kinds together. We'd have both the axisProfile and the kind to tell them apart. If in the future we find that learning logs and personal documents have different axis values then we'd make a new axis profile and migrate the existing documents using the combination of kind and axisProfile. Normally only the axisProfile would be used, but if we need to split an axisProfile then we can use the kind.

Copying

This is now spelled out better in axes.md, but here is a summary.

Publishing

Publishing a problem or personal document is basically copying it. What fields should change when publishing can be based on the kind. In both cases the owner stays the same, but the permissions change (when they exist for real). The permissions would no longer allow the owner or anyone else to edit it. They would let the class view it. For problem documents they are no longer canonical, so that axis would change differently than for personal documents.

Problem documents don't officially use the canonical field, but the canonical axis was built to support them.

Copying

Copying a personal document just makes a copy and changes the owner to the person who made the copy.

Copying a problem document is different. It changes the owner, but it also turns the new document into a personal document. So this currently means setting the kind, curriculum, and container axes.

So rather than having a branch based on the kind in the copying implementation of CLUE, the current design proposes that the kinds define a template for this copying. The template defines how to change the axis values from the original when making the copy. This template could be defined in a few ways. The copy template could be looked up from the value of the kind axis, similar to how the title can be looked up from the kind. Or we could use the new axisProfile to find the copy template. Or we could add a new field to the document metadata which would name a copy template.

Token Burning

I think the PRs are worth the tokens. Having concrete code to look at helped me. There is a finer point here about how it is the training the AI models that uses the most energy, and just using the models doesn't use a huge amount. But it is nuanced and companies don't publish their stats.

[Copilot]

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.