-
-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(env): rename custom ucd headers #435
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
Updated the header constant to better reflect its purpose and ensure consistency across the codebase. Adjusted all relevant imports and usages in the API routes and tests.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRenamed public header constant Changes
(*Tests grouped where path prefixes match; see repo for exact test file list.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{test,spec}.?(c|m)[jt]s?(x)📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (7)📚 Learning: 2025-12-27T11:08:54.630ZApplied to files:
📚 Learning: 2025-06-09T05:10:32.105ZApplied to files:
📚 Learning: 2025-10-14T07:15:35.199ZApplied to files:
📚 Learning: 2025-12-27T11:08:54.630ZApplied to files:
📚 Learning: 2025-08-23T05:20:36.940ZApplied to files:
📚 Learning: 2025-08-23T05:20:36.940ZApplied to files:
📚 Learning: 2025-08-23T05:16:26.866ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (4)
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. Comment |
🌏 Preview DeploymentsNote No deployable apps affected by changes in this PR. Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Pull request overview
This PR refactors custom UCD headers to follow a more standard naming convention, changing from UCD-File-Stat-Type to the X-UCD-Stat-* pattern. It also introduces four new header constants that will be used in future API rewrites.
- Renames
UCD_FILE_STAT_TYPE_HEADERtoUCD_STAT_TYPE_HEADERwith valueX-UCD-Stat-Type - Adds four new header constants:
UCD_STAT_SIZE_HEADER,UCD_STAT_CHILDREN_HEADER,UCD_STAT_CHILDREN_FILES_HEADER, andUCD_STAT_CHILDREN_DIRS_HEADER - Updates all references to the renamed header across the API implementation and tests
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/env/src/constants.ts |
Renames header constant and adds four new header constants with standardized X-UCD-Stat-* naming |
packages/env/src/index.ts |
Exports all five header constants (one renamed, four new) |
apps/api/src/routes/v1_files/$wildcard.ts |
Updates import and all usage references from old header name to new UCD_STAT_TYPE_HEADER |
apps/api/test/routes/v1_files/$wildcard.test.ts |
Updates test to use new header constant name |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/routes/v1_files/$wildcard.ts (1)
126-133: Runpnpm build:openapito regenerate the OpenAPI specification.The OpenAPI route definitions correctly use the
UCD_STAT_TYPE_HEADERconstant in the header schemas for both GET and HEAD routes. Since these are changes to API routes in this file, regenerate the OpenAPI spec per the build process.
🧹 Nitpick comments (1)
packages/env/src/constants.ts (1)
1-14: Update JSDoc to reflect multiple header constants.The JSDoc comment refers to a singular "HTTP header name" and "this constant," but the file now exports five distinct header constants. Consider either:
- Converting this to a general module-level comment about UCD stat headers, or
- Moving individual JSDoc comments to each constant definition
🔎 Suggested JSDoc improvements
Option 1: Module-level documentation
/** - * HTTP header name for specifying the type of UCD statistics. + * HTTP header names for UCD statistics. * - * This constant defines the header key used to indicate the type of - * statistics being transmitted or requested in UCD-related HTTP communications. + * These constants define the header keys used for UCD statistics metadata + * in HTTP responses, including resource type, size, and directory child counts. * * @example * ```typescript * import { UCD_STAT_TYPE_HEADER, UCD_STAT_SIZE_HEADER } from "@ucdjs/env"; * * headers[UCD_STAT_TYPE_HEADER] = "file"; * headers[UCD_STAT_SIZE_HEADER] = "1024"; * ``` */Option 2: Individual constant documentation
-/** - * HTTP header name for specifying the type of UCD statistics. - * - * This constant defines the header key used to indicate the type of - * statistics being transmitted or requested in UCD-related HTTP communications. - * - * @example - * ```typescript - * import { UCD_STAT_TYPE_HEADER, UCD_STAT_SIZE_HEADER } from "@ucdjs/env"; - * - * headers[UCD_STAT_TYPE_HEADER] = "file"; - * headers[UCD_STAT_SIZE_HEADER] = "1024"; - * ``` - */ - +/** Header indicating whether the resource is a "file" or "directory". */ export const UCD_STAT_TYPE_HEADER = "X-UCD-Stat-Type"; +/** Header indicating the size of the resource in bytes. */ export const UCD_STAT_SIZE_HEADER = "X-UCD-Stat-Size"; +/** Header indicating the total number of children in a directory. */ export const UCD_STAT_CHILDREN_HEADER = "X-UCD-Stat-Children"; +/** Header indicating the number of file children in a directory. */ export const UCD_STAT_CHILDREN_FILES_HEADER = "X-UCD-Stat-Children-Files"; +/** Header indicating the number of directory children in a directory. */ export const UCD_STAT_CHILDREN_DIRS_HEADER = "X-UCD-Stat-Children-Dirs";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/src/routes/v1_files/$wildcard.tsapps/api/test/routes/v1_files/$wildcard.test.tspackages/env/src/constants.tspackages/env/src/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Regenerate OpenAPI spec by running 'pnpm build:openapi' after changing API routes or Zod schemas
Files:
apps/api/src/routes/v1_files/$wildcard.ts
apps/api/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/api/src/**/*.ts: Build API (apps/api) using Hono web framework with OpenAPI plugin (@hono/zod-openapi) and organize routes by version and type
Use Zod schemas for defining API route schemas and request/response types in the API application
Files:
apps/api/src/routes/v1_files/$wildcard.ts
apps/api/src/routes/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Document API endpoints with OpenAPI annotations using @hono/zod-openapi decorators
Files:
apps/api/src/routes/v1_files/$wildcard.ts
**/*.{test,spec}.?(c|m)[jt]s?(x)
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.?(c|m)[jt]s?(x): Import test utilities from #test-utils/* path aliases instead of @ucdjs/test-utils in test files to avoid build step requirements and prevent cyclic dependencies
Use testdir() from vitest-testdirs to create temporary test directories in filesystem tests
Use mockStoreApi() utility from #test-utils/mock-store for MSW-based mocking of the UCD API in test files
Files:
apps/api/test/routes/v1_files/$wildcard.test.ts
apps/api/test/**/*.{test,spec}.?(c|m)[jt]s?(x)
📄 CodeRabbit inference engine (AGENTS.md)
Place API app tests in apps/api/test/routes/** for endpoint tests and apps/api/test/unit/** for unit tests with mocks
Files:
apps/api/test/routes/v1_files/$wildcard.test.ts
🧠 Learnings (10)
📚 Learning: 2025-07-20T05:37:40.565Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 131
File: tooling/eslint-plugin/package.json:0-0
Timestamp: 2025-07-20T05:37:40.565Z
Learning: In the ucdjs/ucd project, internal tooling packages (private packages in the tooling/ directory) export TypeScript files directly without requiring a build step, unlike published packages which use tsdown to build and export from ./dist/. Examples include ucdjs/tsdown-config and ucdjs/eslint-plugin.
Applied to files:
packages/env/src/index.ts
📚 Learning: 2025-06-09T05:10:32.105Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.105Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
Applied to files:
apps/api/src/routes/v1_files/$wildcard.tsapps/api/test/routes/v1_files/$wildcard.test.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/routes/**/*.ts : Document API endpoints with OpenAPI annotations using hono/zod-openapi decorators
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.ts : Use Zod schemas for defining API route schemas and request/response types in the API application
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.ts : Build API (apps/api) using Hono web framework with OpenAPI plugin (hono/zod-openapi) and organize routes by version and type
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.{ts,tsx} : Regenerate OpenAPI spec by running 'pnpm build:openapi' after changing API routes or Zod schemas
Applied to files:
apps/api/src/routes/v1_files/$wildcard.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Import test utilities from #test-utils/* path aliases instead of ucdjs/test-utils in test files to avoid build step requirements and prevent cyclic dependencies
Applied to files:
apps/api/test/routes/v1_files/$wildcard.test.ts
📚 Learning: 2025-10-14T07:15:35.199Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 326
File: packages/shared/src/ucd-config.ts:34-58
Timestamp: 2025-10-14T07:15:35.199Z
Learning: In packages/shared/src/ucd-config.ts, the `getDefaultUCDEndpointConfig` function uses a build-time define `__UCD_ENDPOINT_DEFAULT_CONFIG__` that is replaced at build time by the bundler. The `??` operator is evaluated at build time, so the final code contains no runtime branching. Therefore, runtime unit tests for this function are not valuable—the build process itself handles the injection and fallback logic.
Applied to files:
apps/api/test/routes/v1_files/$wildcard.test.ts
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to **/*.{test,spec}.?(c|m)[jt]s?(x) : Use mockStoreApi() utility from #test-utils/mock-store for MSW-based mocking of the UCD API in test files
Applied to files:
apps/api/test/routes/v1_files/$wildcard.test.ts
📚 Learning: 2025-08-23T05:20:36.940Z
Learnt from: luxass
Repo: ucdjs/ucd PR: 209
File: packages/ucd-store/test/clean.test.ts:9-9
Timestamp: 2025-08-23T05:20:36.940Z
Learning: In the ucdjs/ucd codebase using modern Vitest, the assert export from "vitest" works as a callable function assert(condition, message) and provides TypeScript type narrowing, making it the preferred choice over expect() for runtime assertions that need compile-time type safety. The existing usage pattern assert(condition, message) throughout the test suite is working correctly.
Applied to files:
apps/api/test/routes/v1_files/$wildcard.test.ts
🧬 Code graph analysis (3)
apps/api/src/routes/v1_files/$wildcard.ts (2)
packages/env/src/constants.ts (1)
UCD_STAT_TYPE_HEADER(16-16)packages/env/src/index.ts (1)
UCD_STAT_TYPE_HEADER(6-6)
packages/env/src/constants.ts (1)
packages/env/src/index.ts (5)
UCD_STAT_TYPE_HEADER(6-6)UCD_STAT_SIZE_HEADER(5-5)UCD_STAT_CHILDREN_HEADER(4-4)UCD_STAT_CHILDREN_FILES_HEADER(3-3)UCD_STAT_CHILDREN_DIRS_HEADER(2-2)
apps/api/test/routes/v1_files/$wildcard.test.ts (2)
packages/env/src/constants.ts (1)
UCD_STAT_TYPE_HEADER(16-16)packages/env/src/index.ts (1)
UCD_STAT_TYPE_HEADER(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: typecheck
- GitHub Check: lint
- GitHub Check: Agent
- GitHub Check: Greptile Review
- GitHub Check: test (windows-latest)
- GitHub Check: test (macos-latest)
🔇 Additional comments (6)
apps/api/test/routes/v1_files/$wildcard.test.ts (2)
2-2: LGTM! Import updated correctly.The import has been properly updated to use the renamed constant
UCD_STAT_TYPE_HEADER.
478-478: LGTM! Header usage updated correctly.The test correctly uses the renamed header constant to verify the response header value.
apps/api/src/routes/v1_files/$wildcard.ts (2)
6-6: LGTM! Import updated correctly.The import statement has been properly updated to use the renamed constant.
347-347: LGTM! Runtime header usage updated correctly.The response headers are correctly set using the renamed constant, with values matching the OpenAPI schema definitions.
Also applies to: 362-362
packages/env/src/index.ts (1)
1-7: LGTM! Public exports updated correctly.The module now correctly exports the five new header constants and removes the deprecated
UCD_FILE_STAT_TYPE_HEADER. The new headers are well-prepared for future API usage as mentioned in the PR objectives.packages/env/src/constants.ts (1)
16-20: LGTM! Header constants defined correctly.The five new header constants follow a consistent naming pattern and HTTP custom header conventions. The constant names clearly describe their purpose, and the
X-UCD-Stat-*header names provide good semantic meaning.
Greptile SummaryThis PR renames custom UCD headers to follow standard Key Changes
Critical Issues FoundThe refactor is incomplete - several files still reference the old constant name:
These will cause runtime failures when the CLI tries to read file metadata or when the web app fetches files. Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Client/CLI
participant API as API (/api/v1/files)
participant Env as @ucdjs/env
participant Unicode as Unicode.org
Note over Env: Constants defined:<br/>X-UCD-Stat-Type (renamed)<br/>X-UCD-Stat-Size (new)<br/>X-UCD-Stat-Children (new)<br/>X-UCD-Stat-Children-Files (new)<br/>X-UCD-Stat-Children-Dirs (new)
Client->>API: GET /api/v1/files/{path}
activate API
API->>Env: Import UCD_STAT_TYPE_HEADER
Env-->>API: "X-UCD-Stat-Type"
API->>Unicode: Fetch file/directory
activate Unicode
Unicode-->>API: File content or directory listing
deactivate Unicode
API->>API: Set response headers
Note over API: headers[UCD_STAT_TYPE_HEADER]<br/>= "file" or "directory"
API-->>Client: Response with X-UCD-Stat-Type header
deactivate API
Client->>Client: Read X-UCD-Stat-Type header
Note over Client: Determines if response is<br/>file content or directory listing
|
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.
Additional Comments (9)
-
packages/cli/src/cmd/files/info.ts, line 4 (link)syntax: breaking change:
UCD_FILE_STAT_TYPE_HEADERrenamed toUCD_STAT_TYPE_HEADERin@ucdjs/envThis file imports the old name which no longer exists after the refactor. Update to:
-
packages/cli/src/cmd/files/info.ts, line 98 (link)syntax: update to use renamed constant
UCD_STAT_TYPE_HEADER -
packages/cli/test/cmd/files/info.test.ts, line 4 (link)syntax: breaking change: import renamed constant
-
packages/cli/test/cmd/files/info.test.ts, line 35 (link)syntax: update to use renamed constant
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
packages/cli/test/cmd/files/info.test.ts, line 59 (link)syntax: update to use renamed constant
-
packages/cli/test/cmd/files/info.test.ts, line 81 (link)syntax: update to use renamed constant
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
packages/cli/test/cmd/files/info.test.ts, line 126 (link)syntax: update to use renamed constant
-
apps/web/src/apis/files.ts, line 5 (link)logic: inconsistent header name hardcoded
This should import from
@ucdjs/envfor consistency and use the new standardized name. The header value also needs to match the API's newX-UCD-Stat-Typename:Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
apps/web/src/apis/files.ts, line 23 (link)style: use imported constant instead of local duplicate
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
4 files reviewed, 9 comments
🔗 Linked issue
📚 Description
This PR changes the ucd headers to be more standard. It also adds new headers, which will be used in the future, when we rewrite the api.
This helps close the mega pr of #420
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.