:docs: Improve contest labels (#1526) #1528
Conversation
WalkthroughThe changes in this pull request enhance the contest classification and labeling functionality within the codebase. Notable modifications include the addition of new contest types, updates to regex patterns for better contest ID matching, and a refactor of functions to improve readability. The formatting of contest names and expected outputs in test cases has been standardized, ensuring consistency across various contest types. Additionally, translations for contest types have been adjusted for better presentation. Changes
Assessment against linked issues
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/test/lib/utils/test_cases/contest_name_labels.ts (1)
Test case update to ABC 380 appears problematic
The highest actual ABC contest ID in the codebase is ABC 331, with test cases typically using ABC 365. Changing the test case to ABC 380 seems premature as this contest doesn't exist yet. I recommend keeping the test case at ABC 365 or using a contest ID that's already in the system.
- Consider reverting the test case back to ABC 365
- If a higher contest number is needed, suggest using ABC 331 which is the latest actual contest in the codebase
🔗 Analysis chain
Line range hint
14-31: Verify the change of contest ID from 365 to 380While the spacing improvements align with the PR objectives, please confirm if changing the test case from ABC 365 to ABC 380 was intentional.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if ABC 380 exists or is planned rg -l "abc.*380" --type tsLength of output: 28
Script:
#!/bin/bash # Let's check for the highest ABC contest ID in the codebase rg -o "abc\d{3}" --type ts | sort -uLength of output: 4940
src/lib/utils/contest.ts (3)
278-279: Consider Making 'regexForAtCoderUniversity' Case-InsensitiveFor consistency with 'regexForAxc', consider adding the
iflag toregexForAtCoderUniversityto make it case-insensitive. This ensures that contest IDs with different cases are correctly matched.Apply this diff:
-const regexForAtCoderUniversity = /^(ut|tt|tu)(pc)(\d{4})/; +const regexForAtCoderUniversity = /^(ut|tt|tu)(pc)(\d{4})/i;
317-319: Optimize University Contest ID Matching Using RegexConsider using
regexForAtCoderUniversitydirectly in the conditional check for matching contest IDs, similar toregexForAxc, for consistency and potential performance improvement.Apply this diff:
-if (atCoderUniversityPrefixes.some((prefix) => contestId.startsWith(prefix))) { +if (regexForAtCoderUniversity.exec(contestId)) { return getAtCoderUniversityContestLabel(contestId); }
359-360: Simplify the Use of theSPACEConstantDefining a
SPACEconstant for a single space character may not be necessary and can reduce code readability. Consider using spaces directly in the strings.Apply this diff:
-const SPACE = ' ';And update the translations accordingly:
In
PCK_TRANSLATIONS:const PCK_TRANSLATIONS = { PCK: 'パソコン甲子園', - Prelim: SPACE + '予選' + SPACE, - Final: SPACE + '本選' + SPACE, + Prelim: ' 予選 ', + Final: ' 本選 ', };In
JAG_TRANSLATIONS:const JAG_TRANSLATIONS = { - Prelim: SPACE + '模擬国内' + SPACE, - Regional: SPACE + '模擬地区' + SPACE, + Prelim: ' 模擬国内 ', + Regional: ' 模擬地区 ', };
📜 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(7 hunks)src/test/lib/utils/test_cases/contest_name_labels.ts(7 hunks)
🔇 Additional comments (11)
src/test/lib/utils/test_cases/contest_name_labels.ts (4)
224-250: LGTM! Consistent spacing improvements
The spacing changes in AGC test cases align with the PR objectives and maintain consistency with other contest types.
Line range hint 259-284: LGTM! Well-documented changes
The spacing improvements are consistent, and the comment block provides clear context about the contest years.
337-369: LGTM! Consistent translation formatting
The PCK translations maintain consistent formatting with proper spacing and parentheses while preserving the Japanese text.
408-412: Verify JAG Prelim 2024 contest
Please confirm if JAG Prelim 2024 is already planned or announced.
✅ Verification successful
JAG Prelim 2024 appears to be a valid contest with assigned problem IDs
The test files show that JAG Prelim 2024 is properly integrated into the system with:
- Consistent naming pattern following previous years (2021-2023)
- Assigned problem IDs (3386, 3387, 3394) in task_url.ts
- Proper Japanese translation format matching other JAG contests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if JAG Prelim 2024 exists in other test files
rg -l "JAGPrelim2024" --type ts
Length of output: 172
Script:
#!/bin/bash
# Check the context of JAGPrelim2024 usage in test files
rg "JAGPrelim2024" --type ts -C 3
Length of output: 2209
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (6)
17-23: LGTM! Improved readability of ABC contest labels.
The spacing changes in both test names and expected outputs make the contest labels more readable and consistent.
166-172: LGTM! Consistent formatting with ABC contest labels.
The spacing changes align with the ABC format, maintaining consistency across different contest types.
188-194: LGTM! Consistent formatting with ABC/ARC contest labels.
The spacing changes maintain consistency across all AtCoder regular contest types (ABC/ARC/AGC).
435-435: LGTM! Improved PCK contest labels with proper Japanese terms.
The addition of proper Japanese terms (予選/本選) and parentheses improves readability and accuracy of the PCK contest labels.
537-537: LGTM! Improved JAG contest labels with proper Japanese terms.
The addition of proper Japanese terms (模擬国内/模擬地区) and parentheses improves readability and accuracy of the JAG contest labels, maintaining consistency with PCK contest formatting.
304-308: Verify the behavior of getAtCoderUniversityContestLabel function.
The use of a dedicated function for university contest labels improves maintainability. However, we should verify its behavior across all university contest types.
✅ Verification successful
Let me gather more information about the regex pattern used in the implementation.
Let me check the actual test cases to verify the behavior.
Function implementation and test coverage are appropriate
The getAtCoderUniversityContestLabel function is well-implemented with:
- A clear regex pattern
^(ut|tt|tu)(pc)(\d{4})that handles university contest IDs - Proper transformation of contest IDs (e.g., 'utpc2011' → 'UTPC 2011')
- Comprehensive test coverage across multiple university contest types
- Test cases including historical contests from 2011 to 2020
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of getAtCoderUniversityContestLabel
# Search for the function implementation
ast-grep --pattern 'function getAtCoderUniversityContestLabel($_) {
$$$
}'
# Search for its test cases
rg -A 5 'getAtCoderUniversityContestLabel'
Length of output: 2285
Script:
#!/bin/bash
# Search for the regex pattern definition
rg "regexForAtCoderUniversity" -B 2 -A 2
# Search for test cases or examples of university contest IDs
rg "university" --type ts
Length of output: 2001
Script:
#!/bin/bash
# Get the university contest IDs and their test cases
rg -A 5 "universityContestIds = \["
# Get the test cases for the function
rg -B 2 -A 5 "when contest_id matches contests held by university students" src/test/lib/utils/contest.test.ts
Length of output: 2005
src/lib/utils/contest.ts (1)
Line range hint 111-128: Addition of University Contests is Correct
The inclusion of 'UTPC', 'TTPC', and 'TUPC' into the university contests mapping is correctly implemented with appropriate documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/lib/utils/contest.ts (2)
Line range hint
193-242: Consider enhancing type safety for priority valuesWhile the priority system is well-documented, consider the following improvements:
- Define priority range constants
- Add type definition for the INF value
+ const MIN_PRIORITY = 0; + const MAX_PRIORITY = 20; + const PRIORITY_INF: number = 10 ** 3; /** - * Contest type priorities (0 = Highest, 20 = Lowest) + * Contest type priorities (${MIN_PRIORITY} = Highest, ${MAX_PRIORITY} = Lowest) */ export const contestTypePriorities: Map<ContestType, number> = new Map([ [ContestType.ABS, 0], // ... other entries [ContestType.AOJ_JAG, 20], ]); export function getContestPriority(contestId: string): number { const contestType = classifyContest(contestId); - const INF: number = 10 ** 3; if (contestType === null || contestType === undefined) { - return INF; + return PRIORITY_INF; } else { return contestTypePriorities.get(contestType) as number; } }
405-414: Consider making AOJ contest handling more maintainableThe current implementation might need updates as more AOJ contest types are added.
+ const AOJ_PREFIXES = ['PCK', 'JAG'] as const; + type AojPrefix = typeof AOJ_PREFIXES[number]; - function isAojContest(contestId: string): boolean { - return contestId.startsWith('PCK') || contestId.startsWith('JAG'); + function isAojContest(contestId: string): boolean { + return AOJ_PREFIXES.some(prefix => contestId.startsWith(prefix)); }src/test/lib/utils/test_cases/contest_name_and_task_index.ts (1)
Line range hint
203-207: Address TODO and HACK comments regarding university contest support.The comments indicate this is a pilot implementation limited to UTPC contests. Consider:
- Validating the current implementation
- Extending support for other university contests (e.g., ICPC regional contests)
Would you like me to help create a GitHub issue to track the remaining work for supporting additional university contests?
📜 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(8 hunks)src/test/lib/utils/test_cases/contest_name_labels.ts(7 hunks)
🔇 Additional comments (14)
src/lib/utils/contest.ts (3)
Line range hint 111-144: Well-structured university contest definitions with excellent documentation!
The implementation provides clear documentation about current contest types and future considerations. The JSDoc comments effectively explain the structure and maintenance guidelines.
243-279: Verify regex patterns with unit tests
The regex patterns are well-documented with examples, but should be verified with comprehensive unit tests.
Would you like me to generate comprehensive test cases for these regex patterns?
371-372: Verify the impact of adding spaces in translations
The addition of spaces around translations improves readability, but we should verify it doesn't affect existing usage.
Also applies to: 385-386
✅ Verification successful
Adding spaces around translations is safe and consistent with test expectations
The test cases in contest_name_labels.ts and contest_name_and_task_index.ts already expect spaces around the translations in the output. The test cases verify that:
- PCK translations:
（パソコン甲子園 予選 2023）,（パソコン甲子園 本選 2023） - JAG translations:
（JAG 模擬国内 2023）,（JAG 模擬地区 2023）
The addition of spaces in the translations improves readability and matches the expected format in all test cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of translations in the codebase
# Search for usage of translations
rg -A 5 "PCK_TRANSLATIONS|JAG_TRANSLATIONS"
# Look for test cases involving these translations
fd -e ts -e js "test" -x rg -l "PCK_TRANSLATIONS|JAG_TRANSLATIONS" {}
Length of output: 1097
Script:
#!/bin/bash
# Let's check how these translations are used in the getAojChallengeLabel function
ast-grep --pattern 'function getAojChallengeLabel($_) {
$$$
}'
# Also search for any direct usage of these translation keys
rg -B 2 -A 2 "[^A-Za-z](Prelim|Final|Regional)[^A-Za-z]"
Length of output: 36325
Script:
#!/bin/bash
# Let's check how the getAojChallengeLabel function uses these translations
ast-grep --pattern 'function getAojChallengeLabel(translations, contestId) {
$$$
}'
# Also check if there are any direct string matches for the translations without spaces
rg -B 2 -A 2 "予選|本選|模擬国内|模擬地区"
Length of output: 20609
src/test/lib/utils/test_cases/contest_name_labels.ts (6)
14-16: LGTM! Improved readability with consistent spacing
The addition of spaces in contest names and the update to include newer contests enhance the test coverage.
Also applies to: 31-31
173-219: LGTM! Consistent formatting and valid contest additions
The changes maintain consistent spacing in contest names and include valid test cases for ARC 187 and ARC 188, which have been confirmed as already held contests.
224-250: LGTM! Consistent spacing in AGC contest names
The changes maintain the consistent spacing pattern established across other contest types.
Line range hint 259-284: LGTM! Well-documented university contest test cases
The changes maintain consistent spacing and are accompanied by helpful documentation about contest years and availability.
335-369: LGTM! Improved Japanese contest name formatting
The changes enhance readability by adding proper spacing and consistent parentheses around Japanese contest names.
376-452: LGTM! Consistent Japanese contest name formatting
The changes maintain the same improved formatting pattern established in the PCK test cases.
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (5)
2-2: LGTM!
The import of getAtCoderUniversityContestLabel aligns with the PR's objective to improve contest label visibility.
304-308: LGTM! Consistent label formatting using utility function.
Using getAtCoderUniversityContestLabel ensures consistent formatting of university contest labels.
386-417: LGTM! Comprehensive test data coverage for PCK contests.
The test data covers both preliminary (予選) and final (本選) rounds across multiple years (2003-2023).
435-435: LGTM! Improved formatting for contest labels with Japanese text.
The changes enhance readability by:
- Adding proper spacing around Japanese text
- Using parentheses to group contest information
- Maintaining consistent formatting between PCK and JAG contests
Let's verify the consistency of Japanese text formatting:
Also applies to: 537-537
✅ Verification successful
Japanese text formatting is consistently applied across contest labels
The verification confirms that Japanese text formatting follows a consistent pattern throughout the codebase:
- All contest labels use full-width parentheses （）
- Proper spacing is maintained between English/numbers and Japanese text
- Consistent formatting for both PCK (パソコン甲子園) and JAG contest labels
- Uniform spacing around contest types (予選, 本選, 模擬国内, 模擬地区)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent Japanese text formatting in contest labels
# Expected: Japanese text should be properly spaced and wrapped in parentheses
# Check for Japanese text formatting patterns
rg -n '（[^）]*[予選|本選|模擬国内|模擬地区][^）]*）' src/test/lib/utils/test_cases/
Length of output: 3037
17-23: LGTM! Consistent formatting improvements across contest generators.
The changes add proper spacing between contest type and ID (e.g., ABC 001 instead of ABC001), improving readability. The pattern is consistently applied across ABC, ARC, and AGC generators.
Let's verify the consistency of the formatting pattern:
Also applies to: 166-172, 188-194
✅ Verification successful
Consistent formatting pattern verified across all contest types
Based on the search results, the spacing pattern is consistently applied across all contest types:
- ABC format:
ABC ${contestId} - ${taskIndex} - ARC format:
ARC ${contestId} - ${taskIndex} - AGC format:
AGC ${contestId} - ${taskIndex}
All contest labels maintain proper spacing between contest type and ID, and use a consistent hyphen separator for task indices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent spacing in contest label formatting
# Expected: All contest generators should follow the same pattern with space between contest type and ID
# Check for inconsistent spacing patterns in contest label formatting
rg -n 'expected: [`"](?:\w+) ?\d' src/test/lib/utils/test_cases/
Length of output: 66
Script:
#!/bin/bash
# Let's try a more comprehensive search for contest formatting patterns
# Search for test case definitions with expected values
# Search for all expected contest name patterns
rg -n 'expected:.*(?:ABC|ARC|AGC).*\d' src/test/lib/utils/test_cases/
# Also search for actual test case definitions to see the formatting
ast-grep --pattern 'createTestCaseForContestNameAndTaskIndex($$$)({
$$$
expected: $_,
$$$
})'
Length of output: 17902
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (2)
442-442: Fix inconsistent spacing in template literal.The spacing in the replacement strings is inconsistent. Consider standardizing the spacing.
- expected: `AOJ ${taskIndex}（パソコン甲子園${contestId.replace('Prelim', ' 予選 ').replace('Final', ' 本選 ')}）`, + expected: `AOJ ${taskIndex}（パソコン甲子園${contestId.replace('Prelim', '予選').replace('Final', '本選')}）`,
544-544: Fix inconsistent spacing in template literal.Similar to the PCK test cases, standardize the spacing in the replacement strings.
- `AOJ ${taskIndex}（JAG${contestId.replace('Prelim', ' 模擬国内 ').replace('Regional', ' 模擬地区 ')}）`, + `AOJ ${taskIndex}（JAG${contestId.replace('Prelim', '模擬国内').replace('Regional', '模擬地区')}）`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/test/lib/utils/contest.test.ts(0 hunks)src/test/lib/utils/test_cases/contest_name_and_task_index.ts(9 hunks)src/test/lib/utils/test_cases/contest_name_labels.ts(0 hunks)
💤 Files with no reviewable changes (2)
- src/test/lib/utils/contest.test.ts
- src/test/lib/utils/test_cases/contest_name_labels.ts
🔇 Additional comments (4)
src/test/lib/utils/test_cases/contest_name_and_task_index.ts (4)
2-2: LGTM! Import aligns with the PR objectives.
The addition of getAtCoderUniversityContestLabel import supports the goal of improving contest label visibility and consistency.
17-23: LGTM! Consistent formatting improvements across contest generators.
The changes add a space between contest type and ID (e.g., ABC 001 instead of ABC001) in ABC, ARC, and AGC test case generators, improving readability and maintaining consistency.
Also applies to: 166-172, 188-194
224-230: LGTM! Well-documented university contest information.
The added documentation provides clear context about contest availability and includes helpful references, improving maintainability.
401-407: LGTM! Added support for recent PCK contests.
The changes add test data for 2022-2023 PCK contests, keeping the test suite up-to-date.
Also applies to: 417-423
KATO-Hiro
changed the title
|
修正範囲は広いですが、それぞれ影響範囲は狭いため少しずつマージしていく方針です。 |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation