Chore/remove esbuild tempo#59
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
🚧 Files skipped from review as they are similar to previous changes (14)
📝 WalkthroughWalkthroughThis release adds the ChangesTempo runtime and release
tempo-fns package
Tempo documentation and deployment
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tempo/src/tempo.class.ts (1)
564-591: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard against a non-string
namebefore using it in the version-suffix key.The branch condition now admits any object whose
typeis'plugin'|'namespace'|'module', even withoutisString(item.name)also holding (the check is... || isString(item.name), an OR, not AND). If such an item also sets.version,name.endsWith(suffix)at line 585 will throwTypeErroron a non-stringname. All current factories (definePlugin/defineModule/defineNamespace) always set a stringname, so this isn't reachable today, but it's a cheap, worthwhile guard against future/third-party plugin objects.🛡️ Suggested guard
- else if (isObject(item) && isFunction(item.install) && (item.type === 'plugin' || item.type === 'namespace' || item.type === 'module' || isString(item.name))) { + else if (isObject(item) && isFunction(item.install) && isString(item.name) && (item.type === 'plugin' || item.type === 'namespace' || item.type === 'module' || true)) {🤖 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/tempo.class.ts` around lines 564 - 591, Guard the version-registration logic in the object-form plugin branch before calling name.endsWith in Tempo.install. Only construct the version-suffix key when name is a string; preserve the existing suffix selection and version assignment for valid named plugin, namespace, or module objects.
🟡 Minor comments (16)
packages/tempo/doc/2-core-concepts/tempo.duration.md-21-31 (1)
21-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a fixed anchor for the numeric duration examples.
new Tempo()makes the results depend on the reader’s execution date and time, but the examples claim values such as~219days and~31.28weeks. Use a fixed date/time or mark the outputs as illustrative so the documentation remains reproducible.🤖 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/doc/2-core-concepts/tempo.duration.md` around lines 21 - 31, Update the numeric duration examples around `now.until` to use a fixed, explicit anchor date/time instead of `new Tempo()`, so the shown minute, day, week, and hour outputs remain reproducible; alternatively, clearly label the values as illustrative rather than exact.packages/tempo/doc/7-ecosystem/comparison.md-67-67 (1)
67-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the Quick Start fragment
../../README.mdresolves topackages/tempo/README.md, and that file’s heading is## ⚡ Quick Start, so the link should use the GitHub slug#quick-startinstead of#🛠️-quick-start.🤖 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/doc/7-ecosystem/comparison.md` at line 67, Update the Quick Start Guide link in comparison.md to use the README heading’s GitHub fragment `#quick-start` instead of #🛠️-quick-start, keeping the existing relative README target unchanged.packages/tempo/doc/2-core-concepts/tempo.parse.md-13-15 (1)
13-15: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid a hard-coded date for this relative input.
next Tuesdayis evaluated relative to the current clock, so2026-04-28is already stale in the July 2026 documentation context and will eventually disagree with actual output. Use a fixed reference-time option if supported, or show only stable output properties such as the timezone and weekday.🤖 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/doc/2-core-concepts/tempo.parse.md` around lines 13 - 15, Update the parse example and its console output in the documentation to avoid the hard-coded date for the relative “next Tuesday” input. Provide a fixed reference-time option if the parse API supports it; otherwise, assert or display only stable properties such as the timezone and weekday while preserving the Australia/Sydney timezone example.packages/tempo/doc/3-extending-tempo/tempo.namespace.md-162-179 (1)
162-179: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRegister the Format module before calling
tempo.format().This example uses the core build and only extends
SecretPlugin, butSecretPlugincallstempo.format(...). Core does not provide that method unlessFormatModuleis extended, sot[InternalTools].audit()will fail at runtime.🤖 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/doc/3-extending-tempo/tempo.namespace.md` around lines 162 - 179, Update the consumer example around SecretPlugin and Tempo.extend so the FormatModule is registered before t[InternalTools].audit() invokes tempo.format(). Import and extend the existing FormatModule alongside SecretPlugin, while preserving the namespace access example.packages/tempo/doc/3-extending-tempo/tempo.modularity.md-33-33 (1)
33-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the bulk-extension example valid TypeScript.
Tempo.extend(DurationModule, FormatModule, TermsModule, MutateModule, ...);contains a bare..., so copying it produces a syntax error. Use concrete modules or a real spread variable.🤖 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/doc/3-extending-tempo/tempo.modularity.md` at line 33, Update the bulk-extension example using Tempo.extend so it is valid TypeScript: replace the bare spread token with concrete module arguments or a declared module collection spread. Preserve the example’s intent of extending Tempo with multiple modules.packages/tempo/doc/8-project-and-support/releases/v3.x.md-6-6 (1)
6-6: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the
taxYearexample consistent with the namespace API.The namespace guide declares
taxYear: numberand accesses it ast.finance.taxYear, but this changelog showst.finance.taxYear(). Change the example to property access or update the API documentation consistently.🤖 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/doc/8-project-and-support/releases/v3.x.md` at line 6, Update the namespace example in the v3.x release documentation to access taxYear as the property t.finance.taxYear, matching the documented taxYear: number API; leave the isFiscalYearStart() method example unchanged.packages/tempo/doc/4-advanced-reference/tempo.shorthand.md-14-17 (1)
14-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the shorthand example’s missing modifier.
#timeOfDay.nextplacesnextin the range-key position while leaving the modifier empty; the supplied runtime grammar rejects that form for non-weekday keys. Use an operator plus a registered range key, or remove this example.🤖 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/doc/4-advanced-reference/tempo.shorthand.md` around lines 14 - 17, Update the shorthand example using `#timeOfDay.next` so it uses a valid operator and registered range key, rather than placing next in the range-key position with an empty modifier; alternatively remove that example while leaving the valid `#qtr`.>q1 example unchanged.packages/tempo/doc/7-ecosystem/extending-temporal.md-117-122 (1)
117-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAnchor the
until()example to a fixed reference date.
new Tempo()has no explicit reference date, but the example asserts a stable219; the result will vary depending on when the documentation is read or executed. Use a fixed date or omit the numeric output.🤖 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/doc/7-ecosystem/extending-temporal.md` around lines 117 - 122, Update the until() example using new Tempo() so its output is deterministic: initialize Tempo with an explicit fixed reference date matching the documented calculation, or remove the hardcoded numeric result. Keep the duration example and until() API usage intact.packages/tempo/doc/5-architecture-and-internals/tempo.benchmarks.md-97-100 (1)
97-100: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winDo not infer GC behavior from a negative heap delta.
A negative
heapUsedDeltaMbonly means the measured end-of-run heap was smaller than the start measurement; it does not prove that defer mode caused garbage collection to run faster or that fewer Proxy allocations occurred. Rephrase this as an observational result and document the measurement limitations.🤖 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/doc/5-architecture-and-internals/tempo.benchmarks.md` around lines 97 - 100, Revise the “Defer Mode is Highly Optimized” takeaway to describe the negative heapUsedDeltaMb only as an observed lower end-of-run heap measurement. Remove claims that defer mode accelerated garbage collection or reduced Proxy allocations, and document that start/end heap measurements cannot establish GC behavior or allocation rates.packages/tempo/doc/5-architecture-and-internals/architecture.md-131-135 (1)
131-135: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winQualify the Master Guard complexity claim.
The scan-and-consume loop is input-dependent, and line 134 explicitly describes registry-rebuild work after mutations. Therefore, the complete guard path is not “O(1) regardless of plugin depth” in the absolute sense. Describe this as average O(1) token lookup after cache construction, with additional input-scanning and rebuild costs.
Also applies to: 148-152
🤖 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/doc/5-architecture-and-internals/architecture.md` around lines 131 - 135, Update the “High-Speed Gatekeeper” claim in the architecture documentation to qualify O(1) as average token-lookup performance after wordlist cache construction, not an absolute end-to-end guarantee. Explicitly mention input-dependent scan-and-consume costs and registry wordlist rebuild costs after valid mutations, and apply the same clarification to the corresponding claim around the later referenced section.packages/tempo/doc/5-architecture-and-internals/tempo.benchmarks.md-27-32 (1)
27-32: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the guard description and complexity wording with the architecture page.
This page describes the guard as a static
#guardregex, butpackages/tempo/doc/5-architecture-and-internals/architecture.md:131-134describes a scan-and-consume guard that avoids backtracking regexes. The unconditionalO(1)claim also needs qualification.🤖 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/doc/5-architecture-and-internals/tempo.benchmarks.md` around lines 27 - 32, Update the “Master Guard (Fast-Fail)” section to describe the scan-and-consume `#guard` approach consistently with the architecture documentation, rather than calling it a static regex. Qualify the complexity statement based on the input being scanned instead of claiming unconditional O(1) behavior, and retain the existing fast-fail purpose and performance context.packages/tempo-fns/doc/README.md-42-47 (1)
42-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCDN example references undefined
Tempoglobal.The CDN example loads only
tempo-fns.global.jsbut usesnew Tempo(), which is not defined. SinceisFirstDayOfMonthis a Temporal-Only function (per CONTRIBUTING.md), the example should use a native Temporal type instead.📝 Proposed fix for the CDN example
<script src="https://cdn.jsdelivr.net/npm/@magmacomputing/tempo-fns/dist/tempo-fns.global.js"></script> <script> - // Access functions via the TempoFns global - TempoFns.isFirstDayOfMonth(new Tempo()); + // Access functions via the TempoFns global + TempoFns.isFirstDayOfMonth(Temporal.Now.plainDateISO()); </script>🤖 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-fns/doc/README.md` around lines 42 - 47, Update the CDN example in the README to replace the undefined Tempo constructor with the native Temporal type required by TempoFns.isFirstDayOfMonth, while keeping the existing TempoFns global invocation unchanged.packages/tempo-fns/doc/support.md-7-7 (1)
7-7: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLink to the tempo-fns contribution guide.
This page is for
tempo-fns, but the link targetspackages/tempo/CONTRIBUTING.md, sending contributors to the wrong package. Point it to the tempo-fns guide instead.🤖 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-fns/doc/support.md` at line 7, Update the Contributing Guide link in packages/tempo-fns/doc/support.md to target the tempo-fns contribution guide under packages/tempo-fns, while preserving the surrounding invitation text.packages/tempo-fns/src/scheduling/Interval.ts-4-5 (1)
4-5: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the normalized interval invariant.
startandendare publicly writable, so callers can reverse or otherwise corrupt the ordering established by the constructor. Make the endpoints readonly/private with readonly accessors, and freeze the instance if runtime mutation must also be prevented.🤖 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-fns/src/scheduling/Interval.ts` around lines 4 - 5, Update the Interval class’s start and end endpoint properties to prevent public reassignment while preserving constructor normalization: store them in private readonly fields and expose readonly accessors, and freeze the instance when runtime mutation protection is required. Ensure existing read access remains compatible and the invariant start <= end cannot be corrupted after construction.packages/tempo/test/core/bootstrap.test.ts-11-11 (1)
11-11: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBump the Node floor or avoid
import.meta.dirnamehere.package.jsonandpackages/tempo/package.jsonboth allow Node>=20.0.0, so this test can fail on 20.0–20.10 whereimport.meta.dirnameisn’t available.🤖 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/test/core/bootstrap.test.ts` at line 11, Update the fixture path setup in the bootstrap test to avoid import.meta.dirname, or raise the declared Node.js minimum versions in both package manifests to a version that supports it. Prefer the existing compatible path-resolution convention used by the test suite, preserving the resolved __fixtures__/config location.packages/tempo/rollup.config.js-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the remaining esbuild usage from Tempo
packages/tempo/rollup.config.jsstill importsrollup-plugin-esbuildand uses it for the premium license bundle, andpackages/tempo/package.jsonstill lists the devDependency. If the goal is full removal, drop those last references too.🤖 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/rollup.config.js` at line 7, Remove the remaining esbuild integration from the Tempo Rollup configuration: delete the rollup-plugin-esbuild import and remove its usage in the premium license bundle while preserving the bundle’s required behavior through the existing terser setup. Also remove the corresponding rollup-plugin-esbuild entry from Tempo’s package devDependencies.
🧹 Nitpick comments (7)
packages/tempo/doc/6-utility-library/tempo.enumerators.md (1)
87-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove duplicate code example from section 3.
Lines 88–95 repeat the
enumsnamespace import example from section 2 (lines 62–68) nearly verbatim — sameCOMPASS/MONTHdestructuring and usage, with only added inline comments and minor spacing differences. This redundancy under "Creating Custom Enums" is confusing since it doesn't demonstrate custom enum creation.♻️ Suggested edit: remove the duplicate block
or import from the explicit enums subpath export ```typescript -import { enums } from '`@magmacomputing/tempo/enums`'; - -const {COMPASS, MONTH} = enums; - -const direction = COMPASS.North; // 'north' -const monthIndex = MONTH.Feb; // 2 (since 'All' was index 0) -``` - -or import only the enum you need in a single line: -```typescript import { COMPASS, MONTH } from '`@magmacomputing/tempo/enums`'; console.log('compass: ', COMPASS.keys());🤖 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/doc/6-utility-library/tempo.enumerators.md` around lines 87 - 95, Remove the repeated `enums` namespace import example from section 3 of the enumerators documentation, including its code fence and “or import…” transition, while preserving the following direct `COMPASS, MONTH` import example as the section’s relevant content.packages/tempo-fns/src/business/workingHoursUntil.ts (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anywith a concrete type for thedeadlineparameter.Using
anyfordeadlinedefeats TypeScript's type safety. Sincethisis typed asTempo,deadlineshould beTempo(orTempo | stringif raw date strings are accepted).♻️ Proposed type fix
-export const workingHoursUntil = function (this: Tempo, deadline: any) { +export const workingHoursUntil = function (this: Tempo, deadline: Tempo | 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/tempo-fns/src/business/workingHoursUntil.ts` at line 13, Update the workingHoursUntil function’s deadline parameter from any to the concrete supported type, using Tempo if only Tempo values are accepted or Tempo | string if raw date strings are handled by the implementation.packages/tempo-fns/vitest.config.ts (1)
7-7: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCross-package polyfill path is fragile — consider a local or shared test fixture.
The polyfill is resolved via
../tempo/bin/temporal-polyfill.ts, creating a hard dependency on thetempopackage's internalbin/directory. If that file moves or thetempopackage restructures, alltempo-fnstests break with an opaque module-not-found error. Consider either copying the polyfill intotempo-fnsor extracting it to a shared test utility.🤖 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-fns/vitest.config.ts` at line 7, Replace the cross-package path in the Vitest configuration’s polyfill resolution with a local tempo-fns fixture or a shared test utility, rather than referencing tempo’s internal bin directory. Update the polyfill symbol used by the config so tempo-fns tests no longer depend on tempo’s package structure.packages/tempo-fns/src/index.ts (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate cron exports.
nextCronandprevCronare exported from the same module on separate lines. They can be combined into a single export statement for consistency with the other category exports.♻️ Proposed refactor
- export { nextCron } from './scheduling/cron.js'; - export { prevCron } from './scheduling/cron.js'; + export { nextCron, prevCron } from './scheduling/cron.js';🤖 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-fns/src/index.ts` around lines 10 - 11, Consolidate the nextCron and prevCron exports in the package index into one export statement from the scheduling/cron.js module, preserving both public exports and matching the style of the other category exports.packages/tempo-fns/doc/preview.html (1)
7-7: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the
markeddependency and add SRI.The unversioned CDN URL can silently serve a changed or compromised script. Pin a known version and add
integritypluscrossoriginattributes.🤖 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-fns/doc/preview.html` at line 7, Update the marked script tag in preview.html to use a specific known marked version instead of the unversioned CDN URL, and add matching integrity and crossorigin attributes for the pinned asset.Source: Linters/SAST tools
packages/library/package.json (1)
90-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBuild scripts hardcode relative path to root
node_modules.The
../../node_modules/typescript-7/bin/tscpath assumes a fixed monorepo depth. This works today but breaks silently if the package directory moves. Consider using a workspace-aware bin resolution (e.g.,npx tscwith the workspace TypeScript, or a script in the rootpackage.jsonthat delegates).🤖 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/package.json` around lines 90 - 91, Update the build and clean scripts in package.json to avoid the fixed ../../node_modules/typescript-7/bin/tsc path. Use the repository’s workspace-aware TypeScript command or an existing root-level delegation mechanism, while preserving the current tsc build and --clean behavior.packages/tempo/src/plugin/plugin.type.ts (1)
5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider typing
optionsinstead ofany.
install's new third parameter is typedany, discarding type safety for every plugin author. Call sites elsewhere (Tempo.extend(plugin, options?: t.Options)) already have a concreteOptionstype available — using it here (or a genericPlugin<T, O = t.Options>) would preserve type-checking without limiting flexibility.♻️ Suggested typing
-export interface Plugin<T = any> { +export interface Plugin<T = any, O = unknown> { name: string; version?: string; - install: (this: T, t: T, options?: any) => void; + install: (this: T, t: T, options?: O) => void; }🤖 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/plugin.type.ts` around lines 5 - 9, Update the Plugin interface’s install options parameter to use the existing concrete Options type, or introduce a second generic with that type as its default, so plugin authors retain type safety while supporting custom option types. Keep the existing T-based this and t parameters unchanged.
🤖 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/international.library.ts`:
- Around line 116-119: Resolve the misleading sync annotations on getHemisphere
and getISOWeekOfYear: either document that their implementations intentionally
differ from tempo-fns, including return and fallback behavior, or extract a
shared core while preserving this library’s existing consumer-facing outputs and
edge-case behavior. Apply the chosen approach consistently to both functions and
remove the implication that their implementations can be directly synchronized.
In `@packages/tempo-fns/doc/functions/index.md`:
- Around line 7-9: Clarify the public API contract in the documentation around
the exported Interval class: state that Interval accepts Tempo values and relies
on tempo.epoch.ns, rather than implying all utilities accept native Temporal
instances. Keep the native Temporal guidance only where it accurately applies,
and ensure examples do not suggest unsupported inputs.
In `@packages/tempo-fns/src/business/workingHoursUntil.ts`:
- Around line 21-25: Update preloadHolidays so cached holiday dates are derived
from its year argument rather than always using 2026 values; alternatively, make
the function explicitly throw a “not implemented” error instead of populating
incorrect data. Preserve the existing cacheKey behavior and async interface.
- Around line 13-19: The exported workingHoursUntil utility is still a hardcoded
stub and must not appear as a normal public API. Before release, either
implement its working-hours calculation using the deadline, weekends, holidays,
and region cache, or mark and expose it explicitly as experimental through the
package export path and JSDoc; update the Tempo interface augmentation
consistently.
In `@packages/tempo-fns/src/calendar/getISOWeekOfYear.ts`:
- Around line 1-17: Remove the `@js-temporal/polyfill` import and use the input’s
Temporal methods to normalize zdt to an ISO-calendar PlainDate before the
ISO-week calculations. Update getISOWeekOfYear so both ZonedDateTime and
PlainDate inputs are converted consistently via toPlainDate when available and
withCalendar('iso8601'), then perform targetThursday, week1Thursday, and since
calculations entirely on PlainDate values.
In `@packages/tempo-fns/src/scheduling/cron.ts`:
- Around line 3-9: Implement actual cron traversal in nextCron and prevCron
using the provided pattern, returning the next and previous matching Tempo
occurrence respectively instead of this. If traversal cannot be implemented yet,
remove nextCron and prevCron from the public API rather than exposing no-op
methods.
In `@packages/tempo-fns/src/timezone/getHemisphere.ts`:
- Around line 9-10: Update the jan === jul branch in getHemisphere to check the
existing southPrefixes classification before returning 'E', returning 'S' for
matching Southern-hemisphere zones such as Australia/Perth and 'E' otherwise;
preserve the current jan < jul and jan > jul behavior.
- Around line 9-10: Correct the return mapping in getHemisphere: when jan < jul,
return 'N', and when jan > jul, return 'S'. Preserve the existing 'E' result
when jan === jul.
- Around line 11-13: Update the catch fallback in getHemisphere to return
undefined for invalid timezones instead of 'N', and widen its return type
accordingly so unknown hemisphere values propagate correctly to consumers such
as config.sphere.
In `@packages/tempo-fns/src/timezone/isDST.ts`:
- Around line 6-8: Update the string-handling branch in the isDST implementation
so the provided timeZone is applied when converting a plain date string, rather
than calling Temporal.ZonedDateTime.from without it. Ensure bare ISO date
strings are accepted while preserving the existing behavior for ZonedDateTime
inputs and omitted dates.
In `@packages/tempo-fns/test/isFirstDayOfMonth.test.ts`:
- Around line 1-2: Update the isFirstDayOfMonth import in the test to use the
calendar subdirectory path matching the source location and index.ts barrel
export, while leaving the Tempo import unchanged.
In `@packages/tempo-fns/test/isSameFiscalQuarter.test.ts`:
- Around line 1-2: Update the isSameFiscalQuarter import in the test to use the
business barrel path, matching the source location and index.ts export: import
it from the business subpath rather than directly from src.
In `@packages/tempo/doc/1-getting-started/installation.md`:
- Around line 167-184: Update the Static CDNs import-map example to include the
`@js-temporal/polyfill` mapping and import, matching the existing Global Bundle
and Smart CDN examples so browsers without native Temporal can load the setup
successfully.
- Around line 9-12: Update the Temporal support description to reflect native
support in Node.js 26 and remove the outdated future-release and
--harmony-temporal guidance. Revise the browser support wording to explicitly
identify Safari and iOS as polyfill-only, while retaining the caniuse reference
and the recommendation that Tempo does not bundle a polyfill by default.
In `@packages/tempo/doc/2-core-concepts/tempo.format.md`:
- Line 194: Update the formatter behavior described around the direct
Intl.DateTimeFormatOptions path so withTimeZone and withCalendar errors are not
swallowed indiscriminately. Only apply the epochMilliseconds fallback for the
known Temporal/Intl mismatch case; rethrow errors caused by invalid user
timeZone or calendar options so invalid input cannot produce an incorrect
formatted date.
In `@packages/tempo/doc/4-advanced-reference/tempo.ticker.md`:
- Around line 7-11: Update the “Explore the Tempo Ticker Plugin” link in the
ticker documentation to point to the official Tempo Registry used by the runtime
contract in extend.ticker.ts, so users reach the correct installation and
license-token endpoint; if retaining the current documentation URL, separate it
clearly from the registry and token-access link.
In `@packages/tempo/doc/4-advanced-reference/tempo.weekday.md`:
- Around line 19-26: The weekday modifier table incorrectly describes <= as
preceding-or-equal to today. Update the <= entry in the weekday reference to
document its implemented boundary behavior, and add an example contrasting <
with <=, using the lexer semantics as the source of truth.
In `@packages/tempo/src/plugin/plugin.util.ts`:
- Around line 95-99: Update defineModule and definePlugin to create new plugin
objects via spreading the input before adding their respective type fields,
rather than mutating the caller’s object with Object.assign. Register and return
the new object while preserving the existing type-specific behavior and
compatibility with frozen inputs.
- Around line 215-265: Update the namespace getter in defineNamespace so it does
not attempt to define cacheKey on non-extensible or frozen Tempo instances.
Reuse the Reflect.isExtensible guard and behavior established by `#setLazy`, or
store proxies in a WeakMap fallback, while preserving proxy caching for
extensible instances.
---
Outside diff comments:
In `@packages/tempo/src/tempo.class.ts`:
- Around line 564-591: Guard the version-registration logic in the object-form
plugin branch before calling name.endsWith in Tempo.install. Only construct the
version-suffix key when name is a string; preserve the existing suffix selection
and version assignment for valid named plugin, namespace, or module objects.
---
Minor comments:
In `@packages/tempo-fns/doc/README.md`:
- Around line 42-47: Update the CDN example in the README to replace the
undefined Tempo constructor with the native Temporal type required by
TempoFns.isFirstDayOfMonth, while keeping the existing TempoFns global
invocation unchanged.
In `@packages/tempo-fns/doc/support.md`:
- Line 7: Update the Contributing Guide link in
packages/tempo-fns/doc/support.md to target the tempo-fns contribution guide
under packages/tempo-fns, while preserving the surrounding invitation text.
In `@packages/tempo-fns/src/scheduling/Interval.ts`:
- Around line 4-5: Update the Interval class’s start and end endpoint properties
to prevent public reassignment while preserving constructor normalization: store
them in private readonly fields and expose readonly accessors, and freeze the
instance when runtime mutation protection is required. Ensure existing read
access remains compatible and the invariant start <= end cannot be corrupted
after construction.
In `@packages/tempo/doc/2-core-concepts/tempo.duration.md`:
- Around line 21-31: Update the numeric duration examples around `now.until` to
use a fixed, explicit anchor date/time instead of `new Tempo()`, so the shown
minute, day, week, and hour outputs remain reproducible; alternatively, clearly
label the values as illustrative rather than exact.
In `@packages/tempo/doc/2-core-concepts/tempo.parse.md`:
- Around line 13-15: Update the parse example and its console output in the
documentation to avoid the hard-coded date for the relative “next Tuesday”
input. Provide a fixed reference-time option if the parse API supports it;
otherwise, assert or display only stable properties such as the timezone and
weekday while preserving the Australia/Sydney timezone example.
In `@packages/tempo/doc/3-extending-tempo/tempo.modularity.md`:
- Line 33: Update the bulk-extension example using Tempo.extend so it is valid
TypeScript: replace the bare spread token with concrete module arguments or a
declared module collection spread. Preserve the example’s intent of extending
Tempo with multiple modules.
In `@packages/tempo/doc/3-extending-tempo/tempo.namespace.md`:
- Around line 162-179: Update the consumer example around SecretPlugin and
Tempo.extend so the FormatModule is registered before t[InternalTools].audit()
invokes tempo.format(). Import and extend the existing FormatModule alongside
SecretPlugin, while preserving the namespace access example.
In `@packages/tempo/doc/4-advanced-reference/tempo.shorthand.md`:
- Around line 14-17: Update the shorthand example using `#timeOfDay.next` so it
uses a valid operator and registered range key, rather than placing next in the
range-key position with an empty modifier; alternatively remove that example
while leaving the valid `#qtr`.>q1 example unchanged.
In `@packages/tempo/doc/5-architecture-and-internals/architecture.md`:
- Around line 131-135: Update the “High-Speed Gatekeeper” claim in the
architecture documentation to qualify O(1) as average token-lookup performance
after wordlist cache construction, not an absolute end-to-end guarantee.
Explicitly mention input-dependent scan-and-consume costs and registry wordlist
rebuild costs after valid mutations, and apply the same clarification to the
corresponding claim around the later referenced section.
In `@packages/tempo/doc/5-architecture-and-internals/tempo.benchmarks.md`:
- Around line 97-100: Revise the “Defer Mode is Highly Optimized” takeaway to
describe the negative heapUsedDeltaMb only as an observed lower end-of-run heap
measurement. Remove claims that defer mode accelerated garbage collection or
reduced Proxy allocations, and document that start/end heap measurements cannot
establish GC behavior or allocation rates.
- Around line 27-32: Update the “Master Guard (Fast-Fail)” section to describe
the scan-and-consume `#guard` approach consistently with the architecture
documentation, rather than calling it a static regex. Qualify the complexity
statement based on the input being scanned instead of claiming unconditional
O(1) behavior, and retain the existing fast-fail purpose and performance
context.
In `@packages/tempo/doc/7-ecosystem/comparison.md`:
- Line 67: Update the Quick Start Guide link in comparison.md to use the README
heading’s GitHub fragment `#quick-start` instead of #🛠️-quick-start, keeping the
existing relative README target unchanged.
In `@packages/tempo/doc/7-ecosystem/extending-temporal.md`:
- Around line 117-122: Update the until() example using new Tempo() so its
output is deterministic: initialize Tempo with an explicit fixed reference date
matching the documented calculation, or remove the hardcoded numeric result.
Keep the duration example and until() API usage intact.
In `@packages/tempo/doc/8-project-and-support/releases/v3.x.md`:
- Line 6: Update the namespace example in the v3.x release documentation to
access taxYear as the property t.finance.taxYear, matching the documented
taxYear: number API; leave the isFiscalYearStart() method example unchanged.
In `@packages/tempo/rollup.config.js`:
- Line 7: Remove the remaining esbuild integration from the Tempo Rollup
configuration: delete the rollup-plugin-esbuild import and remove its usage in
the premium license bundle while preserving the bundle’s required behavior
through the existing terser setup. Also remove the corresponding
rollup-plugin-esbuild entry from Tempo’s package devDependencies.
In `@packages/tempo/test/core/bootstrap.test.ts`:
- Line 11: Update the fixture path setup in the bootstrap test to avoid
import.meta.dirname, or raise the declared Node.js minimum versions in both
package manifests to a version that supports it. Prefer the existing compatible
path-resolution convention used by the test suite, preserving the resolved
__fixtures__/config location.
---
Nitpick comments:
In `@packages/library/package.json`:
- Around line 90-91: Update the build and clean scripts in package.json to avoid
the fixed ../../node_modules/typescript-7/bin/tsc path. Use the repository’s
workspace-aware TypeScript command or an existing root-level delegation
mechanism, while preserving the current tsc build and --clean behavior.
In `@packages/tempo-fns/doc/preview.html`:
- Line 7: Update the marked script tag in preview.html to use a specific known
marked version instead of the unversioned CDN URL, and add matching integrity
and crossorigin attributes for the pinned asset.
In `@packages/tempo-fns/src/business/workingHoursUntil.ts`:
- Line 13: Update the workingHoursUntil function’s deadline parameter from any
to the concrete supported type, using Tempo if only Tempo values are accepted or
Tempo | string if raw date strings are handled by the implementation.
In `@packages/tempo-fns/src/index.ts`:
- Around line 10-11: Consolidate the nextCron and prevCron exports in the
package index into one export statement from the scheduling/cron.js module,
preserving both public exports and matching the style of the other category
exports.
In `@packages/tempo-fns/vitest.config.ts`:
- Line 7: Replace the cross-package path in the Vitest configuration’s polyfill
resolution with a local tempo-fns fixture or a shared test utility, rather than
referencing tempo’s internal bin directory. Update the polyfill symbol used by
the config so tempo-fns tests no longer depend on tempo’s package structure.
In `@packages/tempo/doc/6-utility-library/tempo.enumerators.md`:
- Around line 87-95: Remove the repeated `enums` namespace import example from
section 3 of the enumerators documentation, including its code fence and “or
import…” transition, while preserving the following direct `COMPASS, MONTH`
import example as the section’s relevant content.
In `@packages/tempo/src/plugin/plugin.type.ts`:
- Around line 5-9: Update the Plugin interface’s install options parameter to
use the existing concrete Options type, or introduce a second generic with that
type as its default, so plugin authors retain type safety while supporting
custom option types. Keep the existing T-based this and t parameters unchanged.
🪄 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: 9e5a39e8-48e1-4c77-b36c-579100e88339
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/tempo-fns/doc/public/img/tempo-fns-logo.svgis excluded by!**/*.svgpackages/tempo-fns/img/tempo-fns-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (100)
.github/workflows/deploy-docs.ymlpackage.jsonpackages/library/package.jsonpackages/library/src/common/class.library.tspackages/library/src/common/international.library.tspackages/library/src/common/pledge.class.tspackages/library/src/common/temporal.library.tspackages/tempo-fns/bin/sync-docs.mjspackages/tempo-fns/doc/.vitepress/config.mtspackages/tempo-fns/doc/CONTRIBUTING.mdpackages/tempo-fns/doc/README.mdpackages/tempo-fns/doc/functions/business/index.mdpackages/tempo-fns/doc/functions/calendar/index.mdpackages/tempo-fns/doc/functions/duration/index.mdpackages/tempo-fns/doc/functions/index.mdpackages/tempo-fns/doc/functions/scheduling/index.mdpackages/tempo-fns/doc/functions/timezone/index.mdpackages/tempo-fns/doc/index.mdpackages/tempo-fns/doc/preview.htmlpackages/tempo-fns/doc/support.mdpackages/tempo-fns/package.jsonpackages/tempo-fns/rollup.config.jspackages/tempo-fns/src/business/README.mdpackages/tempo-fns/src/business/isSameFiscalQuarter.tspackages/tempo-fns/src/business/workingHoursUntil.tspackages/tempo-fns/src/calendar/README.mdpackages/tempo-fns/src/calendar/getISOWeekOfYear.tspackages/tempo-fns/src/calendar/isFirstDayOfMonth.tspackages/tempo-fns/src/duration/README.mdpackages/tempo-fns/src/duration/normaliseFractionalDurations.tspackages/tempo-fns/src/index.tspackages/tempo-fns/src/scheduling/Interval.tspackages/tempo-fns/src/scheduling/README.mdpackages/tempo-fns/src/scheduling/cron.tspackages/tempo-fns/src/timezone/README.mdpackages/tempo-fns/src/timezone/getHemisphere.tspackages/tempo-fns/src/timezone/getOffsets.tspackages/tempo-fns/src/timezone/isDST.tspackages/tempo-fns/src/timezone/normalizeUtcOffset.tspackages/tempo-fns/test/isFirstDayOfMonth.test.tspackages/tempo-fns/test/isSameFiscalQuarter.test.tspackages/tempo-fns/test/tsconfig.jsonpackages/tempo-fns/tsconfig.jsonpackages/tempo-fns/vitest.config.tspackages/tempo/.vitepress/config.tspackages/tempo/CHANGELOG.mdpackages/tempo/CONTRIBUTING.mdpackages/tempo/bin/tempo.jspackages/tempo/doc/1-getting-started/installation.mdpackages/tempo/doc/1-getting-started/tempo.cookbook.mdpackages/tempo/doc/2-core-concepts/tempo.config.mdpackages/tempo/doc/2-core-concepts/tempo.duration.mdpackages/tempo/doc/2-core-concepts/tempo.format.mdpackages/tempo/doc/2-core-concepts/tempo.layout.mdpackages/tempo/doc/2-core-concepts/tempo.parse.mdpackages/tempo/doc/2-core-concepts/tempo.registry.mdpackages/tempo/doc/3-extending-tempo/ecosystem.mdpackages/tempo/doc/3-extending-tempo/tempo.extension.mdpackages/tempo/doc/3-extending-tempo/tempo.modularity.mdpackages/tempo/doc/3-extending-tempo/tempo.namespace.mdpackages/tempo/doc/3-extending-tempo/tempo.plugin.mdpackages/tempo/doc/3-extending-tempo/tempo.term.mdpackages/tempo/doc/4-advanced-reference/sandbox-factory.mdpackages/tempo/doc/4-advanced-reference/tempo.debugging.mdpackages/tempo/doc/4-advanced-reference/tempo.locale.mdpackages/tempo/doc/4-advanced-reference/tempo.planner.mdpackages/tempo/doc/4-advanced-reference/tempo.shorthand.mdpackages/tempo/doc/4-advanced-reference/tempo.ticker.mdpackages/tempo/doc/4-advanced-reference/tempo.weekday.mdpackages/tempo/doc/5-architecture-and-internals/architecture.mdpackages/tempo/doc/5-architecture-and-internals/lazy-evaluation-pattern.mdpackages/tempo/doc/5-architecture-and-internals/soft_freeze_strategy.mdpackages/tempo/doc/5-architecture-and-internals/tempo.benchmarks.mdpackages/tempo/doc/6-utility-library/tempo.decorators.mdpackages/tempo/doc/6-utility-library/tempo.enumerators.mdpackages/tempo/doc/6-utility-library/tempo.library.mdpackages/tempo/doc/6-utility-library/tempo.pledge.mdpackages/tempo/doc/6-utility-library/tempo.serializers.mdpackages/tempo/doc/7-ecosystem/comparison.mdpackages/tempo/doc/7-ecosystem/extending-temporal.mdpackages/tempo/doc/7-ecosystem/vision.mdpackages/tempo/doc/8-project-and-support/commercial.mdpackages/tempo/doc/8-project-and-support/migration-guide.mdpackages/tempo/doc/8-project-and-support/releases/index.mdpackages/tempo/doc/8-project-and-support/releases/v0.x.mdpackages/tempo/doc/8-project-and-support/releases/v1.x.mdpackages/tempo/doc/8-project-and-support/releases/v2.x.mdpackages/tempo/doc/8-project-and-support/releases/v3.x.mdpackages/tempo/doc/8-project-and-support/releases/v4.x.mdpackages/tempo/doc/architecture.mdpackages/tempo/doc/tempo-vs-temporal.mdpackages/tempo/package.jsonpackages/tempo/rollup.config.jspackages/tempo/src/plugin/plugin.type.tspackages/tempo/src/plugin/plugin.util.tspackages/tempo/src/plugin/term/term.util.tspackages/tempo/src/tempo.class.tspackages/tempo/src/tempo.version.tspackages/tempo/test/core/bootstrap.test.tspackages/tempo/test/core/discovery-extend.test.ts
💤 Files with no reviewable changes (3)
- packages/tempo/doc/tempo-vs-temporal.md
- packages/tempo/doc/architecture.md
- packages/tempo/bin/tempo.js
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (18)
packages/library/src/common/international.library.ts (1)
116-119: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Sync notes reference
tempo-fnsbut implementations are already divergent.The
@note also maintained in tempo-fns — please sync changesannotations are added togetHemisphereandgetISOWeekOfYear, but the implementations already differ significantly:
getHemisphere: This library returns'north'/'south'/undefinedwith southern-prefix fallback logic. Thetempo-fnsversion returns'E'/'S'/'N'with a'N'fallback for invalid timezones. Different return value formats and different fallback strategies.getISOWeekOfYear: This library uses a nearest-Thursday +Math.ceil(dayOfYear / 7)approach. Thetempo-fnsversion uses a first-Thursday +Math.floor(daysBetween / 7) + 1approach. These can produce different results for edge-case dates near year boundaries.The sync notes imply the implementations should stay identical, but they can't be synced without breaking consumers that depend on the current return formats. Consider either (1) documenting the expected differences explicitly, or (2) extracting a shared core that both packages consume.
Also applies to: 151-151
🤖 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 116 - 119, Resolve the misleading sync annotations on getHemisphere and getISOWeekOfYear: either document that their implementations intentionally differ from tempo-fns, including return and fallback behavior, or extract a shared core while preserving this library’s existing consumer-facing outputs and edge-case behavior. Apply the chosen approach consistently to both functions and remove the implication that their implementations can be directly synchronized.packages/tempo-fns/doc/functions/index.md (1)
7-9: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clarify the supported input type for public utilities.
The documentation says
tempo-fnsconsumes native Temporal objects, but the exportedIntervalclass acceptsTempoand readstempo.epoch.ns. Either makeIntervalaccept native Temporal values or document the Tempo-specific API explicitly; otherwise users following this page will encounter a compile-time or runtime mismatch.🤖 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-fns/doc/functions/index.md` around lines 7 - 9, Clarify the public API contract in the documentation around the exported Interval class: state that Interval accepts Tempo values and relies on tempo.epoch.ns, rather than implying all utilities accept native Temporal instances. Keep the native Temporal guidance only where it accurately applies, and ensure examples do not suggest unsupported inputs.packages/tempo-fns/src/business/workingHoursUntil.ts (2)
13-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stub
workingHoursUntilreturns a hardcoded value — clarify intent before shipping.
workingHoursUntilalways returns12regardless of inputs. If this function is exported from the package's public API (the module augmentation on lines 27–32 adds it to theTempointerface), consumers will get a non-functional utility. Consider either gating this behind anexperimentalexport path, adding a@experimentalJSDoc tag, or implementing real logic before release.🤖 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-fns/src/business/workingHoursUntil.ts` around lines 13 - 19, The exported workingHoursUntil utility is still a hardcoded stub and must not appear as a normal public API. Before release, either implement its working-hours calculation using the deadline, weekends, holidays, and region cache, or mark and expose it explicitly as experimental through the package export path and JSDoc; update the Tempo interface augmentation consistently.
21-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
preloadHolidayshardcodes 2026 holiday dates — will be stale in 2027+.The stub caches
['2026-01-01', '2026-12-25']regardless of theyearargument passed. If this ships as-is, consumers callingpreloadHolidays('AU-NSW', 2027)will silently get 2026 holidays. At minimum, theyearparameter should influence the generated dates, or the function should throw a "not implemented" error.🤖 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-fns/src/business/workingHoursUntil.ts` around lines 21 - 25, Update preloadHolidays so cached holiday dates are derived from its year argument rather than always using 2026 values; alternatively, make the function explicitly throw a “not implemented” error instead of populating incorrect data. Preserve the existing cacheKey behavior and async interface.packages/tempo-fns/src/calendar/getISOWeekOfYear.ts (1)
1-17: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== files ==" git ls-files | rg '^(CONTRIBUTING\.md|packages/tempo-fns/src/calendar/getISOWeekOfYear\.ts|packages/tempo-fns/src/.*tempo\.class\.ts|packages/tempo-fns/src/.*international\.library\.ts|package\.json)$' echo echo "== contributing ==" if [ -f CONTRIBUTING.md ]; then rg -n -C 2 'Temporal polyfill|polyfill|withCalendar|iso8601' CONTRIBUTING.md fi echo echo "== getISOWeekOfYear ==" cat -n packages/tempo-fns/src/calendar/getISOWeekOfYear.ts echo echo "== call sites ==" rg -n -C 3 'getISOWeekOfYear\(' packages/tempo-fns/src echo echo "== relevant Temporal usages ==" rg -n -C 3 'withCalendar\(|toDateTime\(|dayOfWeek|since\(' packages/tempo-fns/src echo echo "== package deps ==" if [ -f package.json ]; then rg -n -C 2 '"`@js-temporal/polyfill`"|temporal' package.json fiRepository: magmacomputing/magma
Length of output: 3912
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== package files ==" git ls-files | rg '(^|/)package\.json$|(^|/)tempo\.class\.ts$|(^|/)international\.library\.ts$|(^|/)getISOWeekOfYear\.ts$' echo echo "== root package.json ==" cat -n package.json echo echo "== package-level package.json files ==" for f in $(git ls-files | rg '(^|/)package\.json$'); do if [ "$f" != "package.json" ]; then echo "--- $f ---" cat -n "$f" echo fi done echo echo "== tempo.class.ts / international.library.ts ==" for f in $(git ls-files | rg 'tempo\.class\.ts$|international\.library\.ts$'); do echo "--- $f ---" cat -n "$f" echo doneRepository: magmacomputing/magma
Length of output: 50377
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== tempo-fns source around getISOWeekOfYear ==" sed -n '1,80p' packages/tempo-fns/src/calendar/getISOWeekOfYear.ts echo echo "== tempo source callsite around getISOWeekOfYear ==" sed -n '300,360p' packages/tempo/src/tempo.class.ts echo echo "== library source around getISOWeekOfYear ==" sed -n '138,175p' packages/library/src/common/international.library.ts echo echo "== tempo-fns package config ==" cat -n packages/tempo-fns/package.jsonRepository: magmacomputing/magma
Length of output: 7212
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== tempo-fns build files ==" git ls-files packages/tempo-fns | rg 'rollup|vite|tsconfig|package\.json|src/' echo echo "== search for temporal polyfill handling ==" rg -n -C 2 '`@js-temporal/polyfill`|temporal\.polyfill|external|rollup' packages/tempo-fns packages/library packages/tempo echo echo "== tempo-fns tsconfig ==" for f in $(git ls-files packages/tempo-fns | rg 'tsconfig.*\.json$'); do echo "--- $f ---" cat -n "$f" echo done echo echo "== tempo-fns rollup config candidates ==" for f in $(git ls-files packages/tempo-fns | rg 'rollup.*\.(js|mjs|ts)$'); do echo "--- $f ---" cat -n "$f" echo doneRepository: magmacomputing/magma
Length of output: 40968
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== call sites for getISOWeekOfYear ==" rg -n -C 4 'getISOWeekOfYear\(' packages/tempo packages/library packages/tempo-fns echo echo "== adjacent ISO week code in tempo-fns ==" sed -n '1,220p' packages/tempo-fns/src/calendar/getISOWeekOfYear.ts echo echo "== any PlainDate/ZonedDateTime coercion around tempo callsites ==" rg -n -C 4 'weekOfYear|yearOfWeek|getISOWeekOfYear|toPlainDate\(|toDateTime\(' packages/tempo/src packages/library/src packages/tempo-fns/srcRepository: magmacomputing/magma
Length of output: 44877
Remove the polyfill import and normalize the input before computing ISO weeks.
packages/tempo-fnsis meant to stay polyfill-free, so importing@js-temporal/polyfillturns this entry point into a runtime dependency that consumers must install themselves.targetThursday.since(week1Thursday)mixesTemporal.ZonedDateTimewithTemporal.PlainDate, so anyZonedDateTimeinput will throw here.- Convert to a
PlainDatefirst andwithCalendar('iso8601')before readingdayOfWeek/year; otherwise non-ISO calendars can return the wrong week/year.🤖 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-fns/src/calendar/getISOWeekOfYear.ts` around lines 1 - 17, Remove the `@js-temporal/polyfill` import and use the input’s Temporal methods to normalize zdt to an ISO-calendar PlainDate before the ISO-week calculations. Update getISOWeekOfYear so both ZonedDateTime and PlainDate inputs are converted consistently via toPlainDate when available and withCalendar('iso8601'), then perform targetThursday, week1Thursday, and since calculations entirely on PlainDate values.packages/tempo-fns/src/scheduling/cron.ts (1)
3-9: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Implement the cron traversal instead of returning the current value.
nextCronandprevCronignorepatternand returnthis, so callers never move to a matching occurrence despite the augmented API contract. Implement the calculation or keep these methods out of the public API until the behavior exists.🤖 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-fns/src/scheduling/cron.ts` around lines 3 - 9, Implement actual cron traversal in nextCron and prevCron using the provided pattern, returning the next and previous matching Tempo occurrence respectively instead of this. If traversal cannot be implemented yet, remove nextCron and prevCron from the public API rather than exposing no-op methods.packages/tempo-fns/src/timezone/getHemisphere.ts (2)
9-10: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Equator case loses Southern-hemisphere fallback for non-DST zones.
The upstream contract checks
southPrefixes(e.g.,'Australia','Antarctica','America/Buenos_Aires') whenjan === julto correctly classify Southern-hemisphere zones that don't observe DST. This implementation returns'E'for alljan === julcases, misclassifying zones likeAustralia/Perthas equatorial.🤖 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-fns/src/timezone/getHemisphere.ts` around lines 9 - 10, Update the jan === jul branch in getHemisphere to check the existing southPrefixes classification before returning 'E', returning 'S' for matching Southern-hemisphere zones such as Australia/Perth and 'E' otherwise; preserve the current jan < jul and jan > jul behavior.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Hemisphere logic is inverted — N and S are swapped.
jan < julis equivalent tojul > jan. The upstream contract inpackages/library/src/common/international.library.tsmapsjul > jan→'north'(Northern hemisphere has longer July offset due to summer DST). However, this implementation returns'S'whenjan < jul, which is the opposite. This will misclassify every timezone that observes DST.🐛 Proposed fix
if (jan === jul) return 'E'; - return (jan < jul) ? 'S' : 'N'; + return (jan < jul) ? 'N' : 'S';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (jan === jul) return 'E'; return (jan < jul) ? 'N' : 'S';🤖 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-fns/src/timezone/getHemisphere.ts` around lines 9 - 10, Correct the return mapping in getHemisphere: when jan < jul, return 'N', and when jan > jul, return 'S'. Preserve the existing 'E' result when jan === jul.
11-13: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Catch fallback returns
'N'for invalid timezones — misleading and inconsistent with upstream.The upstream
getHemisphereininternational.library.tsreturnsundefinedon error, signalling "unknown." Returning'N'silently classifies invalid timezones as Northern, which propagates incorrect hemisphere data to downstream consumers (e.g.,support.init.tspopulatesconfig.spherefrom this value). Consider returningundefinedand widening the return type, or rethrowing.🛡️ Proposed fix
-export function getHemisphere(timeZone: string = Intl.DateTimeFormat().resolvedOptions().timeZone): Hemisphere { +export function getHemisphere(timeZone: string = Intl.DateTimeFormat().resolvedOptions().timeZone): Hemisphere | undefined { try { const { jan, jul } = getOffsets(timeZone); if (jan === jul) return 'E'; return (jan < jul) ? 'N' : 'S'; } catch { - return 'N'; + return undefined; } }🤖 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-fns/src/timezone/getHemisphere.ts` around lines 11 - 13, Update the catch fallback in getHemisphere to return undefined for invalid timezones instead of 'N', and widen its return type accordingly so unknown hemisphere values propagate correctly to consumers such as config.sphere.packages/tempo-fns/src/timezone/isDST.ts (1)
6-8: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
timeZoneparameter is ignored whendateis a string, and bare date strings will throw.When
dateis a string,Temporal.ZonedDateTime.from(date)is called without thetimeZoneargument.ZonedDateTime.fromrequires timezone info embedded in the string (e.g.,"2026-07-01T00:00:00[America/New_York]"); a bare ISO date like"2026-07-01"will throw. ThetimeZoneparameter is silently discarded in this branch.🛡️ Proposed fix
export function isDST(date?: Temporal.ZonedDateTime | string, timeZone: string = Intl.DateTimeFormat().resolvedOptions().timeZone) { const zdt = typeof date === 'string' - ? Temporal.ZonedDateTime.from(date) + ? Temporal.PlainDate.from(date).toZonedDateTime(timeZone) : (date ?? Temporal.Now.zonedDateTimeISO(timeZone)); const { jan, jul } = getOffsets(zdt.timeZoneId, zdt.year);This assumes the string is a plain date. If mixed-format support is needed, a try/catch with
ZonedDateTime.fromfallback could be added.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const zdt = typeof date === 'string' ? Temporal.PlainDate.from(date).toZonedDateTime(timeZone) : (date ?? Temporal.Now.zonedDateTimeISO(timeZone));🤖 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-fns/src/timezone/isDST.ts` around lines 6 - 8, Update the string-handling branch in the isDST implementation so the provided timeZone is applied when converting a plain date string, rather than calling Temporal.ZonedDateTime.from without it. Ensure bare ISO date strings are accepted while preserving the existing behavior for ZonedDateTime inputs and omitted dates.packages/tempo-fns/test/isFirstDayOfMonth.test.ts (1)
1-2: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Import path is incorrect — will fail to resolve.
The import
'../src/isFirstDayOfMonth.js'resolves topackages/tempo-fns/src/isFirstDayOfMonth.js, but the source file is atpackages/tempo-fns/src/calendar/isFirstDayOfMonth.ts. The barrelindex.tsconfirms the export path is'./calendar/isFirstDayOfMonth.js'.🐛 Proposed fix
-import { isFirstDayOfMonth } from '../src/isFirstDayOfMonth.js'; +import { isFirstDayOfMonth } from '../src/calendar/isFirstDayOfMonth.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { isFirstDayOfMonth } from '../src/calendar/isFirstDayOfMonth.js'; import { Tempo } from '`@magmacomputing/tempo`';🤖 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-fns/test/isFirstDayOfMonth.test.ts` around lines 1 - 2, Update the isFirstDayOfMonth import in the test to use the calendar subdirectory path matching the source location and index.ts barrel export, while leaving the Tempo import unchanged.packages/tempo-fns/test/isSameFiscalQuarter.test.ts (1)
1-2: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Import path is incorrect — will fail to resolve.
The import
'../src/isSameFiscalQuarter.js'resolves topackages/tempo-fns/src/isSameFiscalQuarter.js, but the source file is atpackages/tempo-fns/src/business/isSameFiscalQuarter.ts. The barrelindex.tsconfirms the export path is'./business/isSameFiscalQuarter.js'.🐛 Proposed fix
-import { isSameFiscalQuarter } from '../src/isSameFiscalQuarter.js'; +import { isSameFiscalQuarter } from '../src/business/isSameFiscalQuarter.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { isSameFiscalQuarter } from '../src/business/isSameFiscalQuarter.js'; import { Tempo } from '`@magmacomputing/tempo`';🤖 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-fns/test/isSameFiscalQuarter.test.ts` around lines 1 - 2, Update the isSameFiscalQuarter import in the test to use the business barrel path, matching the source location and index.ts export: import it from the business subpath rather than directly from src.packages/tempo/doc/1-getting-started/installation.md (2)
9-12: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant doc section with line numbers. sed -n '1,80p' packages/tempo/doc/1-getting-started/installation.md | cat -n # Look for other Temporal support claims in the docs. rg -n "Temporal|harmony-temporal|polyfill|Safari|iOS|Node.js" packages/tempo/doc -SRepository: magmacomputing/magma
Length of output: 30127
🌐 Web query:
Node.js 26.1 Temporal unflagged support build documentation Temporal support release notes Safari iOS Temporal caniuse💡 Result:
As of July 12, 2026, the Temporal API is enabled by default in Node.js, starting with the release of version 26.0.0 on May 5, 2026 [1][2][3]. There is no "unflagging" process required for recent Node.js versions, as the API is available out-of-the-box [2]. Regarding browser support, the Temporal API is currently not supported in Safari on iOS [4][5][6]. According to current tracking data, browser implementation for the Temporal API remains blocked in Safari (WebKit issue 223166) [4], and it is not available in any current iOS Safari versions [7][8][5].
Citations:
- 1: https://nodejs.org/en/blog/release/v26.0.0
- 2: nodejs/node#62526
- 3: nodejs/node@1eaf8a9
- 4: https://web-platform-dx.github.io/web-features-explorer/features/temporal/
- 5: https://caniuse.com/mdn-javascript_builtins_temporal_now
- 6: https://caniuse.com/mdn-javascript_builtins_temporal_plaintime
- 7: https://caniuse.com/mdn-javascript_builtins_temporal
- 8: https://caniuse.com/temporal
Refresh the Temporal support matrix. The Node.js guidance is stale: recent Node 26 releases ship
Temporalby default, so the--harmony-temporalpath and “future release” wording should go. Also call out Safari/iOS as polyfill-only instead of implying broad native browser coverage.🤖 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/doc/1-getting-started/installation.md` around lines 9 - 12, Update the Temporal support description to reflect native support in Node.js 26 and remove the outdated future-release and --harmony-temporal guidance. Revise the browser support wording to explicitly identify Safari and iOS as polyfill-only, while retaining the caniuse reference and the recommendation that Tempo does not bundle a polyfill by default.Source: MCP tools
167-184: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Import the Temporal polyfill in the static-CDN example.
Unlike the Global Bundle and Smart CDN examples, this production-oriented snippet never imports
@js-temporal/polyfill.packages/library/src/common/temporal.polyfill.tsthrows during import whenglobalThis.Temporalis absent, so this setup fails on unsupported browsers before plugin registration. Add the polyfill mapping/import or label this example as native-Temporal-only.Proposed fix
"imports": { + "`@js-temporal/polyfill`": "https://cdn.jsdelivr.net/npm/@js-temporal/polyfill@0.5.1/dist/index.esm.js", "`@magmacomputing/tempo`": "https://cdn.jsdelivr.net/npm/@magmacomputing/tempo@3/dist/tempo.index.js",+import '`@js-temporal/polyfill`'; import { Tempo } from '`@magmacomputing/tempo`';🤖 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/doc/1-getting-started/installation.md` around lines 167 - 184, Update the Static CDNs import-map example to include the `@js-temporal/polyfill` mapping and import, matching the existing Global Bundle and Smart CDN examples so browsers without native Temporal can load the setup successfully.packages/tempo/doc/2-core-concepts/tempo.format.md (1)
194-194: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the relevant file and find the documented API plus implementation/tests. git ls-files 'packages/tempo/doc/2-core-concepts/tempo.format.md' \ 'packages/tempo/**' \ | sed -n '1,200p' echo '--- SEARCH: withTimeZone / withCalendar / epochMilliseconds / format fallback ---' rg -n "withTimeZone|withCalendar|epochMilliseconds|Intl.DateTimeFormatOptions|swallow|retry formatting|timeZone|calendar" packages/tempo -S echo '--- OUTLINE: likely implementation files if found ---' fd -a ".*" packages/tempo | sed -n '1,200p'Repository: magmacomputing/magma
Length of output: 50376
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the documentation section around the cited lines. sed -n '170,220p' packages/tempo/doc/2-core-concepts/tempo.format.mdRepository: magmacomputing/magma
Length of output: 2356
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find tests or examples that exercise format() fallback behavior. rg -n "format\\(.*timeZone|withTimeZone|withCalendar|epochMilliseconds|DateTimeFormatOptions|zdt\\.epochMilliseconds" packages/tempo -S --glob '!**/doc/**'Repository: magmacomputing/magma
Length of output: 9231
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the doc section around the cited lines. cat -n packages/tempo/doc/2-core-concepts/tempo.format.md | sed -n '185,205p'Repository: magmacomputing/magma
Length of output: 1400
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the format implementation around the fallback path. cat -n packages/tempo/src/module/module.format.ts | sed -n '1,180p'Repository: magmacomputing/magma
Length of output: 8500
🏁 Script executed:
#!/bin/bash set -euo pipefail # Look for format tests that cover Intl options, timezone/calendar overrides, or fallback behavior. rg -n "format\\(|Intl.DateTimeFormatOptions|withTimeZone|withCalendar|timeZone:|calendar:" packages/tempo/test -S --glob '!**/doc/**'Repository: magmacomputing/magma
Length of output: 32778
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the formatting module and nearby helper logic. sed -n '1,170p' packages/tempo/src/module/module.format.tsRepository: magmacomputing/magma
Length of output: 6780
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect tests more specifically. rg -n "Intl.DateTimeFormatOptions|toLocaleString|format\\(.*timeZone|format\\(.*calendar|invalid.*timeZone|invalid.*calendar|fallback" packages/tempo/test -SRepository: magmacomputing/magma
Length of output: 1294
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the core fallback section in the formatter implementation. sed -n '70,125p' packages/tempo/src/module/module.format.tsRepository: magmacomputing/magma
Length of output: 1927
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the format implementation around the fallback path and any tests for it. sed -n '1,180p' packages/tempo/src/module/module.format.ts rg -n "Intl.DateTimeFormatOptions|withTimeZone|withCalendar|toLocaleString|timeZone:|calendar:|fallback" packages/tempo/test -SRepository: magmacomputing/magma
Length of output: 21134
Avoid swallowing
withTimeZone/withCalendarerrors. The formatter catches those exceptions and falls back tozdt.epochMilliseconds, which can hide invalid usertimeZone/calendarinput and return the wrong date. Narrow this to the Temporal/Intl mismatch case, or rethrow invalid options.🤖 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/doc/2-core-concepts/tempo.format.md` at line 194, Update the formatter behavior described around the direct Intl.DateTimeFormatOptions path so withTimeZone and withCalendar errors are not swallowed indiscriminately. Only apply the epochMilliseconds fallback for the known Temporal/Intl mismatch case; rethrow errors caused by invalid user timeZone or calendar options so invalid input cannot produce an incorrect formatted date.packages/tempo/doc/4-advanced-reference/tempo.ticker.md (1)
7-11: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Link to the actual registry for installation and license tokens.
The runtime contract in
packages/tempo/src/plugin/extend/extend.ticker.ts:5-27directs users to the official registry for the premium plugin and license key, while this page labels a documentation-hosting URL as the registry and token-generation endpoint. Align this link with the runtime error message, or clearly separate plugin documentation from registry/token access.🤖 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/doc/4-advanced-reference/tempo.ticker.md` around lines 7 - 11, Update the “Explore the Tempo Ticker Plugin” link in the ticker documentation to point to the official Tempo Registry used by the runtime contract in extend.ticker.ts, so users reach the correct installation and license-token endpoint; if retaining the current documentation URL, separate it clearly from the registry and token-access link.packages/tempo/doc/4-advanced-reference/tempo.weekday.md (1)
19-26: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the documented
<=semantics.
packages/tempo/src/engine/engine.lexer.ts:95-168implements<=asperiod < offset ? -adjust : -(adjust - 1). For example, on Monday,<=Wedresolves to the previous Wednesday, not the Wednesday immediately preceding-or-equal to Monday as this table implies. Document the actual boundary behavior and add an example showing the difference between<and<=.🧰 Tools
🪛 LanguageTool
[style] ~23-~23: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...-Tue| Tuesday of last week | |<=| Prior to |<=Wed` | The Wednesday immediately p...(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~24-~24: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ...preceding (or equal to) today | |<| Prior to (strict) |<Thu| The Thursday strict...(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~25-~25: Consider using “after”
Context: ...ursday strictly before today | |>=| Subsequent to |>=Fri| The Friday immediately foll...(SUBSEQUENT_TO)
[style] ~26-~26: Consider using “after”
Context: ...following (or equal to) today | |>| Subsequent to (strict) |>Sat| The Saturday strict...(SUBSEQUENT_TO)
🤖 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/doc/4-advanced-reference/tempo.weekday.md` around lines 19 - 26, The weekday modifier table incorrectly describes <= as preceding-or-equal to today. Update the <= entry in the weekday reference to document its implemented boundary behavior, and add an example contrasting < with <=, using the lexer semantics as the source of truth.packages/tempo/src/plugin/plugin.util.ts (2)
95-99: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
defineModule/definePluginmutate the caller's plugin object instead of returning a copy.Both use
Object.assign(module/plugin, {type: ...}), mutating the input in place. Contrast withdefineTerm(term.util.ts), which spreads into a new object ({...term, type: 'term', ...}) beforedeepFreeze-ing it. This inconsistency means:
- Callers holding a reference to their original plugin literal will silently see it mutated with a new
typefield.- If a caller ever passes an already-frozen object (a real possibility given this PR's broader move toward freezing runtime objects),
Object.assignwill throwTypeErrorbecause adding a new own property to a non-extensible object is disallowed.Aligning these with
defineTerm's spread-then-freeze pattern removes both risks cheaply.🛡️ Suggested fix (non-mutating)
export function defineModule<T extends Plugin<TempoType>>(module: T): T & { type: 'module' } { - const result = Object.assign(module, { type: 'module' } as const); + const result = { ...module, type: 'module' as const }; registerPlugin(result); return result; }export function definePlugin<T extends Plugin<TempoType>>(plugin: T): T & { type: 'plugin' } { - const result = Object.assign(plugin, { type: 'plugin' } as const); + const result = { ...plugin, type: 'plugin' as const }; registerPlugin(result); return result; }Also applies to: 179-183
🤖 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/plugin.util.ts` around lines 95 - 99, Update defineModule and definePlugin to create new plugin objects via spreading the input before adding their respective type fields, rather than mutating the caller’s object with Object.assign. Register and return the new object while preserving the existing type-specific behavior and compatibility with frozen inputs.
215-265: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n "defineNamespace" packages -g '*.test.ts'Repository: magmacomputing/magma
Length of output: 158
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== defineNamespace locations ==" rg -n "defineNamespace|Object\.freeze\(this\)|`#setLazy`|Reflect\.isExtensible|Object\.defineProperty\(this, cacheKey|Object\.defineProperty\(target, name" packages/tempo/src -g '*.ts' echo echo "== plugin.util.ts excerpt ==" sed -n '200,290p' packages/tempo/src/plugin/plugin.util.ts echo echo "== tempo.class.ts excerpt around freeze/lazy helper ==" sed -n '1240,1325p' packages/tempo/src/tempo.class.tsRepository: magmacomputing/magma
Length of output: 7562
🏁 Script executed:
#!/bin/bash set -euo pipefail # Narrowly inspect any tests that reference namespace plugin behavior. rg -n "namespace|defineNamespace|plugin.util" packages/tempo/src packages/tempo -g '*.test.ts' -g '*.spec.ts' || trueRepository: magmacomputing/magma
Length of output: 540
Avoid caching namespace proxies on frozen instances —
defineNamespacewritescacheKeyontothison first access, butTempoinstances are frozen, so the first namespace lookup throws aTypeError. Use the sameReflect.isExtensibleguard as#setLazy, or move the cache to aWeakMap.🤖 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/plugin.util.ts` around lines 215 - 265, Update the namespace getter in defineNamespace so it does not attempt to define cacheKey on non-extensible or frozen Tempo instances. Reuse the Reflect.isExtensible guard and behavior established by `#setLazy`, or store proxies in a WeakMap fallback, while preserving proxy caching for extensible instances.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/tempo/src/plugin/plugin.util.ts (2)
235-269: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRegistered plugin has wrong
$PluginTypediscriminator
defineNamespacecallsdefinePlugin(line 235), which registers a clone tagged with$PluginType = 'plugin'. Line 268 then creates a second clone tagged'namespace'and returns it — but this object is never registered. The plugin stored in the discovery database and extensions list therefore carries the wrong type discriminator ('plugin'instead of'namespace'), which will break any discovery or introspection logic that distinguishes namespaces from regular plugins.🔧 Fix: register directly with the correct type tag
const pluginName = `${namespaceStr}Namespace`; - const plugin = definePlugin({ + const plugin = { name: pluginName, version: config.version ?? TEMPO_VERSION, install(this: TempoType, TempoClass: TempoType, options?: any) { @@ -263,11 +263,11 @@ }); } - }); - - const result = { ...plugin, [sym.$PluginType]: 'namespace' }; - return result as unknown as Plugin<TempoType>; + }; + + const result = { ...plugin, [sym.$PluginType]: 'namespace' }; + registerPlugin(result); + return result as unknown as Plugin<TempoType>; }🤖 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/plugin.util.ts` around lines 235 - 269, Update defineNamespace to register the namespace object with the $PluginType discriminator set to 'namespace' directly, rather than calling definePlugin and retagging an unregistered clone afterward. Preserve the existing namespace installation behavior and ensure the object stored in discovery and extension registries is the same correctly tagged object returned by defineNamespace.
241-260: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winInfinite recursion when
config.nameis a SymbolWhen
config.nameis a symbol,cacheKeyis set toconfig.nameitself (line 241). The getter is defined onTempoClass.prototypefor that same symbol key (line 239). On the first access,this[cacheKey]on line 242 triggers the prototype getter again — there is no own property yet to shadow it — causing unbounded recursion and a stack overflow. String namespaces are unaffected because their cacheKey (_${name}) differs fromconfig.name.🔧 Fix: use a string-prefixed cache key for all namespace types
get() { - const cacheKey = isSymbol(config.name) ? config.name : `_${String(config.name)}`; + const cacheKey = `__ns_${isSymbol(config.name) ? config.name.description : String(config.name)}`; if (!this[cacheKey]) {🤖 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/plugin.util.ts` around lines 241 - 260, Update the cacheKey computation in the namespace getter to always use a string-prefixed key, including when config.name is a Symbol, so it cannot match the getter’s prototype property and recurse. Preserve the existing name distinction while ensuring the cached proxy is stored and returned through the non-conflicting key.
🧹 Nitpick comments (2)
packages/tempo/.vitepress/config.ts (1)
88-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrailing comma inconsistency in "Utility Library" sidebar items.
Line 96 has a trailing comma after the last array element, while all other sidebar
itemsarrays (lines 43, 54, 65, 77, 86, 105, 113) do not. This is valid TypeScript but stylistically inconsistent within the same config object.♻️ Proposed fix
{ text: 'Advanced Promises (Pledge)', link: '/doc/6-utility-library/tempo.pledge' }, - ] + ]🤖 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/.vitepress/config.ts` around lines 88 - 97, Remove the trailing comma after the final “Advanced Promises (Pledge)” entry in the “Utility Library” items array, matching the formatting used by the surrounding sidebar item arrays.packages/tempo/src/plugin/plugin.util.ts (1)
249-258: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffUncached proxy on non-extensible instances creates a new proxy per access
When
Reflect.isExtensible(this)is false, the proxy is returned directly without caching (line 257). Every subsequent property access re-enters the getter and constructs a freshtarget+proxypair. Consider aWeakMap-based cache keyed on the instance to avoid this repeated allocation regardless of extensibility.🤖 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/plugin.util.ts` around lines 249 - 258, Update the proxy getter around Reflect.isExtensible(this) to cache proxies in a WeakMap keyed by the instance, so non-extensible instances reuse the same target/proxy pair across accesses. Preserve the existing property-definition cache for extensible instances and return the WeakMap-cached proxy for non-extensible instances.
🤖 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/tempo-fns/src/business/workingHoursUntil.ts`:
- Around line 16-22: The unfinished SLA API must not remain publicly exported:
either implement workingHoursUntil with the complete SLAOptions-aware contract
and authoritative region holiday loading, including preloadHolidays, or
remove/privatize the public exports until that implementation is ready. Update
workingHoursUntil, SLAOptions, and preloadHolidays consistently so the signature
accepts the options contract and holiday data is not limited to January 1 and
December 25.
In `@packages/tempo/CHANGELOG.md`:
- Around line 32-34: Remove the duplicated “Plugin Argument Parsing” and
“Registry Merge Contracts” Fixed entries from every historical changelog release
identified in the comment, preserving them only under the 3.8.0 entry. Do not
alter each release’s existing Fixed sections or other changelog content.
---
Outside diff comments:
In `@packages/tempo/src/plugin/plugin.util.ts`:
- Around line 235-269: Update defineNamespace to register the namespace object
with the $PluginType discriminator set to 'namespace' directly, rather than
calling definePlugin and retagging an unregistered clone afterward. Preserve the
existing namespace installation behavior and ensure the object stored in
discovery and extension registries is the same correctly tagged object returned
by defineNamespace.
- Around line 241-260: Update the cacheKey computation in the namespace getter
to always use a string-prefixed key, including when config.name is a Symbol, so
it cannot match the getter’s prototype property and recurse. Preserve the
existing name distinction while ensuring the cached proxy is stored and returned
through the non-conflicting key.
---
Nitpick comments:
In `@packages/tempo/.vitepress/config.ts`:
- Around line 88-97: Remove the trailing comma after the final “Advanced
Promises (Pledge)” entry in the “Utility Library” items array, matching the
formatting used by the surrounding sidebar item arrays.
In `@packages/tempo/src/plugin/plugin.util.ts`:
- Around line 249-258: Update the proxy getter around Reflect.isExtensible(this)
to cache proxies in a WeakMap keyed by the instance, so non-extensible instances
reuse the same target/proxy pair across accesses. Preserve the existing
property-definition cache for extensible instances and return the WeakMap-cached
proxy for non-extensible instances.
🪄 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: ac8d9afe-147e-48d6-8c9c-a9d3cac22860
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
packages/library/package.jsonpackages/library/src/common/international.library.tspackages/tempo-fns/doc/README.mdpackages/tempo-fns/doc/functions/index.mdpackages/tempo-fns/doc/preview.htmlpackages/tempo-fns/doc/support.mdpackages/tempo-fns/src/business/workingHoursUntil.tspackages/tempo-fns/src/calendar/getISOWeekOfYear.tspackages/tempo-fns/src/index.tspackages/tempo-fns/src/scheduling/Interval.tspackages/tempo-fns/src/timezone/getHemisphere.tspackages/tempo-fns/src/timezone/isDST.tspackages/tempo-fns/test/isFirstDayOfMonth.test.tspackages/tempo-fns/test/isSameFiscalQuarter.test.tspackages/tempo-fns/test/setup.tspackages/tempo-fns/vitest.config.tspackages/tempo/.vitepress/config.tspackages/tempo/CHANGELOG.mdpackages/tempo/doc/1-getting-started/installation.mdpackages/tempo/doc/2-core-concepts/tempo.duration.mdpackages/tempo/doc/2-core-concepts/tempo.format.mdpackages/tempo/doc/2-core-concepts/tempo.parse.mdpackages/tempo/doc/3-extending-tempo/tempo.modularity.mdpackages/tempo/doc/3-extending-tempo/tempo.namespace.mdpackages/tempo/doc/4-advanced-reference/tempo.shorthand.mdpackages/tempo/doc/4-advanced-reference/tempo.ticker.mdpackages/tempo/doc/4-advanced-reference/tempo.weekday.mdpackages/tempo/doc/5-architecture-and-internals/architecture.mdpackages/tempo/doc/5-architecture-and-internals/tempo.benchmarks.mdpackages/tempo/doc/6-utility-library/tempo.enumerators.mdpackages/tempo/doc/7-ecosystem/comparison.mdpackages/tempo/doc/7-ecosystem/extending-temporal.mdpackages/tempo/doc/8-project-and-support/releases/v3.x.mdpackages/tempo/package.jsonpackages/tempo/rollup.config.jspackages/tempo/src/plugin/plugin.type.tspackages/tempo/src/plugin/plugin.util.tspackages/tempo/src/plugin/term/term.util.tspackages/tempo/src/support/support.symbol.tspackages/tempo/src/tempo.class.tspackages/tempo/test/core/bootstrap.test.ts
💤 Files with no reviewable changes (3)
- packages/tempo/package.json
- packages/tempo/doc/6-utility-library/tempo.enumerators.md
- packages/tempo/rollup.config.js
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/tempo-fns/doc/support.md
- packages/tempo-fns/doc/README.md
- packages/tempo-fns/doc/functions/index.md
- packages/tempo-fns/src/timezone/isDST.ts
- packages/tempo/test/core/bootstrap.test.ts
- packages/tempo-fns/test/isSameFiscalQuarter.test.ts
- packages/tempo/doc/7-ecosystem/comparison.md
- packages/tempo-fns/src/timezone/getHemisphere.ts
- packages/tempo-fns/vitest.config.ts
- packages/library/src/common/international.library.ts
- packages/tempo-fns/src/scheduling/Interval.ts
- packages/tempo-fns/src/calendar/getISOWeekOfYear.ts
- packages/tempo/src/plugin/plugin.type.ts
- packages/tempo/doc/8-project-and-support/releases/v3.x.md
- packages/tempo/doc/3-extending-tempo/tempo.namespace.md
- packages/tempo/doc/3-extending-tempo/tempo.modularity.md
- packages/tempo/doc/2-core-concepts/tempo.format.md
- packages/tempo/doc/7-ecosystem/extending-temporal.md
- packages/tempo/doc/5-architecture-and-internals/tempo.benchmarks.md
- packages/tempo/src/plugin/term/term.util.ts
- packages/tempo/doc/5-architecture-and-internals/architecture.md
- packages/tempo-fns/doc/preview.html
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tempo-fns/src/business/workingHoursUntil.ts (1)
89-93: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReplace the hardcoded holiday list
preloadHolidaysstill caches only January 1 and December 25 for every region, so holiday-based working-hour calculations will be wrong until this is backed by real region-specific holiday data or an API fetch.🤖 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-fns/src/business/workingHoursUntil.ts` around lines 89 - 93, Update preloadHolidays to populate remoteHolidaysCache with real region-specific holidays for the requested region and year, using the existing holiday data source or API-fetch mechanism rather than the hardcoded January 1 and December 25 values. Preserve the cache key format and asynchronous preload behavior.
🧹 Nitpick comments (5)
packages/tempo-fns/src/index.ts (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider combining exports from the same module for consistency.
Line 3 combines multiple exports from
workingHoursUntil.jsin a single statement. Applying the same pattern tonextCronandprevCronwould improve consistency.♻️ Proposed refactor
-export { nextCron } from './scheduling/cron.js'; -export { prevCron } from './scheduling/cron.js'; +export { nextCron, prevCron } from './scheduling/cron.js';🤖 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-fns/src/index.ts` around lines 10 - 11, Combine the separate nextCron and prevCron exports from scheduling/cron.js into a single export statement, matching the existing grouped-export style used in index.ts.packages/tempo-fns/test/scheduling/interval.test.ts (1)
49-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
intersectionandunionreturningnull.Both methods return
nullwhen intervals don't overlap (and don't abut forunion), but no test covers these paths. Adding these cases would improve coverage of the null-return branches.🧪 Suggested additional tests
+ it('intersection should return null for non-overlapping intervals', () => { + const i1 = new Interval(new Tempo('2026-07-01'), new Tempo('2026-07-10')); + const i2 = new Interval(new Tempo('2026-07-15'), new Tempo('2026-07-20')); + expect(i1.intersection(i2)).toBeNull(); + }); + + it('union should return null for non-overlapping, non-abutting intervals', () => { + const i1 = new Interval(new Tempo('2026-07-01'), new Tempo('2026-07-10')); + const i2 = new Interval(new Tempo('2026-07-15'), new Tempo('2026-07-20')); + expect(i1.union(i2)).toBeNull(); + });🤖 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-fns/test/scheduling/interval.test.ts` around lines 49 - 67, Add test cases alongside the existing intersection and union tests for non-overlapping intervals, asserting that Interval.intersection returns null and Interval.union returns null when the intervals neither overlap nor abut. Use clearly separated date ranges and preserve the existing overlapping behavior tests.packages/tempo-fns/src/scheduling/Interval.ts (2)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent type parameters across
Intervalmethods.
overlapsandabutsacceptInterval<any>, butintersectionandunionrequireInterval<T>. This prevents operations between intervals of different concrete types (e.g.,Interval<Tempo>.intersection(Interval<Temporal.Instant>)). Consider usingInterval<any>consistently for comparison methods, or introduce a common base type parameter.♻️ Proposed fix
- intersection(other: Interval<T>): Interval<T> | null { + intersection(other: Interval<any>): Interval<T> | null { if (!this.overlaps(other)) return null; const maxStart = getNs(this.start) > getNs(other.start) ? this.start : other.start; const minEnd = getNs(this.end) < getNs(other.end) ? this.end : other.end; return new Interval<T>(maxStart, minEnd); } /** Returns the union of this interval and another, or null if they do not overlap/abut */ - union(other: Interval<T>): Interval<T> | null { + union(other: Interval<any>): Interval<T> | null { if (!this.overlaps(other) && !this.abuts(other)) return null; const minStart = getNs(this.start) < getNs(other.start) ? this.start : other.start; const maxEnd = getNs(this.end) > getNs(other.end) ? this.end : other.end; return new Interval<T>(minStart, maxEnd); }Note:
maxStart/minEndwould need a cast sinceother.start/other.endwould beany— alternatively, keepInterval<T>and document that mixed-type operations are unsupported.Also applies to: 49-49, 54-54, 62-62
🤖 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-fns/src/scheduling/Interval.ts` at line 44, Make the Interval operation type parameters consistent across overlaps, abuts, intersection, and union. Update intersection and union to accept intervals of differing concrete types like the comparison methods, and add the necessary casts around maxStart/minEnd when accessing other.start or other.end; preserve existing same-type behavior.
5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider stronger typing for
getNs.The
anyparameter type bypasses TypeScript's type checking. A type guard or function overload would provide better safety while still handling the union type.♻️ Suggested improvement
-function getNs(point: any): bigint { - if (point?.epoch?.ns !== undefined) return point.epoch.ns as bigint; - if (point?.epochNanoseconds !== undefined) return point.epochNanoseconds as bigint; - throw new TypeError('Invalid TemporalPoint: missing epoch.ns or epochNanoseconds'); +function getNs(point: TemporalPoint): bigint { + if ('epoch' in point && point.epoch.ns !== undefined) return point.epoch.ns; + if ('epochNanoseconds' in point) return point.epochNanoseconds; + throw new TypeError('Invalid TemporalPoint: missing epoch.ns or epochNanoseconds'); }🤖 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-fns/src/scheduling/Interval.ts` around lines 5 - 9, Strengthen the parameter typing of getNs by replacing any with an explicit Temporal point union or a type guard covering epoch.ns and epochNanoseconds. Preserve the existing precedence, bigint conversions, and TypeError behavior for unsupported points while ensuring TypeScript validates the accessed properties.packages/tempo-fns/src/business/workingHoursUntil.ts (1)
75-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLines 75-76 are dead code — the clamps never trigger.
actualStartis only ever set tocurrent(which is> dayWorkStartby the check on line 66) or left asdayWorkStart(equal). SoactualStart < dayWorkStartis always false. The same logic applies toactualEndon line 76. These lines can be safely removed.♻️ Remove redundant clamps
- if (actualStart.epochNanoseconds < dayWorkStart.epochNanoseconds) actualStart = dayWorkStart; - if (actualEnd.epochNanoseconds > dayWorkEnd.epochNanoseconds) actualEnd = dayWorkEnd; - if (actualStart.epochNanoseconds < actualEnd.epochNanoseconds) {🤖 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-fns/src/business/workingHoursUntil.ts` around lines 75 - 76, Remove the redundant start and end clamp conditionals from the working-hours calculation, specifically the assignments comparing actualStart with dayWorkStart and actualEnd with dayWorkEnd. Preserve the existing actualStart and actualEnd initialization logic and all surrounding working-hours behavior.
🤖 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/tempo-fns/doc/functions/scheduling/index.md`:
- Around line 22-48: Update the Interval documentation and example to match the
actual Interval implementation: document the (start, end) constructor and its
set-operation methods instead of the durationObj, factory, next(), and iterator
API. Use the implemented Interval symbols and behavior as the source of truth,
keeping the scheduling example consistent with that API.
In `@packages/tempo-fns/src/business/workingHoursUntil.ts`:
- Around line 35-36: Validate the resolved startWorkHour and endWorkHour values
in workingHoursUntil so startHour must be strictly less than endHour; reject
invalid configurations at runtime before calculating dayWorkStart/dayWorkEnd,
while preserving the existing behavior for valid ranges.
In `@packages/tempo-fns/src/scheduling/cron.ts`:
- Around line 22-34: Validate the parsed step in parseCronField before entering
the step-based for loop, requiring it to be a positive integer. Reject zero,
negative, and invalid step values so */0 and */-1 cannot reach the loop or hang
processing; preserve existing behavior for valid positive steps.
- Around line 35-39: Validate that parsed range starts do not exceed their ends
in both the plain range branch and the step-range branch of the cron parser.
Reject reversed ranges such as 5-3 and 10-5/2 using the parser’s existing
invalid-pattern behavior, rather than allowing an empty allowed set.
In `@packages/tempo-fns/src/scheduling/README.md`:
- Around line 22-48: Update the Interval section in README.md to match the
implemented Interval API: document the (start, end) constructor and
set-operation methods such as contains, overlaps, abuts, intersection, and
union. Remove or replace the incompatible next(), iterator, duration-object, and
factory example; if iterator behavior is planned separately, label it explicitly
as a distinct future API.
---
Outside diff comments:
In `@packages/tempo-fns/src/business/workingHoursUntil.ts`:
- Around line 89-93: Update preloadHolidays to populate remoteHolidaysCache with
real region-specific holidays for the requested region and year, using the
existing holiday data source or API-fetch mechanism rather than the hardcoded
January 1 and December 25 values. Preserve the cache key format and asynchronous
preload behavior.
---
Nitpick comments:
In `@packages/tempo-fns/src/business/workingHoursUntil.ts`:
- Around line 75-76: Remove the redundant start and end clamp conditionals from
the working-hours calculation, specifically the assignments comparing
actualStart with dayWorkStart and actualEnd with dayWorkEnd. Preserve the
existing actualStart and actualEnd initialization logic and all surrounding
working-hours behavior.
In `@packages/tempo-fns/src/index.ts`:
- Around line 10-11: Combine the separate nextCron and prevCron exports from
scheduling/cron.js into a single export statement, matching the existing
grouped-export style used in index.ts.
In `@packages/tempo-fns/src/scheduling/Interval.ts`:
- Line 44: Make the Interval operation type parameters consistent across
overlaps, abuts, intersection, and union. Update intersection and union to
accept intervals of differing concrete types like the comparison methods, and
add the necessary casts around maxStart/minEnd when accessing other.start or
other.end; preserve existing same-type behavior.
- Around line 5-9: Strengthen the parameter typing of getNs by replacing any
with an explicit Temporal point union or a type guard covering epoch.ns and
epochNanoseconds. Preserve the existing precedence, bigint conversions, and
TypeError behavior for unsupported points while ensuring TypeScript validates
the accessed properties.
In `@packages/tempo-fns/test/scheduling/interval.test.ts`:
- Around line 49-67: Add test cases alongside the existing intersection and
union tests for non-overlapping intervals, asserting that Interval.intersection
returns null and Interval.union returns null when the intervals neither overlap
nor abut. Use clearly separated date ranges and preserve the existing
overlapping behavior tests.
🪄 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: 1a848f70-2bb8-48fd-a815-05decb2d5b01
📒 Files selected for processing (28)
packages/tempo-fns/CHANGELOG.mdpackages/tempo-fns/LICENSEpackages/tempo-fns/README.mdpackages/tempo-fns/doc/functions/business/index.mdpackages/tempo-fns/doc/functions/calendar/index.mdpackages/tempo-fns/doc/functions/duration/index.mdpackages/tempo-fns/doc/functions/scheduling/Interval.mdpackages/tempo-fns/doc/functions/scheduling/cron.mdpackages/tempo-fns/doc/functions/scheduling/index.mdpackages/tempo-fns/doc/functions/timezone/index.mdpackages/tempo-fns/package.jsonpackages/tempo-fns/src/business/README.mdpackages/tempo-fns/src/business/workingHoursUntil.tspackages/tempo-fns/src/calendar/README.mdpackages/tempo-fns/src/duration/README.mdpackages/tempo-fns/src/index.tspackages/tempo-fns/src/scheduling/Interval.tspackages/tempo-fns/src/scheduling/README.mdpackages/tempo-fns/src/scheduling/cron.tspackages/tempo-fns/src/timezone/README.mdpackages/tempo-fns/test/business/isSameFiscalQuarter.test.tspackages/tempo-fns/test/business/workingHoursUntil.test.tspackages/tempo-fns/test/calendar/isFirstDayOfMonth.test.tspackages/tempo-fns/test/scheduling/cron.test.tspackages/tempo-fns/test/scheduling/interval.test.tspackages/tempo/.vitepress/config.tspackages/tempo/CHANGELOG.mdpackages/tempo/src/plugin/plugin.util.ts
💤 Files with no reviewable changes (1)
- packages/tempo/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/tempo-fns/package.json
- packages/tempo/.vitepress/config.ts
- packages/tempo/src/plugin/plugin.util.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tempo/src/plugin/license/license.validator.ts (1)
39-44: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTag function-valued premium plugins too.
Line 39 returns before the new marker assignment, so function plugins are returned without
sym.$PluginTypeand bypass namespace classification.Proposed fix
- if (isFunction(plugin)) return throwLicense as unknown as T; + if (isFunction(plugin)) { + (throwLicense as any)[sym.$PluginType] = 'plugin'; + return throwLicense as unknown as T; + }🤖 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/license/license.validator.ts` around lines 39 - 44, Update the function-plugin branch in the license validation flow so function-valued premium plugins receive the sym.$PluginType marker before being returned. Preserve the existing throwLicense behavior while ensuring the early return no longer bypasses namespace classification.
🧹 Nitpick comments (1)
packages/tempo/rollup.config.js (1)
100-117: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTypeScript diagnostics from
transpileModuleare silently discarded.Only
.outputTextis used;ts.transpileModule's.diagnosticsare never inspected/logged. Syntax errors in.tsfiles would silently pass through to the bundler and fail later (or worse, produce subtly broken output) instead of failing the build with a clear TS error.♻️ Suggested fix
transform(code, id) { if (!id.endsWith('.ts')) return null; - return { - code: ts.transpileModule(code, { - compilerOptions: { ... } - }).outputText, - map: null - }; + const result = ts.transpileModule(code, { compilerOptions: { ... } }); + if (result.diagnostics?.length) { + this.error(ts.flattenDiagnosticMessageText(result.diagnostics[0].messageText, '\n')); + } + return { code: result.outputText, map: null }; }🤖 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/rollup.config.js` around lines 100 - 117, Update the manual-typescript plugin’s transform method to inspect the diagnostics returned by ts.transpileModule and fail the build with clear formatted TypeScript errors when diagnostics are present, before returning outputText. Preserve the existing transpilation options and successful output flow.
🤖 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/tempo-fns/src/scheduling/README.md`:
- Around line 25-34: Update the Interval constructor declaration to accept
nullable boundaries by documenting both start and end parameters as T | null,
while preserving the existing generic type and method signatures.
In `@packages/tempo-fns/src/support/fetch.ts`:
- Around line 12-13: Update the request flow around fetch and its status check
to use try/finally, ensuring clearTimeout(timeoutId) executes for successful
responses and all rejected requests, including network errors, caller aborts,
and timeouts.
---
Outside diff comments:
In `@packages/tempo/src/plugin/license/license.validator.ts`:
- Around line 39-44: Update the function-plugin branch in the license validation
flow so function-valued premium plugins receive the sym.$PluginType marker
before being returned. Preserve the existing throwLicense behavior while
ensuring the early return no longer bypasses namespace classification.
---
Nitpick comments:
In `@packages/tempo/rollup.config.js`:
- Around line 100-117: Update the manual-typescript plugin’s transform method to
inspect the diagnostics returned by ts.transpileModule and fail the build with
clear formatted TypeScript errors when diagnostics are present, before returning
outputText. Preserve the existing transpilation options and successful output
flow.
🪄 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: bdde697f-3501-4e83-9412-08e59d8c4cd4
📒 Files selected for processing (25)
packages/tempo-fns/CHANGELOG.mdpackages/tempo-fns/doc/functions/scheduling/index.mdpackages/tempo-fns/src/business/workingHoursUntil.tspackages/tempo-fns/src/calendar/README.mdpackages/tempo-fns/src/calendar/getISOWeekOfYear.tspackages/tempo-fns/src/calendar/getPublicHolidays.tspackages/tempo-fns/src/calendar/isFirstDayOfMonth.tspackages/tempo-fns/src/index.tspackages/tempo-fns/src/scheduling/README.mdpackages/tempo-fns/src/scheduling/cron.tspackages/tempo-fns/src/support/README.mdpackages/tempo-fns/src/support/fetch.tspackages/tempo-fns/src/support/intl.tspackages/tempo-fns/src/timezone/isDST.tspackages/tempo/.vitepress/config.tspackages/tempo/CHANGELOG.mdpackages/tempo/doc/2-core-concepts/tempo.interval.mdpackages/tempo/doc/8-project-and-support/releases/v3.x.mdpackages/tempo/rollup.config.jspackages/tempo/src/core.index.tspackages/tempo/src/interval.class.tspackages/tempo/src/plugin/license/license.validator.tspackages/tempo/src/tempo.class.tspackages/tempo/src/tempo.index.tspackages/tempo/test/core/interval.test.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/tempo-fns/CHANGELOG.md
- packages/tempo-fns/src/calendar/README.md
- packages/tempo-fns/src/timezone/isDST.ts
- packages/tempo-fns/src/index.ts
- packages/tempo-fns/src/calendar/isFirstDayOfMonth.ts
- packages/tempo/doc/8-project-and-support/releases/v3.x.md
- packages/tempo/.vitepress/config.ts
- packages/tempo-fns/src/scheduling/cron.ts
- packages/tempo/CHANGELOG.md
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tempo-fns/src/calendar/getPublicHolidays.ts (1)
34-39: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCheck
response.okbefore caching API response data.If the Nager.Date API returns a non-200 status (e.g., 404 for an unsupported country code),
response.json()may still succeed with an error body, which is then cached as valid holiday data. Subsequent calls return the stale error payload. Add a status check before parsing and caching.🛡️ Proposed fix
const response = await fetchWithTimeout(`${holidayURL}/${year}/${countryCode}`, 2000); + if (!response.ok) { + throw new Error(`[tempo-fns] Holiday API returned ${response.status} for ${countryCode}/${year}`); + } const data = await response.json();🤖 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-fns/src/calendar/getPublicHolidays.ts` around lines 34 - 39, Update the response handling in getPublicHolidays to validate response.ok immediately after fetchWithTimeout and before response.json(). Only parse and cache the response data through holidaysCache.set when the status is successful; preserve the existing error-handling behavior for unsuccessful responses without caching their payload.
🧹 Nitpick comments (1)
packages/tempo-fns/src/support/temporal.ts (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
@ts-expect-errorover@ts-ignore.
@ts-ignoresuppresses all errors on the next line, which could hide future issues.@ts-expect-erroris safer — it fails the build if the error is ever resolved (e.g., when TypeScript adds nativeTemporaltolib).♻️ Proposed refactor
- // `@ts-ignore` - Check for global Temporal + // `@ts-expect-error` - Temporal is not yet in TS lib defs; remove when it is if (typeof Temporal !== 'undefined') return Temporal;🤖 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-fns/src/support/temporal.ts` around lines 10 - 11, Replace the `@ts-ignore` directive above the global Temporal check in the Temporal support logic with `@ts-expect-error`, preserving the existing runtime check and return behavior.
🤖 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/tempo-fns/src/calendar/getPublicHolidays.ts`:
- Around line 34-36: Validate the countryCode derived in getPublicHolidays
before constructing the holiday URL or calling fetchWithTimeout, accepting only
exactly two uppercase A-Z letters and rejecting malformed regions such as path
traversal input. Preserve the existing URL construction and fetch flow for valid
country codes.
---
Outside diff comments:
In `@packages/tempo-fns/src/calendar/getPublicHolidays.ts`:
- Around line 34-39: Update the response handling in getPublicHolidays to
validate response.ok immediately after fetchWithTimeout and before
response.json(). Only parse and cache the response data through
holidaysCache.set when the status is successful; preserve the existing
error-handling behavior for unsuccessful responses without caching their
payload.
---
Nitpick comments:
In `@packages/tempo-fns/src/support/temporal.ts`:
- Around line 10-11: Replace the `@ts-ignore` directive above the global Temporal
check in the Temporal support logic with `@ts-expect-error`, preserving the
existing runtime check and return 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: b073b97a-9b81-41ba-8d2c-e25b12404232
📒 Files selected for processing (15)
packages/tempo-fns/package.jsonpackages/tempo-fns/src/business/isSameFiscalQuarter.tspackages/tempo-fns/src/business/workingHoursUntil.tspackages/tempo-fns/src/calendar/getISOWeekOfYear.tspackages/tempo-fns/src/calendar/getPublicHolidays.tspackages/tempo-fns/src/scheduling/README.mdpackages/tempo-fns/src/support/README.mdpackages/tempo-fns/src/support/assert.tspackages/tempo-fns/src/support/fetch.tspackages/tempo-fns/src/support/index.tspackages/tempo-fns/src/support/temporal.tspackages/tempo-fns/src/timezone/getOffsets.tspackages/tempo-fns/src/timezone/isDST.tspackages/tempo/rollup.config.jspackages/tempo/src/plugin/license/license.validator.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/tempo-fns/src/timezone/getOffsets.ts
- packages/tempo-fns/src/scheduling/README.md
- packages/tempo-fns/src/business/isSameFiscalQuarter.ts
- packages/tempo-fns/src/support/fetch.ts
- packages/tempo-fns/package.json
- packages/tempo-fns/src/timezone/isDST.ts
- packages/tempo-fns/src/calendar/getISOWeekOfYear.ts
- packages/tempo/rollup.config.js
|
@coderabbitai please review |
|
✅ Action performedReview finished.
|
1 participant
Summary by CodeRabbit
Intervalprimitive for working with time ranges.Tempo.extendplugin/term typing.@magmacomputing/tempo-fnsTemporal utilities (calendar, timezone, duration, scheduling/cron, and business helpers), including a browser/global bundle.tempo-fnsdocumentation and refreshed Tempo docs navigation and release pages.Pledgesupport for environments withoutPromise.withResolvers.tempoCLI scaffolding command.