Conversation
- Implemented `LIST_ALL_UNICODE_VERSIONS_ROUTE` to list all Unicode versions with metadata. - Created `GET_UNICODE_MAPPINGS` and `GET_UNICODE_VERSION_METADATA` routes for fetching mappings and metadata. - Defined schemas for Unicode version data using Zod.
* Introduced a new endpoint description for accessing information about Unicode versions. * This includes metadata retrieval for version numbers, release dates, and available data files. * Added details for listing all available Unicode versions and retrieving specific version metadata.
|
Warning Rate limit exceeded@luxass has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis change introduces a new API router for Unicode version information under Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Server
participant UnicodeOrg
Client->>API_Server: GET /api/v1/versions/releases
API_Server->>UnicodeOrg: Fetch Unicode versions HTML
UnicodeOrg-->>API_Server: Unicode versions HTML
API_Server->>API_Server: Parse, map, and structure version data
API_Server-->>Client: JSON array of Unicode versions
Client->>API_Server: GET /api/v1/versions/mappings
API_Server-->>Client: JSON object of version mappings
Client->>API_Server: GET /api/v1/versions/metadata
API_Server-->>Client: JSON object of version metadata
Possibly related issues
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Pull Request Overview
This PR adds a new set of API endpoints for retrieving Unicode version information and mappings.
- Introduces
/api/v1/versionsrouter with/releases,/mappings, and/metadataendpoints - Defines Zod schemas for version objects, mappings, and metadata
- Adds OpenAPI route definitions and updates global OpenAPI config with a "Versions" tag
- Registers the new router in the application entrypoint
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/src/routes/v1_versions.ts | Added version listing, mappings, and metadata endpoints |
| apps/api/src/routes/v1_versions.schemas.ts | Defined Zod schemas for version data |
| apps/api/src/routes/v1_versions.openapi.ts | Added OpenAPI route definitions for the new endpoints |
| apps/api/src/openapi.ts | Updated OpenAPI config with a "Versions" tag |
| apps/api/src/index.ts | Registered V1_VERSIONS_ROUTER in the main app |
Comments suppressed due to low confidence (1)
apps/api/src/routes/v1_versions.ts:1
- There are no tests covering the new
/releases,/mappings, and/metadataendpoints. Please add unit or integration tests to ensure these routes behave as expected.
import type { HonoEnv } from "../types";
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
Preview DeploymentsThe workers have been deployed successfully. API Preview URL: https://preview.api.ucdjs.dev This preview was built from commit 618fbac 🤖 This comment will be updated automatically when you push new commits to this PR. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/src/routes/v1_versions.ts (1)
16-113: Optimize regex compilation for better performance.The implementation is solid with good error handling and comprehensive version parsing. However, consider moving the regex compilation outside the loop for better performance.
+ // Compile regex outside the loop for better performance + const versionExtractRegex = new RegExp(`<a[^>]+href="([^"]+)"[^>]*>\\s*(${versionPattern.source})\\s*</a>`); + for (const row of rows) { // look for Unicode version pattern in the row - const versionMatch = row.match(new RegExp(`<a[^>]+href="([^"]+)"[^>]*>\\s*(${versionPattern.source})\\s*</a>`)); + const versionMatch = row.match(versionExtractRegex); if (!versionMatch) continue;The current non-null assertions are safe due to the preceding null checks, and the overall logic for handling draft versions and status determination is well-implemented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/src/index.ts(1 hunks)apps/api/src/openapi.ts(1 hunks)apps/api/src/routes/v1_versions.openapi.ts(1 hunks)apps/api/src/routes/v1_versions.schemas.ts(1 hunks)apps/api/src/routes/v1_versions.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: luxass
PR: ucdjs/ucd#20
File: packages/ucd-store/test/index.test.ts:1-7
Timestamp: 2025-05-31T07:44:02.123Z
Learning: User luxass prefers to implement functionality in separate PRs rather than including everything in a single large PR, keeping changes focused and manageable.
🧬 Code Graph Analysis (1)
apps/api/src/routes/v1_versions.openapi.ts (1)
apps/api/src/routes/v1_versions.schemas.ts (3)
UnicodeVersionSchema(3-26)UnicodeVersionMappingsSchema(30-33)UnicodeVersionMetadataSchema(35-38)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: deploy (api)
- GitHub Check: deploy (proxy)
- GitHub Check: build
🔇 Additional comments (6)
apps/api/src/index.ts (1)
11-11: LGTM! Clean integration of the new versions router.The import and route registration follow the established pattern and properly integrate the new Unicode versions API endpoints into the main routing structure.
Also applies to: 18-18
apps/api/src/openapi.ts (1)
95-106: LGTM! Well-documented API tag addition.The new "Versions" tag provides comprehensive documentation for the Unicode version endpoints and follows the established pattern of other OpenAPI tags.
apps/api/src/routes/v1_versions.schemas.ts (1)
1-38: LGTM! Comprehensive schema definitions with proper validation.The Zod schemas are well-structured with:
- Clear field descriptions for OpenAPI documentation
- Appropriate validation (4-digit year regex, URL validation)
- Proper nullable handling for the date field
- Constrained status values to prevent invalid states
- Clean TypeScript type inference
The schemas provide a solid foundation for the API endpoints.
apps/api/src/routes/v1_versions.openapi.ts (1)
1-94: LGTM! Comprehensive OpenAPI route definitions.The route definitions are well-structured with:
- Clear descriptions for each endpoint
- Appropriate HTTP methods and paths
- Proper error response handling with relevant status codes
- Consistent use of the defined schemas
- Good API documentation that will generate helpful OpenAPI docs
The varying error response coverage (more for the complex scraping endpoint, fewer for the simple data endpoints) is appropriate.
apps/api/src/routes/v1_versions.ts (2)
9-14: LGTM! Proper router setup with appropriate caching.The router configuration with 4-day caching is well-suited for Unicode version data, which changes infrequently. The base path structure is clean and follows REST conventions.
115-121: LGTM! Clean implementation of mapping and metadata endpoints.The simple endpoints for mappings and metadata are well-implemented, leveraging the external utility library appropriately.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/api/src/routes/v1_versions.ts (1)
28-31: HTML parsing with regex is brittle and may break with page structure changes.The current approach uses regex to parse HTML tables, which can fail if the Unicode.org page structure changes. Consider using a proper HTML parser for more reliable extraction.
Consider using a library like
cheerioorjsdomfor more robust HTML parsing:- const versionPattern = /Unicode \d+\.\d+\.\d+/; - const tableMatch = html.match(/<table[^>]*>[\s\S]*?<\/table>/g)?.find((table) => - versionPattern.test(table), - ); + // Use a proper HTML parser instead of regex + const $ = cheerio.load(html); + const versionPattern = /Unicode \d+\.\d+\.\d+/; + const tableMatch = $('table').filter((_, table) => + versionPattern.test($(table).text()) + ).first().html();
🧹 Nitpick comments (3)
apps/api/src/routes/v1_versions.ts (3)
73-74: Remove unnecessary non-null assertion.The
documentationUrlvariable is already guaranteed to be non-null from the earlier check.- documentationUrl: documentationUrl!, - date: dateMatch[1]!, + documentationUrl: documentationUrl, + date: dateMatch[1],
110-111: Add more specific error logging for debugging.The generic error logging could be more informative for troubleshooting production issues.
- console.error("Error fetching Unicode versions:", error); + console.error("Error fetching Unicode versions:", { + error: error instanceof Error ? error.message : error, + stack: error instanceof Error ? error.stack : undefined + });
16-113: Consider adding unit tests for the main endpoint.Given the complexity of the HTML parsing and version processing logic, comprehensive unit tests would help ensure reliability and catch regressions.
The static analysis shows low test coverage for this new functionality. Consider adding unit tests that mock the HTML response to verify:
- Correct parsing of version information
- Proper handling of draft versions
- Status determination logic
- Error handling for malformed HTML
Would you like me to help generate unit tests for this endpoint?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/routes/v1_versions.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: luxass
PR: ucdjs/ucd#20
File: packages/ucd-store/test/index.test.ts:1-7
Timestamp: 2025-05-31T07:44:02.123Z
Learning: User luxass prefers to implement functionality in separate PRs rather than including everything in a single large PR, keeping changes focused and manageable.
🪛 GitHub Check: codecov/patch
apps/api/src/routes/v1_versions.ts
[warning] 17-18: apps/api/src/routes/v1_versions.ts#L17-L18
Added lines #L17 - L18 were not covered by tests
[warning] 20-20: apps/api/src/routes/v1_versions.ts#L20
Added line #L20 was not covered by tests
[warning] 25-25: apps/api/src/routes/v1_versions.ts#L25
Added line #L25 was not covered by tests
[warning] 28-30: apps/api/src/routes/v1_versions.ts#L28-L30
Added lines #L28 - L30 were not covered by tests
[warning] 34-34: apps/api/src/routes/v1_versions.ts#L34
Added line #L34 was not covered by tests
[warning] 39-39: apps/api/src/routes/v1_versions.ts#L39
Added line #L39 was not covered by tests
[warning] 42-42: apps/api/src/routes/v1_versions.ts#L42
Added line #L42 was not covered by tests
[warning] 44-44: apps/api/src/routes/v1_versions.ts#L44
Added line #L44 was not covered by tests
[warning] 49-49: apps/api/src/routes/v1_versions.ts#L49
Added line #L49 was not covered by tests
[warning] 51-51: apps/api/src/routes/v1_versions.ts#L51
Added line #L51 was not covered by tests
[warning] 54-55: apps/api/src/routes/v1_versions.ts#L54-L55
Added lines #L54 - L55 were not covered by tests
[warning] 58-58: apps/api/src/routes/v1_versions.ts#L58
Added line #L58 was not covered by tests
[warning] 60-60: apps/api/src/routes/v1_versions.ts#L60
Added line #L60 was not covered by tests
[warning] 64-64: apps/api/src/routes/v1_versions.ts#L64
Added line #L64 was not covered by tests
[warning] 66-66: apps/api/src/routes/v1_versions.ts#L66
Added line #L66 was not covered by tests
[warning] 68-68: apps/api/src/routes/v1_versions.ts#L68
Added line #L68 was not covered by tests
[warning] 71-71: apps/api/src/routes/v1_versions.ts#L71
Added line #L71 was not covered by tests
[warning] 82-82: apps/api/src/routes/v1_versions.ts#L82
Added line #L82 was not covered by tests
[warning] 93-93: apps/api/src/routes/v1_versions.ts#L93
Added line #L93 was not covered by tests
[warning] 99-99: apps/api/src/routes/v1_versions.ts#L99
Added line #L99 was not covered by tests
[warning] 105-105: apps/api/src/routes/v1_versions.ts#L105
Added line #L105 was not covered by tests
[warning] 108-108: apps/api/src/routes/v1_versions.ts#L108
Added line #L108 was not covered by tests
[warning] 110-111: apps/api/src/routes/v1_versions.ts#L110-L111
Added lines #L110 - L111 were not covered by tests
[warning] 116-116: apps/api/src/routes/v1_versions.ts#L116
Added line #L116 was not covered by tests
[warning] 120-120: apps/api/src/routes/v1_versions.ts#L120
Added line #L120 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: update-pr-comment
🔇 Additional comments (4)
apps/api/src/routes/v1_versions.ts (4)
1-15: Router setup looks good with appropriate caching strategy.The imports, router configuration, and caching middleware are well-structured. The 4-day cache duration is reasonable for Unicode version data that changes infrequently.
99-106: Semantic version sorting logic is correct.The sorting implementation properly handles semantic versioning by comparing major, minor, and patch versions in descending order.
115-117: Simple mapping endpoint looks good.The implementation is straightforward and correctly returns the constant mapping data.
119-121: Simple metadata endpoint looks good.The implementation is straightforward and correctly returns the constant metadata.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…th-enhanced-unicode-version-support
- Introduced a new route to retrieve currently supported Unicode versions. - Updated the OpenAPI schema to reflect the changes. - Removed the `supported` field from the `UnicodeVersionSchema` as it is no longer needed.
* Removed `GET_UNICODE_VERSION_METADATA` and `GET_SUPPORTED_VERSIONS_ROUTE` endpoints. * Updated imports in `v1_versions.ts` to reflect the removal of these routes.
* Reformatted import statements in `v1_versions.ts` for improved readability. * Removed unnecessary line breaks to streamline the code structure.
luxass
deleted the
100-featapi-new-v1_versions-api-endpoints-with-enhanced-unicode-version-support
branch
1 participant
resolves #100
Summary by CodeRabbit
New Features
Documentation