[Backend] Support multiple characters per player #330
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: MEDIUM. Found in the August 2026 session lifecycle review (#319).
The defect
CampaignMember.character_nameis a single column, so a player running two characters — a main and a sidekick, a party of two, a character who dies and is replaced mid-campaign — cannot be represented. Every line that player speaks is attributed to whichever single character is recorded, and the summary confidently credits the wrong one.This also interacts badly with attribution: the character map is keyed by display name, so there is no way to express "this line was Kira, that line was the familiar".
Proposed fix
Move to a per-member list of characters with an optional active/primary flag, and allow a per-session override so a character swap partway through a campaign does not rewrite history. Keep the existing single-name behaviour as the degenerate case so nothing breaks on migration.
Attribution stays deterministic: with more than one character on a member, the resolver labels lines with the player's display name plus the active character rather than guessing per line, and the legend records all of that member's characters so the beat validator can accept any of them as a valid actor for that speaker.
Acceptance criteria
character_namewith no user actionDone on
feat/v4-deterministic-attributionin two commits:c4ffad2(backend) andb551a08(UI).Acceptance
character_namewith no user action1,130 backend tests passing before → 1,165; frontend 440 (CampaignDetail 30 → 39).
The one design call worth recording
The issue says the resolver should label lines with "the player's display name plus the active character". I built that, but the reasoning is worth writing down because it looks like a downgrade and isn't.
With one character nothing changes — the label is still the character name, and
test_speaker_service.py's existing cases were untouched. With two or more, the label becomesash (Kira). Labelling every lineKirawould assert something no evidence supports: one Discord account carries every character its owner plays, and audio cannot say which one spoke a line, so the familiar's lines would be filed under the main character. The track proves who the player is, so the player leads.That only works because of the last two acceptance criteria together. The legend names every character, and the validator accepts any of them — so a beat correctly attributed to Whiskers still passes even though the transcript label says Kira. Without the validator half this feature would be actively worse than the bug: correct attributions would be rejected as invented names. The alias map is scoped to characters of speakers who actually spoke a cited line, so it doesn't become a hole any invented name fits through —
test_a_character_belonging_to_nobody_is_still_rejectedpins that.Two things the issue didn't anticipate
Member erasure was scrubbing only the current character.
_labels_for_owneraddedmember.character_name; a player who swapped characters mid-campaign appears in older transcripts under the old name, so erasing only the active one would leave those lines attributed to someone who asked to be removed. It now usescharacter_names. Not in the acceptance criteria, but shipping multi-character without it would have created a data-protection bug that did not previously exist.A permission boundary nearly went missing. The pre-#330 member PATCH split character fields two ways: a GM acting on someone else could set name/pronouns/wiki-link but not sheet URL or sheet notes; a player could set their sheet fields but not link an arbitrary wiki page. The new character endpoints carried across only the lore-link half, so a GM could edit any player's sheet notes. Both directions now go through one router helper, with tests for each direction and for the create path, which had the same hole. Flagging it here because it is the classic shape of a refactor regression: the surviving half makes the code look like it enforces a policy.
Migration is expand-only
campaign_characters(composite FK tocampaign_members, partial unique index for one-active-per-member) andsession_character_overrides. The fivecampaign_members.character_*columns are backfilled and then left in place, unmapped by the ORM — a downgrade is twoDROP TABLEs and no data is ever at risk, which is what I want on a database holding recordings that cannot be re-made. It also sidesteps the one row shape a contract would lose: a member who set a sheet URL but never named their character, who would otherwise need backfilling under an invented name that then shows up as a transcript speaker label. Dropping the columns is a later contract migration.Verified against a real Postgres, because the test suite builds schema with
create_alland never runs Alembic: full chain applies clean, backfill trims whitespace and copies all five fields and skips a null-character GM, the partial index rejects a second active and permits a second inactive, deleting a membership takes its characters, and downgrade + re-upgrade leaves the source columns intact.The five
character_*names survive as read-only properties over the active character, so the ~24 read sites were untouched and cannot drift from the table.charactersislazy="selectin"— a lazy load under asyncio raises rather than returning a wrong answer, and auditing two dozen query sites for aselectinloadthey must never forget is a worse bet than one extra SELECT per member query.Notes for later
characterslist per member, with the flatcharacter_namekeys still populated from the active character so a v9 export restores into an older Quest Board. Imports of 2–8 fall back to the flat keys unchanged.PUT /sessions/{id}/character-overrides/{user_id}lives on the sessions router. It first landed under/api/campaigns/sessions/...to dodge the route-ordering rule; moved before anything depends on it.UnlinkedMemberstill carries one inline character. An export describes one per member, and the claim path converts it — withlink_lore=False, because claiming a stub restores recorded state rather than creating something new, and re-deriving the link silently promoted a recordedsecondaryowner toprimary.