Feature/plans#65
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR expands library and plugin documentation, adds browser and type exports, extracts standard Tempo terms into a private plugin workspace, updates build and package exports, adjusts utility contracts, and adds Typedoc, test, version, and publishing configuration. ChangesLibrary APIs and contracts
Standard terms and packaging
Documentation, plugins, and release metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/library/src/common/international.library.ts (1)
210-212: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStandardize the JSDoc formatting.
For consistency with the rest of the newly added documentation in this file, consider capitalizing the sentence and adding a period.
♻️ Proposed fix
/** - * try to infer hemisphere using the timezone's daylight-savings setting + * Tries to infer the hemisphere using the timezone's daylight-savings setting. * `@remarks` This implementation intentionally differs from the version in `tempo-fns`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/library/src/common/international.library.ts` around lines 210 - 212, Standardize the JSDoc summary above the hemisphere-inference implementation by capitalizing its opening sentence and ending it with a period, while preserving the existing `@remarks` content.packages/library/src/common/temporal.library.ts (1)
153-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign JSDoc parameter name with the function signature.
The JSDoc uses
@param tzOrZdt, but the first overload signature directly below it uses the parameter namezdt. Documentation generators like Typedoc typically emit warnings for unmatched parameter names. Consider renaming the JSDoc parameter tozdt(or renaming the overload's parameter totzOrZdt) to ensure correct documentation output.♻️ Proposed refactor
- * `@param` tzOrZdt - The TimeZone string, Calendar string, or ZonedDateTime object - * `@param` cal - The optional Calendar string if `tzOrZdt` is a timezone + * `@param` zdt - The TimeZone string, Calendar string, or ZonedDateTime object + * `@param` cal - The optional Calendar string if `zdt` is a timezone * `@returns` A tuple of `[timeZoneId, calendarId]` */ export function getTemporalIds(zdt: Temporal.ZonedDateTime, cal?: Temporal.CalendarLike): [string, string];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/library/src/common/temporal.library.ts` around lines 153 - 161, Align the JSDoc parameter name with the overload signature in the temporal library: update the `@param tzOrZdt` entry to `@param zdt` for the overload immediately below it, or consistently rename that overload parameter instead. Ensure the documented name matches the function signature used by documentation generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/library/src/common/array.library.ts`:
- Around line 123-136: Expose the key-sequence overloads before the
implementation signatures: in packages/library/src/common/array.library.ts lines
123-136, add the byKey overload accepting a required keyof T followed by
optional keyof T values and returning Record<PropertyKey, T[]>; in lines
162-175, add the corresponding byLkp overload returning Record<PropertyKey, T>.
Keep both existing implementations unchanged.
In `@packages/library/src/common/string.library.ts`:
- Around line 275-284: Update the padding character in the affected
string-padding function in string.library.ts from \u007F to \u00A0, ensuring its
documented non-breaking-space behavior matches the implementation.
In `@packages/library/src/common/webtoken.library.ts`:
- Around line 12-16: Update the documentation for the decoded JWT claims in
webtoken.library.ts to explicitly state that the payload is unverified and
attacker-controlled. Clarify that it is for reading public claims only and must
not be used for authentication, authorization, or any other security decision.
---
Nitpick comments:
In `@packages/library/src/common/international.library.ts`:
- Around line 210-212: Standardize the JSDoc summary above the
hemisphere-inference implementation by capitalizing its opening sentence and
ending it with a period, while preserving the existing `@remarks` content.
In `@packages/library/src/common/temporal.library.ts`:
- Around line 153-161: Align the JSDoc parameter name with the overload
signature in the temporal library: update the `@param tzOrZdt` entry to `@param
zdt` for the overload immediately below it, or consistently rename that overload
parameter instead. Ensure the documented name matches the function signature
used by documentation generation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee297883-9f97-4758-b44a-2e4a9afc05c8
📒 Files selected for processing (40)
packages/library/src/browser/mapper.library.tspackages/library/src/browser/tapper.class.tspackages/library/src/browser/webstore.class.tspackages/library/src/browser/window.library.tspackages/library/src/common/array.library.tspackages/library/src/common/assertion.library.tspackages/library/src/common/boundary.library.tspackages/library/src/common/buffer.library.tspackages/library/src/common/cipher.library.tspackages/library/src/common/class.library.tspackages/library/src/common/coercion.library.tspackages/library/src/common/enumerate.library.tspackages/library/src/common/function.library.tspackages/library/src/common/international.library.tspackages/library/src/common/logger.class.tspackages/library/src/common/number.library.tspackages/library/src/common/object.library.tspackages/library/src/common/pledge.class.tspackages/library/src/common/primitive.library.tspackages/library/src/common/proxy.library.tspackages/library/src/common/reflection.library.tspackages/library/src/common/request.library.tspackages/library/src/common/scopedset.class.tspackages/library/src/common/serialize.library.tspackages/library/src/common/storage.library.tspackages/library/src/common/string.library.tspackages/library/src/common/symbol.library.tspackages/library/src/common/temporal.library.tspackages/library/src/common/type.library.tspackages/library/src/common/utility.library.tspackages/library/src/common/webtoken.library.tspackages/library/src/server/auth.library.tspackages/library/src/server/file.library.tspackages/library/test/common/number_library.test.tspackages/plugins/finance/package.jsonpackages/tempo/.vitepress/config.tspackages/tempo/.vitepress/theme/data/catalog.jsonpackages/tempo/doc/6-utility-library/tempo.library.mdpackages/tempo/package.jsonpackages/tempo/typedoc.library.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/sync/src/index.ts`:
- Around line 10-11: Align startClock in the sync plugin with its documented
usage by returning the clock/buffer object that consumers assign to clock and
read via clock.buffer, and update its declared return type from void
accordingly. Ensure the implementation returns that object at runtime while
preserving existing clock-start behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: db67241d-41ee-4a23-aa08-37dcc34d52e7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
package.jsonpackages/functions/README.mdpackages/functions/package.jsonpackages/library/package.jsonpackages/plugins/.std/README.mdpackages/plugins/.std/package.jsonpackages/plugins/.std/src/index.tspackages/plugins/.std/src/term.quarter.tspackages/plugins/.std/src/term.season.tspackages/plugins/.std/src/term.timeline.tspackages/plugins/.std/src/term.zodiac.tspackages/plugins/.std/test/term.test.tspackages/plugins/.std/test/term_unified.test.tspackages/plugins/.std/test/tsconfig.jsonpackages/plugins/.std/tsconfig.jsonpackages/plugins/astro/src/index.tspackages/plugins/batch/src/BatchOrchestrator.tspackages/plugins/batch/src/index.tspackages/plugins/finance/src/index.tspackages/plugins/snap/src/index.tspackages/plugins/sync/src/AtomicClock.tspackages/plugins/sync/src/AtomicReader.tspackages/plugins/sync/src/index.tspackages/plugins/vitest.shared.tspackages/tempo/bin/resolve-types.tspackages/tempo/doc/3-extending-tempo/tempo.modularity.mdpackages/tempo/doc/3-extending-tempo/tempo.term.mdpackages/tempo/package.jsonpackages/tempo/plan/refactor-path-terms.mdpackages/tempo/rollup.config.jspackages/tempo/src/engine/engine.alias.tspackages/tempo/src/plugin-api.index.tspackages/tempo/src/plugin/term/term.index.tspackages/tempo/src/std.d.tspackages/tempo/src/tempo.index.tspackages/tempo/src/tempo.version.tspackages/tempo/src/tsconfig.jsonpackages/tempo/test/core/alias-engine.test.tspackages/tempo/test/core/discovery.getters.test.tspackages/tempo/test/plugins/debug_term.test.tspackages/tempo/vitest.config.ts
💤 Files with no reviewable changes (4)
- packages/plugins/sync/src/AtomicReader.ts
- packages/tempo/test/plugins/debug_term.test.ts
- packages/tempo/plan/refactor-path-terms.md
- packages/plugins/sync/src/AtomicClock.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/tempo/src/plugin/term/std.d.ts (1)
1-1: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExpose
StandardTermsasreadonly TermPlugin[].
any[]drops element type safety, and the exported array is still mutable at the type level. Use the actual term plugin type here to make the public API safer and clearer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/src/plugin/term/std.d.ts` at line 1, Update the StandardTerms declaration to use a readonly array of TermPlugin values instead of any[]. Import or reference the existing TermPlugin type, preserving the exported symbol while enforcing element type safety and preventing type-level mutation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/tempo/src/plugin/term/std.d.ts`:
- Line 1: Update the StandardTerms declaration to use a readonly array of
TermPlugin values instead of any[]. Import or reference the existing TermPlugin
type, preserving the exported symbol while enforcing element type safety and
preventing type-level mutation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4da303c0-b827-4e4b-ad71-7e2840ef5784
📒 Files selected for processing (3)
packages/plugins/vitest.shared.tspackages/tempo/src/plugin/term/std.d.tspackages/tempo/src/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tempo/src/tsconfig.json
- packages/plugins/vitest.shared.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/plugins/finance/README.md (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a stable local or version-pinned logo asset.
Referencing
mainmakes published documentation non-reproducible and can cause the image to drift or disappear independently of the release being documented. Prefer a repository-local asset for generated docs, or pin the remote URL to a release/tag/commit.
packages/plugins/finance/README.md#L1-L1: replace the mutablemainURL with a stable asset reference.packages/plugins/finance/doc/index.md#L1-L1: replace the mutablemainURL with a stable asset reference.packages/plugins/snap/doc/index.md#L1-L1: replace the mutablemainURL with a stable asset reference.packages/plugins/sync/doc/index.md#L1-L1: replace the mutablemainURL with a stable asset reference.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/finance/README.md` at line 1, Replace the mutable logo URL with a stable repository-local asset or version-pinned reference in packages/plugins/finance/README.md lines 1-1, packages/plugins/finance/doc/index.md lines 1-1, packages/plugins/snap/doc/index.md lines 1-1, and packages/plugins/sync/doc/index.md lines 1-1.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/functions/package.json`:
- Around line 60-68: Move `@js-temporal/polyfill` from devDependencies into
peerDependencies, and add or preserve its peerDependenciesMeta entry with
optional set to true. Keep the existing dependency version and remove the
duplicate devDependency entry.
In `@packages/tempo/src/module/module.benchmark.ts`:
- Line 41: Update the getTime function’s performance availability check to test
whether global.performance is actually defined, rather than comparing it to the
string "undefined". Call global.performance.now() only when the object exists;
otherwise fall back to Date.now().
---
Nitpick comments:
In `@packages/plugins/finance/README.md`:
- Line 1: Replace the mutable logo URL with a stable repository-local asset or
version-pinned reference in packages/plugins/finance/README.md lines 1-1,
packages/plugins/finance/doc/index.md lines 1-1,
packages/plugins/snap/doc/index.md lines 1-1, and
packages/plugins/sync/doc/index.md lines 1-1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46dec1ae-1bad-4fbc-b34f-cb4db77ec227
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
README.mddoc/build-pipeline-internals.mddoc/contributing-and-testing.mddoc/monorepo-architecture.mdpackages/functions/CHANGELOG.mdpackages/functions/README.mdpackages/functions/package.jsonpackages/library/README.mdpackages/library/src/common/enumerate.library.tspackages/library/src/common/type.library.tspackages/plugins/astro/doc/index.mdpackages/plugins/batch/doc/index.mdpackages/plugins/finance/README.mdpackages/plugins/finance/doc/index.mdpackages/plugins/snap/doc/index.mdpackages/plugins/sync/doc/index.mdpackages/plugins/vitest.shared.tspackages/tempo/doc/2-core-concepts/tempo.format.mdpackages/tempo/src/core.index.tspackages/tempo/src/module/module.benchmark.tspackages/tempo/src/module/module.format.tspackages/tempo/src/module/module.index.tspackages/tempo/src/plugin/term/std.d.tspackages/tempo/src/plugin/term/term.index.tspackages/tempo/src/support/support.index.tspackages/tempo/src/tempo.index.ts
💤 Files with no reviewable changes (1)
- packages/tempo/doc/2-core-concepts/tempo.format.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/functions/README.md
- packages/tempo/src/plugin/term/term.index.ts
- packages/tempo/src/tempo.index.ts
- packages/plugins/vitest.shared.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
confirm()dialog via the browser utility API.StandardTermsexport for easier setup.