Skip to content

fix(recon): resolve Classroom 50 roster columns in discover_repos - #32

Merged
agiacalone merged 1 commit into
mainfrom
fix/c50-roster-column-collision
Aug 14, 2026
Merged

fix(recon): resolve Classroom 50 roster columns in discover_repos#32
agiacalone merged 1 commit into
mainfrom
fix/c50-roster-column-collision

Conversation

@agiacalone

Copy link
Copy Markdown
Owner

Problem

A Classroom 50 roster.csv and a legacy GitHub Classroom roster collide on the github_id column, and they mean different things by it:

Column Legacy GitHub Classroom Classroom 50
github_id the login (bruce-wayne) the immutable numeric id (1548364)
username absent the login
name name / canonical_name split across first_name + last_name

Because discover_repos() read github_id before anything else, feeding it a C50 roster produced three failures at once — none of them loud:

  • repo names built from the numeric id: <prefix>-1548364 instead of <prefix>-agiacalone
  • every row with an unresolved numeric id silently dropped from the population
  • student names blank

Reproduced against the live c50-throwaway-tst0 roster: 1 of 11 rows resolved, and the one that did was named wrong.

Fix

Identifier precedence is now github_usernameusernamegithub_id, with a first_name + last_name fallback for the student name.

github_id stays last in the chain, so legacy behavior is unchanged whenever username is absent, and the explicit github_username override still outranks both.

Verification

  • 11/11 rows resolve against the live C50 roster; agiacalone's repo name matches the real repo on GitHub exactly
  • legacy roster shape unchanged (covered by a new back-compat test)
  • full suite: 411 passed, 1 skipped
  • the three new C50 tests were confirmed RED against the unpatched module before landing — a green test proves nothing until it has gone red

Not in this PR

repo_prefix carries two incompatible join conventions: recon_discover and triage_report require a trailing -, while feedback_deliver builds f"{org}/{repo_prefix}-{gid}" and requires none. It isn't broken today — those load from separate manifest files with separate values — but the C50 migration is exactly when a prefix gets copied between them and yields ...test--agiacalone. Left alone here to keep this fix clean and narrowly tested.

A Classroom 50 roster.csv and a legacy GitHub Classroom roster collide on
the `github_id` column: C50 stores the immutable NUMERIC id there and puts
the login in `username`, while legacy rosters store the LOGIN in
`github_id`. Feeding a C50 roster to `lectern recon` therefore built repo
names like `<prefix>-1548364` instead of `<prefix>-agiacalone`, dropped
every row whose numeric id was unresolved, and left the student name blank
(C50 splits the name across first_name/last_name).

Identifier precedence is now github_username -> username -> github_id, so
behavior is unchanged whenever `username` is absent and the explicit
github_username override still outranks both. Student name falls back to
first_name + last_name.

Verified against the live c50-throwaway-tst0 roster: 11/11 rows resolve and
agiacalone's repo name matches the real repo on GitHub. Full suite 411
passed / 1 skipped. The three C50 tests were confirmed RED against the
unpatched module before landing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P2mBwr8n3r97t4g9AUV8j5
@agiacalone
agiacalone merged commit d923487 into main Aug 14, 2026
4 checks passed
@agiacalone
agiacalone deleted the fix/c50-roster-column-collision branch August 14, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant