Skip to content

feat: replace numeric table assignment with dynamic two-floor lettere…#426

Open
jackzheng-cs wants to merge 7 commits intomainfrom
381-update-table-number-to-letter-number
Open

feat: replace numeric table assignment with dynamic two-floor lettere…#426
jackzheng-cs wants to merge 7 commits intomainfrom
381-update-table-number-to-letter-number

Conversation

@jackzheng-cs
Copy link
Contributor

Summary

@jackzheng-cs jackzheng-cs linked an issue Mar 5, 2026 that may be closed by this pull request
@michelleyeoh michelleyeoh requested a review from Copilot March 24, 2026 04:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates CSV ingestion table assignment to generate two-floor letter-number table labels (e.g., A3) and prioritize “Best Hardware Hack” teams on floor 1, aligning with Issue #381’s new table labeling scheme.

Changes:

  • Added a two-floor table assignment algorithm that distributes teams across lettered rows.
  • Prioritized hardware teams on floor 1 and filled remaining floor 1 capacity with non-hardware teams.
  • Replaced the previous sequential numeric table numbering assignment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const { storedValue: tableNumber } = useTableNumberContext();
const { team, judges, loading, error, fetchTeamJudges } =
useTeamJudgesFromTableNumber(tableNumber ?? -1);
useTeamJudgesFromTableNumber(String(tableNumber ?? ''));
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useTableNumberContext() currently normalizes the stored value with Number(storedValue). Once table numbers become letter-number strings (e.g. A13), that conversion becomes NaN, which then turns into the literal string 'NaN' here and also prevents the if (tableNumber) gating from working. This call site should pass the actual string table label, and the context/provider should stop coercing to number so judging works with the new format.

Suggested change
useTeamJudgesFromTableNumber(String(tableNumber ?? ''));
useTeamJudgesFromTableNumber(tableNumber ?? '');

Copilot uses AI. Check for mistakes.
Comment on lines +84 to +85
const rowLetter = secondFloorStartLabel.trim().toUpperCase().charAt(0);
const secondFloorStart = (rowLetter.charCodeAt(0) - 64) * 1000;
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

secondFloorStartLabel is treated as a row letter, but the function doesn’t validate it; charCodeAt(0) on an empty string yields NaN, and non-letter input yields incorrect thresholds. Also, toComparableTableNumber supports legacy numeric table numbers, but the threshold computation cannot accept a numeric cutoff anymore. Consider parsing secondFloorStartLabel as either (a) a number cutoff when it’s numeric, or (b) a validated A-L row letter cutoff when it’s alphabetic.

Suggested change
const rowLetter = secondFloorStartLabel.trim().toUpperCase().charAt(0);
const secondFloorStart = (rowLetter.charCodeAt(0) - 64) * 1000;
const normalizedLabel = secondFloorStartLabel.trim();
let secondFloorStart: number;
if (/^\d+$/.test(normalizedLabel)) {
// Numeric cutoff (supports legacy numeric table numbers)
secondFloorStart = Number(normalizedLabel);
} else if (/^[A-La-l]$/.test(normalizedLabel)) {
// Single row letter cutoff (A–L), converted to the comparable number space
const rowLetter = normalizedLabel.toUpperCase();
secondFloorStart = (rowLetter.charCodeAt(0) - 64) * 1000;
} else {
throw new Error(
`Invalid secondFloorStartLabel: "${secondFloorStartLabel}". Expected a numeric cutoff or a single row letter A-L.`
);
}

Copilot uses AI. Check for mistakes.
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.

Update table number to letter-number

3 participants