-
-
Notifications
You must be signed in to change notification settings - Fork 10
:docs: Improve contest labels (#1526) #1540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes involve enhancements to contest classification and label generation in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/test/lib/utils/test_cases/contest_type.ts (3)
95-103: Ensure consistent naming pattern for JOI qualifiersThe recent qualifiers use different naming patterns:
- Some use "qual 1[ABC]" (2024-2025)
- Others use "qual 2" (2020-2024)
Consider standardizing the naming pattern across all qualifiers for better maintainability.
111-116: Update Spring contest ID pattern for consistencyThe contest IDs for Spring contests changed from
joisctojoispstarting from 2023. This change in pattern should be documented with a comment explaining the transition.Add a comment above line 111:
+// Note: Contest ID pattern changed from joisc to joisp starting from 2023
91-126: Add section comments for better organizationConsider adding section comments to clearly separate different types of JOI contests. This would improve readability and maintenance.
Add section comments:
+// Historical JOI Qualifiers (2006-2007) { name: 'JOI 2006 qual', contestId: 'joi2006yo' }, ... +// Recent JOI Qualifiers (2024-2025) { name: 'JOI 2024 qual 1A', contestId: 'joi2024yo1a' }, ... +// JOI Finals (2006-2024) { name: 'JOI 2006 final', contestId: 'joi2006ho' }, ... +// JOI Spring Contests (2007-2024) { name: 'JOI 2007 Spring', contestId: 'joisc2007' }, ... +// JOI Open Contests (2022-2024) { name: 'JOI Open 2022', contestId: 'joiopen2022' }, ... +// JOIG Contests (2021-2024) { name: 'JOIG 2021 open', contestId: 'joig2021-open' },src/test/lib/utils/test_cases/contest_name_and_task_index.ts (1)
269-303: Consider removing commented-out codeThe code from lines 269 to 303 is commented out. If this code is no longer needed, consider removing it to keep the codebase clean and maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/lib/utils/contest.ts(5 hunks)src/test/lib/utils/contest.test.ts(1 hunks)src/test/lib/utils/test_cases/contest_name_and_task_index.ts(3 hunks)src/test/lib/utils/test_cases/contest_name_labels.ts(0 hunks)src/test/lib/utils/test_cases/contest_type.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/test/lib/utils/test_cases/contest_name_labels.ts
🔇 Additional comments (16)
src/test/lib/utils/test_cases/contest_type.ts (3)
120-126: Verify JOIG contest patterns
The JOIG contests use multiple patterns:
- Open contests:
joig{year}-open - Spring contests: Changed from
joigsctojoigsp
Consider adding a comment explaining these patterns and their transitions for better maintainability.
✅ Verification successful
Add a comment explaining JOIG contest ID patterns
The search results confirm that these are indeed all the JOIG contest patterns used in the codebase, and they follow two distinct formats:
- Open contests:
joig{year}-open(e.g., joig2021-open) - Spring contests: Transitioned from
joigsc{year}tojoigsp{year}(e.g., joigsc2022 → joigsp2023)
Add a comment above the JOIG contest entries explaining these patterns and the transition in Spring contest IDs for better maintainability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if these are all the existing JOIG contest patterns
rg -g '!*.{json,md}' 'joig(sc|sp|[0-9]{4})'
Length of output: 2236
104-110: Consider adding missing JOI finals
There are gaps in the JOI finals coverage:
- Between 2007 and 2019 (2008-2018 missing)
- Between 2020 and 2022 (2021 missing)
Consider adding the missing years for complete coverage.
✅ Verification successful
The gaps in JOI finals appear intentional
Looking at the codebase, we can see that:
- The regex pattern
regexForJoiincontest.tssupports all JOI contest formats - Only specific JOI finals (2006, 2007, 2019, 2020, 2022-2024) are included in the test cases
- These finals have corresponding task definitions in
contest_name_and_task_index.ts
The gaps likely represent finals that are not currently supported in the system, rather than missing entries that should be added.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any JOI final contests in the gaps
rg -g '!*.{json,md}' 'joi20(0[89]|1[0-8]|21)ho'
Length of output: 49
Script:
#!/bin/bash
# Let's check for any references to JOI contests in the codebase to understand the pattern
rg -i "joi.*ho" -g '!*.{json,md}' -A 2 -B 2
Length of output: 3361
91-94: Consider adding missing historical JOI qualifiers
The historical qualifiers have a gap between 2007 and 2018. Consider adding the missing years (2008-2017) for complete historical coverage.
src/test/lib/utils/contest.test.ts (2)
425-447: LGTM! Test blocks follow established patterns.
The new test blocks for "past" and "joi" contest IDs are well-structured and maintain consistency with existing test cases. They properly utilize the test helper functions and type annotations.
426-426: Verify test cases existence and coverage.
Let's ensure that the test cases for "past" and "joi" contests are properly defined in the test cases file.
Also applies to: 438-438
✅ Verification successful
Let me gather more information about the test data and test case generation to ensure completeness.
Let me check one more thing to verify how these test cases are being used in the test file.
Test cases are properly defined and integrated
The test cases for both "past" and "joi" contests are properly defined and used:
- Test data is defined in
PAST_TEST_DATAandJOI_TEST_DATAconstants - Test cases are generated using
Object.entries().flatMap()with appropriate generation functions - The test file correctly imports and uses these test cases in a consistent pattern along with other contest types
- The test structure follows the same pattern across all contest types, ensuring uniform testing coverage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test cases for past and joi contests exist
# Expected: Find definitions for past and joi test cases
# Check for past test cases
rg -A 5 "past.*=.*\[" src/test/lib/utils/test_cases/contest_name_and_task_index.ts
# Check for joi test cases
rg -A 5 "joi.*=.*\[" src/test/lib/utils/test_cases/contest_name_and_task_index.ts
Length of output: 760
Script:
#!/bin/bash
# Check the test data definitions
rg "PAST_TEST_DATA.*=.*{" -A 5 src/test/lib/utils/test_cases/contest_name_and_task_index.ts
rg "JOI_TEST_DATA.*=.*{" -A 5 src/test/lib/utils/test_cases/contest_name_and_task_index.ts
# Check the test case generation functions
ast-grep --pattern 'function generatePastTestCases($_) { $$$ }'
ast-grep --pattern 'function generateJoiTestCases($_) { $$$ }'
Length of output: 523
Script:
#!/bin/bash
# Check the test data definitions with fixed regex
rg "PAST_TEST_DATA\s*=" -A 5 src/test/lib/utils/test_cases/contest_name_and_task_index.ts
rg "JOI_TEST_DATA\s*=" -A 5 src/test/lib/utils/test_cases/contest_name_and_task_index.ts
# Check the test case generation functions with proper patterns
rg "function\s+generate(Past|Joi)TestCases" -A 10 src/test/lib/utils/test_cases/contest_name_and_task_index.ts
Length of output: 554
Script:
#!/bin/bash
# Check the test file structure and usage
rg "TestCasesForContestNameAndTaskIndex" -A 10 src/test/lib/utils/contest.test.ts
# Also check the test data structure
rg "export const (past|joi) =" -A 5 src/test/lib/utils/test_cases/contest_name_and_task_index.ts
Length of output: 6241
src/lib/utils/contest.ts (5)
305-307: Support for 'past' contest IDs implemented correctly
The addition of handling for contest IDs starting with 'past' by calling getPastContestLabel enhances the labeling of PAST contests.
313-315: Support for 'joi' contest IDs implemented correctly
The addition of handling for contest IDs starting with 'joi' by calling getJoiContestLabel improves the accuracy of contest labels for JOI contests.
335-335: Correct usage of getAojContestLabel for AOJ Courses
Using getAojContestLabel with AOJ_COURSES ensures consistent labeling for AOJ Courses contests.
339-339: Consistent labeling for PCK contests
The function getAojContestLabel is appropriately used with PCK_TRANSLATIONS to label PCK contests.
343-343: Consistent labeling for JAG contests
The function getAojContestLabel is used correctly with JAG_TRANSLATIONS to label JAG contests.
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (6)
2-9: Imports are correctly added for new functions and constants
The imports for getPastContestLabel, getJoiContestLabel, getAtCoderUniversityContestLabel, getAojContestLabel, PAST_TRANSLATIONS, and AOJ_COURSES are appropriate and necessary for the new test cases.
87-100: Function generatePastTestCases is correctly implemented
The generatePastTestCases function correctly generates test cases for PAST contests using the provided contest IDs and task indices.
151-164: Function generateJoiTestCases is correctly implemented
The generateJoiTestCases function correctly generates test cases for JOI contests using the provided contest IDs and task indices.
166-263: Verify consistency of task naming in JOI_TEST_DATA
In JOI_TEST_DATA, most tasks are labeled with single letters like 'A', 'B', 'C', but for joisc2007 and joisc2008, the tasks are named ['anagra', 'buildi', 'salt', 'score'] and ['belt', 'typhoon'].
Please verify that the task naming conventions are correct for these contests and consistent with how tasks are referenced elsewhere.
600-600: Enhanced expected value in AOJ Courses test cases
The expected field now dynamically includes the contest label using getAojContestLabel, which improves the accuracy of the test case expectations.
102-145:
Possible inconsistencies in PAST_TEST_DATA contestIds
There appear to be inconsistencies in the contestId values within PAST_TEST_DATA. For example:
- For
'past201912-open'(1st), thecontestIdis'past201904', which does not match the key or the comment indicating it's the 1st contest. - For
'past202112-open'(9th), thecontestIdis'past202203-open', which seems incorrect given the contest numbering.
Please verify that the contestIds correspond correctly to their respective keys and the intended contests.
KATO-Hiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (2)
87-100: Add input validation for task indicesThe
generatePastTestCasesfunction should validate task indices to prevent potential runtime errors.const generatePastTestCases = ( contestIds: string[], taskIndices: string[], ): { name: string; value: TestCaseForContestNameAndTaskIndex }[] => { + if (contestIds.length !== taskIndices.length) { + throw new Error('Contest IDs and task indices must have the same length'); + } return zip(contestIds, taskIndices).map(([contestId, taskIndex]) => {
166-263: Enhance type safety and organization of JOI test dataConsider improving the structure with proper TypeScript types and organizing contests by category.
+type JoiContestType = 'yo' | 'ho' | 'sc' | 'sp' | 'open' | 'gsc' | 'gsp'; +type JoiContestYear = `20${string}`; + +interface JoiTestData { + contestId: `joi${JoiContestYear}${JoiContestType}`; + tasks: string[]; +} + +const JOI_TEST_DATA: Record<string, JoiTestData> = { // Group by contest type for better organization // Preliminary contests joi2006yo: { contestId: 'joi2006yo', tasks: ['A', 'B', 'C', 'D', 'E'], }, // ... rest of the data };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/test/lib/utils/test_cases/contest_name_and_task_index.ts(3 hunks)
🔇 Additional comments (1)
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (1)
565-565: LGTM!
The modification to include the AOJ course label in the expected format is correct and consistent with the requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (7)
src/test/lib/utils/test_cases/contest_type.ts (2)
91-105: Consider reorganizing JOI qualifier test data for better maintainabilityThe JOI qualifier test data could be improved in the following ways:
- Maintain strict chronological ordering
- Consider adding missing years between 2007-2018 if they follow the same pattern
- Group qual 2 contests separately from qual 1 contests
// Historical JOI Qualifiers (2006-2007) { name: 'JOI 2006 qual', contestId: 'joi2006yo' }, { name: 'JOI 2007 qual', contestId: 'joi2007yo' }, + // ... (2008-2017 if applicable) { name: 'JOI 2018 qual', contestId: 'joi2018yo' }, { name: 'JOI 2019 qual', contestId: 'joi2019yo' }, + // JOI Qualifiers Round 2 + { name: 'JOI 2020 qual 2', contestId: 'joi2020yo2' }, + { name: 'JOI 2023 qual 2', contestId: 'joi2023yo2' }, + { name: 'JOI 2024 qual 2', contestId: 'joi2024yo2' }, // Recent JOI Qualifiers Round 1 (2024-) { name: 'JOI 2024 qual 1A', contestId: 'joi2024yo1a' }, { name: 'JOI 2024 qual 1B', contestId: 'joi2024yo1b' }, { name: 'JOI 2024 qual 1C', contestId: 'joi2024yo1c' },
114-120: Add a note about the contest ID pattern changeSimilar to the JOIG section, add a note about the pattern change from 'joisc' to 'joisp' starting from 2023 for better maintainability.
// JOI Spring Camp (2007-) + // Note: Contest ID pattern changed from joisc to joisp starting from 2023 { name: 'JOI 2007 Spring', contestId: 'joisc2007' }, { name: 'JOI 2008 Spring', contestId: 'joisc2008' },src/lib/utils/contest.ts (4)
438-438: Consider adding comprehensive unit tests for the JOI regex patternThe regex pattern is complex and handles multiple formats. To ensure reliability, consider adding unit tests that cover:
- Edge cases (empty strings, invalid formats)
- All valid combinations of optional components
- Invalid combinations that should be rejected
482-530: Add return type documentation for JOI helper functionsThe helper functions lack documentation for their return values. Consider adding
@returnsJSDoc tags to clarify the expected output format.Example for
addJoiSubTypeIfNeeds:+/** + * @param subType - The subtype of the JOI contest + * @returns The formatted subtype string or an empty string if not applicable + */ function addJoiSubTypeIfNeeds(subType: string): string {
Line range hint
584-593: Add validation for translations parameterConsider adding validation to ensure the translations parameter contains the expected keys and values.
export function getAojContestLabel( translations: Readonly<ContestLabelTranslations>, contestId: string, ): string { + if (!translations || Object.keys(translations).length === 0) { + throw new Error('Invalid translations: Must provide non-empty translations object'); + } let label = contestId;
543-545: Enhance error handling with more informative messagesConsider improving the error handling by:
- Including the expected format in the error message
- Using a custom error class for better error handling upstream
+class InvalidContestIdError extends Error { + constructor(contestId: string) { + super( + `Invalid university contest ID format: ${contestId}. ` + + 'Expected format: <ut|tt|tu>pc<YYYY> (e.g., utpc2023)' + ); + this.name = 'InvalidContestIdError'; + } +} if (!regexForAtCoderUniversity.test(contestId)) { - throw new Error(`Invalid university contest ID format: ${contestId}`); + throw new InvalidContestIdError(contestId); }src/test/lib/utils/test_cases/contest_name_and_task_index.ts (1)
566-566: Improve string concatenation readabilityConsider using a template literal for better readability when combining the label components.
- expected: `AOJ ${contestId}_${taskIndex}${getAojContestLabel(AOJ_COURSES, contestId)}`, + expected: `AOJ ${contestId}_${taskIndex} ${getAojContestLabel(AOJ_COURSES, contestId)}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/lib/utils/contest.ts(5 hunks)src/test/lib/utils/test_cases/contest_name_and_task_index.ts(3 hunks)src/test/lib/utils/test_cases/contest_type.ts(1 hunks)
🔇 Additional comments (4)
src/test/lib/utils/test_cases/contest_type.ts (3)
121-124: LGTM! Well-structured test data for JOI Open contests
The test data for JOI Open contests is well-organized with consistent naming patterns and complete sequence from its inception in 2022.
125-134: LGTM! Excellent organization and documentation
The JOIG section demonstrates best practices:
- Clear categorization of contest types
- Complete sequence from inception
- Well-documented pattern changes
- Consistent naming conventions
106-113: Verify completeness of JOI Finals test data
The test data for JOI Finals has significant gaps between 2007-2019. If these contests exist with the same 'ho' suffix pattern, consider adding them for comprehensive testing.
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (1)
151-268: Well-structured and documented JOI contest test cases
The implementation is comprehensive and includes:
- Clear separation of different JOI contest types
- Helpful documentation about contest ID pattern changes
- Consistent test data structure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/test/lib/utils/contest.test.ts (1)
550-570: Consider enhancing test coverage with positive casesThe error case testing is well-implemented, but consider adding positive test cases to ensure the function works correctly with valid university contest IDs. Also, the test implementation could be more concise using
test.each.Example refactor:
describe('get AtCoder university contest label', () => { + describe('expected to return correct label for valid format', () => { + test.each([ + ['utpc2019', 'UTPC 2019'], + ['ttpc2022', 'TTPC 2022'], + ])('when %s is given', (input, expected) => { + expect(getAtCoderUniversityContestLabel(input)).toBe(expected); + }); + }); describe('expected to be thrown an error if an invalid format is given', () => { - test('when utpc is given', () => { - expect(() => getAtCoderUniversityContestLabel('utpc24')).toThrow( - 'Invalid university contest ID format: utpc24', - ); - }); - - test('when ttpc is given', () => { - expect(() => getAtCoderUniversityContestLabel('ttpc')).toThrow( - 'Invalid university contest ID format: ttpc', - ); - }); - - test('when tupc is given', () => { - expect(() => getAtCoderUniversityContestLabel('tupc')).toThrow( - 'Invalid university contest ID format: tupc', - ); - }); + test.each(['utpc24', 'ttpc', 'tupc'])('when %s is given', (input) => { + expect(() => getAtCoderUniversityContestLabel(input)).toThrow( + `Invalid university contest ID format: ${input}`, + ); + }); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/test/lib/utils/contest.test.ts(3 hunks)
🔇 Additional comments (3)
src/test/lib/utils/contest.test.ts (3)
16-16: LGTM!
The new import follows the existing pattern and is correctly placed with other utility imports.
426-436: LGTM!
The test block for "past" contests follows the established pattern and properly validates the contest name to task index mapping.
438-448: LGTM!
The test block for "joi" contests follows the established pattern and properly validates the contest name to task index mapping.
KATO-Hiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary by CodeRabbit
New Features
Bug Fixes
Tests