[iiamlewis]

What

Game records now say which team every human player was on, in every team game. Before, only matchmade games had this, through teamIndex. This is match telemetry: the record now shows who played with whom.

Why the record didn't have it

Public and private Duos/Trios/Quads, numbered-team and Humans vs Nations games get their teams inside the simulation (assignTeams, called from GameImpl.addPlayers). The server never ran that step, so the record never learned the result.

How

• createPartialGameRecord now runs the simulation's own team assignment on the same inputs the game uses:
  • the players, in start order
  • the clan tags and friends the clients simulated with (blanked when clan tags are disabled or names are anonymized, as in toWireGameStartInfo)
  • the team config
  • the number of nations the game creates
• The result goes in a new optional PlayerRecord.team, holding the in-game team name ("Red", "Team 12", "Humans"…). The same naming is used in winner: ["team", …].
• teamIndex is left alone on purpose. It is a simulation input: replays read it back and pin players to that slot. Setting it for every player would change the order players are added in clan games, so replays would desync. team exists only in the record and is never fed back in.
• Humans are always placed before any nation, so a human's team depends on how many nations there are, not on which ones. The rules for which nations a game gets move to engine-lib (nationPickForGame). createNationsForGame builds the nations from that pick, and the record counts them with nationCountForGame, so there is one copy of the rules. Every config still gets the same nations: the compact-map case was already the same draw as a random pick.

Can the recorded team differ from the in-game one?

No. Every input is fixed once the start info is built: who plays, their order, clan tags, friends and the config. Late joins, leaves and kicks after that point don't change the start info. Clan overflow players that the server turns into spectators before the start aren't in the player list at all. The simulation can still kick an overflow player itself. That player gets no team, just as they get no in-game team.

team is left out:

• in FFA
• for players the game kicked from the teams
• on records from before this change
• if the map is unknown or the team config is invalid (the game itself wouldn't start)

Example record (Private Duos on World, a clan pair and a friend pair)

Before:

"players": [
  { "clientID": "aB3dE5fG", "username": "alice", "clanTag": "ABC", ... },
  { "clientID": "hJ7kL9mN", "username": "bob",   "clanTag": "ABC", ... },
  { "clientID": "pQ2rS4tU", "username": "carol", "clanTag": null, "friends": ["vW6xY8zA"], ... },
  { "clientID": "vW6xY8zA", "username": "dave",  "clanTag": null, "friends": ["pQ2rS4tU"], ... }
]

After:

"players": [
  { "clientID": "aB3dE5fG", "username": "alice", "clanTag": "ABC", "team": "Team 1", ... },
  { "clientID": "hJ7kL9mN", "username": "bob",   "clanTag": "ABC", "team": "Team 1", ... },
  { "clientID": "pQ2rS4tU", "username": "carol", "clanTag": null, "friends": ["vW6xY8zA"], "team": "Team 2", ... },
  { "clientID": "vW6xY8zA", "username": "dave",  "clanTag": null, "friends": ["pQ2rS4tU"], "team": "Team 2", ... }
]

(This output comes from createPartialGameRecord. World has 72 nations, so Duos gives 38 numbered teams.)

Wire / storage

• Records are JSON over HTTP. team is a plain optional string on PlayerRecordSchema, so GameRecordSchema and ArchivedAnalyticsRecordSchema read old and new records alike.
• It isn't on WirePlayerSchema, so the binary (zbin) start message is byte-for-byte unchanged.
• The API's ingest schema strips unknown player keys, so it needs its own change before it stores team. Until then the field is dropped quietly, with no error.

Tests

• tests/RecordedTeams.test.ts builds real games the way clients do (createGameRunner: start info → blanking → createNationsForGame with the real World manifest → createGame via tests/util/Setup.ts). For each player it compares player.team() with the recorded team. Cases covered:
  • Duos with clans and friends
  • public numbered teams with uneven counts
  • public compact Quads
  • Trios with a custom nation count
  • Humans vs Nations
  • matchmade pins
  • clan overflow kicks
  • disabled clan tags and anonymized names
  • 40 random lobbies
  • FFA, plus the archive JSON round trip
• I checked the test catches mistakes: breaking the compact-map nation count makes two of its cases fail.
• TeamAssignment.test.ts gains unit tests for humanTeams.
• npm run typecheck and npm run lint are clean. In npm test, everything passes except tests/UpdateRegister.test.ts, which can't run on my machine because jq isn't installed. That test covers update.sh, which this PR doesn't touch.

No UI change, so no screenshots.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: e00e71db-221c-427d-ab85-81809143776d

📥 Commits

Reviewing files that changed from the base of the PR and between 9b2b1b5 and b3a5185.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• No new commits to review - use @coderabbitai full review for a full pass

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6739d709-7229-4d66-b916-41aa1970abac

📥 Commits

Reviewing files that changed from the base of the PR and between 37a2a5f and 9b2b1b5.

📒 Files selected for processing: 2

• packages/engine-lib/src/game/NationCount.ts
• packages/engine/src/game/NationCreation.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

The change adds shared nation-selection and human-team assignment helpers. Nation creation uses the shared selection rules. Partial game records can include assigned player teams, derived from game setup and simulated player values.

Changes

Nation Assignment Recording

Layer / File(s)	Summary
Nation and team calculation packages/engine-lib/src/game/NationCount.ts, packages/engine-lib/src/game/TeamAssignment.ts, packages/engine/src/game/NationCreation.ts, tests/TeamAssignment.test.ts	Shared helpers select the nation count and derive human team assignments. Nation creation delegates nation selection to the shared rules. Tests cover assignment behavior, including Humans vs Nations and players removed from teams.
Team recording in game records packages/shared/src/SharedUtil.ts, packages/shared/src/WireSchemas.ts, tests/RecordedTeams.test.ts	Partial game records derive optional player team fields from simulated player values and game setup. Tests compare recorded teams with played assignments and check archive serialization.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant createPartialGameRecord
  participant withRecordedTeams
  participant simulatedPlayers
  participant nationCountForGame
  participant humanTeams
  createPartialGameRecord->>withRecordedTeams: player records and game start information
  withRecordedTeams->>simulatedPlayers: simulate player values
  simulatedPlayers-->>withRecordedTeams: players in start order
  withRecordedTeams->>nationCountForGame: game configuration and manifest nation count
  nationCountForGame-->>withRecordedTeams: nation count
  withRecordedTeams->>humanTeams: humans, team configuration, and nation count
  humanTeams-->>withRecordedTeams: assigned teams
  withRecordedTeams-->>createPartialGameRecord: player records with assigned teams

Loading

Suggested reviewers: evanpelle

Merge Risk: ⚪ Minimal · up to 9b2b1

The change is mergeable on the reviewed evidence. Recorded teams can be sent to the API, but they will not be stored until the API ingest schema is updated.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 8 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: recording each player's team in team game records.
Description check		The description directly explains the team-recording change, its implementation, exclusions, schema impact, and test coverage.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Nations count and teams align
Players keep their place in line
Records note each team by name
Archives hold the match the same
A tidy tally joins the game}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🤖 Claude Code Review

Verdict: No issues found. Findings: 0 critical, 0 high, 0 medium, 0 low.

I checked for bugs and CLAUDE.md compliance, including:

• humanTeams against GameImpl.addPlayers: the same resolveTeamsList total (humans + nations), the same Duos/Trios/Quads flag, the same default maxTeamSize, and the same playerTeams ?? 0 fallback as EngineConfig.playerTeams(). assignTeams places every human before any nation (nations have no clan tag or pin, and are shuffled after the other players), so leaving the nations out is safe.
• nationCountForGame against createNationsForGame (the 720-case test), and GameRunner passing humans.length as numHumans.
• GameServer.archiveGame builds records from gameStartInfo.players in start order, and the clan-tag/friend blanking matches toWireGameStartInfo.
• Package boundaries: shared imports only engine-api/engine-lib, getCompactMapNationCount moves into engine-lib with no new host APIs, and engine-lib changes come with tests.

[evanpelle]

Looks good overall. The inputs match what the simulation gets (start-info players in order, with the same clan tag/friend blanking), `teamIndex` stays untouched so replays are safe, and the RecordedTeams test checks against real games' `player.team()`, which is the right check.

Two things before merging:

1. One source for the nation count. `nationCountForGame` copies the branching in `createNationsForGame`, and only the 720-case test keeps the two in sync. Could `createNationsForGame` get its count from `nationCountForGame` instead? Then there's only one copy of the rules, and that test can go.
2. API ingest. As the description says, the API strips `team` until its ingest schema changes. Is that change planned? If so, let's link it here. Otherwise this records a field that gets dropped.

[github-actions]

🤖 Claude Code Review

Reviewed range: 37a2a5f..9b2b1b5 (incremental).

Verdict: No issues found. Findings: 0 critical, 0 high, 0 medium, 0 low.

I checked for bugs and CLAUDE.md compliance, including:

• nationPickForGame keeps the old branch order: disabled, then a numeric count, then Public HvN, then Public compact, then the full manifest.
• The compact-map case now goes through createRandomNations. Its count is always ≤ the manifest length, so it takes the shuffleArray(manifestNations).slice(0, count) path, the same PRNG draws as before. Nation names and IDs are unchanged, so the simulation stays deterministic.
• manifestNations?.length ?? 0 behaves as before: a manifest without nations still works only when nations are disabled.
• Deleting the 720-case nationCountForGame test is fine. Both functions now share one pick, so they can't drift apart. tests/RecordedTeams.test.ts and the existing NationCreation.test.ts cases still cover the engine-lib change.

[github-actions]

🤖 Claude Code Review

Reviewed range: 9b2b1b57..b3a51855 (incremental since the last Claude review).

Verdict: No issues found. Findings: 0 high, 0 medium, 0 low.

The only new commit, b3a51855, merges main into the branch. It changes none of this PR's files (NationCount.ts, TeamAssignment.ts, NationCreation.ts, SharedUtil.ts, WireSchemas.ts, tests/RecordedTeams.test.ts, tests/TeamAssignment.test.ts). It also changes nothing they depend on: assignTeams, createNationsForGame, createPartialGameRecord, PlayerRecordSchema, toWireGameStartInfo, tests/util/Setup.ts and the engine game code are all unchanged by the merge.

Checked for bugs and CLAUDE.md compliance.

[VariableVince]

(Merged main to get CI green [apart from Milestone]. After the stack of #6074 landed.)