Feature/modular parse#16
Merged
magmacomputing merged 11 commits intomainfrom Apr 19, 2026
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 36 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughExtracts Tempo's parser into a new ParseModule, refactors module/interpreter registration and resolution (ensureModule, interpret, defineInterpreterModule), delegates Tempo parsing to ParseModule with a fallback, adjusts Mutate/Duration/Format module shapes and registrations, moves the Temporal polyfill, updates package exports/imports/tests/docs/UI, and adds MIT license and export changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Tempo.instance / static
participant Interpret as plugin.util.interpret
participant Registry as _MODULES / REGISTRY
participant Parse as ParseModule.ParseEngine
participant Temporal as Temporal API
Caller->>Interpret: interpret(this, "ParseModule", "parse", true, ...)
Interpret->>Registry: resolve "ParseModule"
Registry-->>Interpret: ParseEngine
Interpret->>Parse: ParseEngine.parse(...)
Parse->>Temporal: compose/build ZonedDateTime
Parse-->>Interpret: ZonedDateTime | null
Interpret-->>Caller: result (or null -> `#fallbackParse`)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: magmacomputing <6935496+magmacomputing@users.noreply.github.com>
Contributor
Resolved and merged |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/tempo/doc/tempo.serializers.md (1)
17-45:⚠️ Potential issue | 🟠 MajorUpdate import paths in code examples:
stringify,objectify, andcloneifyare exported from@magmacomputing/library, not@magmacomputing/tempo/library.The documented examples import these serializers from
@magmacomputing/tempo/library, butpackages/tempo/src/library.index.tsonly re-exportsPledge,enumify(+Enumtype), andproxify. The serializer functions are actually exported from@magmacomputing/library/common.index.ts. Developers copying these examples will hit module-resolution errors. Update all code snippets to import from@magmacomputing/libraryinstead:import { stringify, objectify, cloneify } from '@magmacomputing/library';Alternatively, add these re-exports to
packages/tempo/src/library.index.tsif the intent is to expose them through the Tempo library subpath.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/doc/tempo.serializers.md` around lines 17 - 45, The docs import for the serializer helpers is incorrect: update the example imports so stringify, objectify, and cloneify are imported from `@magmacomputing/library` (e.g. import { stringify, objectify, cloneify } from '@magmacomputing/library') or alternatively add re-exports for stringify, objectify, and cloneify in the Tempo library index so those symbols become available via the Tempo subpath (ensure the examples and library.index re-exports consistently reference the same source).packages/tempo/src/tempo.class.ts (1)
43-46:⚠️ Potential issue | 🟡 MinorRemove the unused
throwMutateModuleNotLoadedhelper function.With
.add()/.set()now routed throughinterpret(this, 'MutateModule', ...), the missing-module error is handled insideinterpret→ensureModule. This function is no longer called anywhere in the codebase.Cleanup
-/** helper to throw error if MutateModule is missing */ -const throwMutateModuleNotLoaded = () => { - throw new Error('Tempo MutateModule not loaded. Did you forget to Tempo.extend(MutateModule)?'); -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/src/tempo.class.ts` around lines 43 - 46, Remove the unused helper by deleting the throwMutateModuleNotLoaded function declaration; since .add()/.set() now call interpret(this, 'MutateModule', ...) and missing-module errors are handled by interpret → ensureModule, there are no remaining references to throwMutateModuleNotLoaded, so remove its const declaration and any related comments to clean up tempo.class.ts.
🧹 Nitpick comments (8)
packages/tempo/plan/standalone_parse.md (2)
54-56: Consider varying the sentence structure for better readability.Three consecutive sentences begin with "Phase", which can feel repetitive. Consider restructuring for smoother flow.
As per coding guidelines, static analysis tools flagged: "Three successive sentences begin with the same word."
✍️ Proposed rewording
## 5. Conclusion Implementing a standalone `parse` function is **Highly Feasible**. -- **Phase 1**: Refactor `ParseEngine` to be stateless (accepting config/state as arguments). -- **Phase 2**: Export a wrapper `parse()` function from `#tempo/parse`. -- **Phase 3**: Update `Tempo.class` to delegate its `#parse` logic to this shared stateless engine. +The implementation can proceed in three phases: +1. **Refactor `ParseEngine`** to be stateless (accepting config/state as arguments). +2. **Export a wrapper** `parse()` function from `#tempo/parse`. +3. **Update `Tempo.class`** to delegate its `#parse` logic to this shared stateless engine.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/plan/standalone_parse.md` around lines 54 - 56, The three sequential sentences starting with "Phase" are repetitive; rephrase them to vary sentence openings and improve flow by e.g. turning one into a clause or reordering: mention refactoring ParseEngine to be stateless (accept config/state as arguments), then describe exporting a wrapper parse() from `#tempo/parse`, and finally explain updating Tempo.class to delegate its `#parse` logic to that shared stateless engine; keep the same actions and symbols (ParseEngine, parse(), Tempo.class, `#parse`) but alter sentence structure so not all three lines begin with "Phase".
45-45: Consider escaping the hash symbol in the Mermaid diagram.The
#parsein the node label might not render correctly in all Mermaid renderers. Consider using an escape sequence or replacing with a word like "internal parse" for better compatibility.📊 Proposed fix for Mermaid compatibility
- B --> D[Tempo Class `#parse`] + B --> D[Tempo Class - parse method]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/plan/standalone_parse.md` at line 45, The Mermaid node label "Tempo Class `#parse`" (seen in the line `B --> D[Tempo Class `#parse`]`) may break rendering due to the unescaped '#' — update that node label to either escape the hash (e.g., replace '#' with its escape sequence) or rename it to a safe identifier such as "Tempo Class internal parse" or "Tempo Class parse" so the diagram renders consistently across Mermaid renderers; ensure you update the exact node text within the `B --> D[...]` token so references to the Tempo Class parse node remain correct.packages/tempo/doc/tempo.library.md (1)
58-59: Nit: trailing---at EOF with no following content renders as a dangling horizontal rule. Harmless, but consider removing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/doc/tempo.library.md` around lines 58 - 59, Remove the trailing markdown horizontal rule at EOF (the dangling line with only '---') in tempo.library.md so the file does not end with an extra horizontal rule; simply delete that final '---' line.packages/tempo/package.json (1)
161-161: New./bundleexport has notypesentry.Every other entry under
exportspairsimportwithtypes, but"./bundle": "./dist/tempo.bundle.js"is a bare string. TS consumers usingmoduleResolution: "bundler"/node16won't get typings for@magmacomputing/tempo/bundle. If that's intentional (e.g., bundle is a side-effect-only entry not meant to be imported for types), consider making it explicit; otherwise add atypesfield.Suggested change
- "./bundle": "./dist/tempo.bundle.js" + "./bundle": { + "types": "./dist/tempo.bundle.d.ts", + "import": "./dist/tempo.bundle.js" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/package.json` at line 161, The "./bundle" export in package.json under "exports" is a bare string and lacks a paired "types" entry, so TypeScript consumers won't get typings for `@magmacomputing/tempo/bundle`; update the "./bundle" export to the object form and add a "types" field pointing to the bundle declaration (e.g., "./dist/tempo.bundle.d.ts"), or if the bundle is intentionally side-effect-only, make that explicit by keeping the object form with "types": null and an explanatory comment; locate the "./bundle" export in package.json and change it accordingly.packages/tempo/src/plugin/module/module.duration.ts (1)
162-162: StaticTempo.durationbypasses module validation error UX.
interpret(TempoClass, 'DurationModule', 'toDuration', false, input)passessilent=false, which is correct for surfacing "module not loaded" diagnostics. However, sincetoDurationis a pure conversion (nothisdependency on a Tempo instance), consider returning a meaningful fallback or documenting thatTempo.duration(...)throws when the module hasn't been installed — the current behaviour differs slightly from instance methods since there's no Tempo instance context in the error message.Non-blocking; flag if the static API is documented as always-available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/src/plugin/module/module.duration.ts` at line 162, The static assignment (TempoClass as any).duration = (input: any) => interpret(TempoClass, 'DurationModule', 'toDuration', false, input); can surface a raw "module not loaded" diagnostic without Tempo instance context; fix by catching that failure and rethrowing a clearer, instance-like error or by instantiating a minimal Tempo context before calling interpret so the error message matches instance-method UX: modify the (TempoClass as any).duration implementation to call interpret('DurationModule','toDuration',...) inside a try/catch and, on error, throw a new Error with a descriptive message (or create a Tempo instance and forward the call) so users get the same helpful diagnostic as instance methods.packages/tempo/src/plugin/module/module.parse.ts (1)
284-292: Dead type inparseMatchsignature.
value: string | number | (() => string)suggests a thunk is supported, butvalue.toString()on a function returns its source code — guaranteed to produce empty/irrelevant match groups. The only caller (Line 247) passestrim: string. Recommend narrowing the type tostring | numberto avoid misleading future callers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/src/plugin/module/module.parse.ts` around lines 284 - 292, The parseMatch signature wrongly allows a thunk; change the parameter type for value from "string | number | (() => string)" to "string | number" in the parseMatch method so callers (e.g., where trim string is passed) aren't misled; update any related type annotations or overloads that reference parseMatch's value parameter and keep the implementation using value.toString(), returning groups as t.Groups unchanged.packages/tempo/src/plugin/module/module.mutate.ts (1)
96-103: Unusedmoduleparameter inparseInner.The third parameter
moduleis never referenced inside the helper. At the only call site (Line 121) thetermvalue is passed through asmodule, which is misleading. Consider removing it or renaming it for clarity if a future consumer plans to use it.♻️ Suggested cleanup
- const parseInner = (input: any, anchor?: any, module?: any) => { + const parseInner = (input: any, anchor?: any) => { const res = (this.constructor as any).from(input, { ...this.config, anchor }); if (res.isValid) { state.matches.push(...res.parse.result); return res.toDateTime(); } return undefined; };And at Line 121:
- const res = parseInner(offset, currZdt, term); + const res = parseInner(offset, currZdt);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/src/plugin/module/module.mutate.ts` around lines 96 - 103, The helper parseInner has an unused third parameter named module (passed the caller's term) which is misleading; remove the unused parameter from parseInner's signature and all call sites (so parseInner only accepts input and optional anchor), or if you expect to use it later, rename it to a clearer name (e.g., term) consistently; update the call at the site that currently passes term to stop passing it, and ensure logic still uses (this.constructor as any).from(input, { ...this.config, anchor }) and state.matches.push(...res.parse.result) and res.toDateTime() unchanged.packages/tempo/src/tempo.class.ts (1)
970-995: Renamed-field + trivial accessor pattern adds indirection without benefit.Each of
#zdt/#errored/#parseDepth/#mutateDepth/#matches/#anchornow has a_-suffixed storage field plus a no-op getter/setter pair. There is no invariant enforcement, no caching, and no side-effect — just re-exposing the same storage under the original name. Unless this is in service of an upcoming change (e.g., serialization hook via@Serializable, or synchronization with external state), it's pure boilerplate that complicates reads and hides the actual storage from debuggers.If the intent is to keep the external name stable while letting
sym.$Internal()(Line 1017-1040) read/write viaself.#…, that works today without the rename because private fields are already accessible throughthis/selfin the same class. Consider reverting to plain private fields, or, if the rename is intentional preparation, adding a brief comment explaining the future use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/src/tempo.class.ts` around lines 970 - 995, The private fields like `#zdt_/`#errored_/#parseDepth_/#mutateDepth_/#matches_/#anchor_ are duplicated with no-op getters/setters (`#zdt/`#errored/#parseDepth/#mutateDepth/#matches/#anchor), adding unnecessary indirection; either revert to single private fields (remove the trailing-underscore storage names and delete the trivial getters/setters) so code accesses the real private fields directly, or keep the renamed storage but add a concise comment explaining the intended future use (e.g., serialization or sync with sym.$Internal()) and ensure sym.$Internal() accesses the chosen single storage; update all references to use the single chosen symbol set to eliminate the redundant pairs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tempo/doc/releases/index.md`:
- Line 11: The "Release Strategy" heading currently uses a third-level heading
(### Release Strategy) which breaks the top-level heading sequence; change that
heading to an H2 by replacing "### Release Strategy" with "## Release Strategy"
in the releases index so the markdown heading level increments correctly and
resolves markdownlint MD001.
In `@packages/tempo/doc/releases/v0.x.md`:
- Line 17: The "Documentation" line is missing the markdown heading prefix and
should be converted to a level-3 heading; update the line containing the literal
text "Documentation" in packages/tempo/doc/releases/v0.x.md to "###
Documentation" so it matches the other subsections and restores proper
ToC/outline rendering.
In `@packages/tempo/doc/tempo.api.md`:
- Line 27: The docs incorrectly list "Function" as an allowed DateTime type;
update the Tempo API doc by removing the "`Function`: A dynamic resolver (max
depth 5)" entry so it matches the DateTime type definition (export type DateTime
= string | number | bigint | Date | Tempo | TemporalObject |
Temporal.ZonedDateTimeLike | undefined | null) and the constructor logic where
isFunction is only used for options/plugin detection, not dateTime parsing;
ensure the remaining dateTime type descriptions remain unchanged.
In `@packages/tempo/doc/tempo.layout.md`:
- Around line 41-47: The Built-in Layouts table references undocumented snippet
tokens {sfx}, {brk}, {www}, {nbr}, and {afx}; update the docs by either adding
entries for these tokens to the "Available Snippets" table (matching
style/columns used for other snippets) or add a clear note under the Built-in
Layouts explaining where each of {sfx}, {brk}, {www}, {nbr}, and {afx} is
defined (and link or point to that definition), so readers can understand and
compose layouts like dtm, dmy, mdy, ymd, and unt.
In `@packages/tempo/package.json`:
- Around line 25-33: The sideEffects array in package.json uses glob patterns
that don't match actual TypeScript filenames; update the sideEffects entries
(the "sideEffects" property) to include patterns that match your real sources —
replace "**/*.module.ts" with a pattern like "**/module.*.ts" (or explicitly
list module.parse.ts, module.mutate.ts, module.duration.ts, module.format.ts)
and replace "**/*.polyfill.ts" with "**/*-polyfill.ts" (or explicitly list
temporal-polyfill.ts); ensure the final list includes both the JS outputs and
the corresponding TS source filenames so bundlers won't tree-shake required
side-effect files.
In `@packages/tempo/src/plugin/module/module.duration.ts`:
- Around line 155-159: The registration fallback creates a local map instead of
updating the global registry, so ensure the global canonical modules reference
is written back to globalThis[sym.$modules] (or use the nullish assignment
operator) inside install() before assigning DurationModule; specifically, ensure
you update (globalThis as any)[sym.$modules] to the modules container and then
set modules['DurationModule'] = duration so subsequent ensureModule/interpret
calls find the module.
In `@packages/tempo/src/plugin/module/module.mutate.ts`:
- Around line 206-209: The eager assignment _MODULES['MutateModule'] =
MutateEngine bypasses the clash check in defineInterpreterModule/install; remove
that top-level assignment and instead let defineInterpreterModule (the
install/registration path) register MutateEngine so the existing clash guard
(modules[name] !== logic) can detect duplicates; locate the occurrence of
"_MODULES['MutateModule'] = MutateEngine" near the MutateModule export and
delete it (or move equivalent registration into the defineInterpreterModule
install callback) so registration uses the normal registry flow.
In `@packages/tempo/src/plugin/module/module.parse.ts`:
- Around line 55-71: The code assumes current = getTermRange(this, list, false,
today) is defined and uses current.key and current.year; add guards after
computing current and currentIdx to handle undefined or not-found cases: check
if list is empty or current is falsy and throw the existing RangeError with the
same message, and also check if currentIdx === -1 and throw RangeError instead
of proceeding; additionally ensure cycleOffset calculation only runs when
currentIdx >= 0 and itemsPerCycle > 0 to avoid negative/invalid indices; update
the block inside the isNumeric branch (references: getRange, getTermRange, list,
current, currentIdx, isMultiCycle, itemsPerCycle, cycleOffset, targetIdx) to
perform these checks before accessing current.key or computing targetIdx.
- Around line 81-100: The code currently calls ParseEngine.conform(this, tempo,
today, isAnchored, resolvingKeys) before handling tempo values that are
term-strings or objects with '#' keys, which lets conform mutate
state.matches/state.config/state.parseDepth unnecessarily; move the early checks
that handle term-strings (isString(tempo) && tempo.startsWith('#') →
resolveTermValue/return) and unsupported '#'-key objects (isUndefined(term) &&
isObject(tempo) && Object.keys(tempo).some(k => k.startsWith('#')) →
logError/throw, and the no-terms case that calls (TempoClass as
any)[sym.$termError] → return) to run before invoking ParseEngine.conform so
those branches short-circuit without calling ParseEngine.conform or mutating
state. Ensure you reference ParseEngine.conform, tempo, resolveTermValue,
sym.$logError and sym.$termError when making the reorder.
In `@packages/tempo/src/plugin/plugin.util.ts`:
- Around line 41-72: The interpret function currently returns undefined when
ensureModule(t, module, silent) is false and no callable fallback is provided,
breaking chainable calls that pass a method name string; update interpret so
that when ensureModule returns false and methodOrFallback is not a function but
t?.config?.catch is true (or silent mode implies catch), it returns t (or a
shallow clone of t if mutability concerns exist) to preserve method-chaining
semantics instead of undefined; change the branch that now returns
isFunction(methodOrFallback) ? methodOrFallback.apply(...) : undefined to detect
the string-method case and return t/clone when t.config.catch is true, otherwise
keep existing behavior, and ensure you reference interpret, ensureModule,
methodOrFallback, and t.config.catch in your change.
In `@packages/tempo/src/tempo.class.ts`:
- Around line 1355-1356: Ticket: add() and set() can return undefined under
catch-mode; fix by guarding their return. In Tempo.add and Tempo.set (methods
calling interpret with 'MutateModule' and 'add'/'set'), if interpret(...) yields
undefined (when MutateModule is absent and config.catch is true), return this
(the current Tempo instance) to preserve fluent chaining instead of propagating
undefined; ensure you still call this.#ensureParsed() before interpret and
return the interpret result only when it's non-undefined.
- Around line 1398-1419: The current `#parse` conflates "module not loaded" with
"module loaded but parse errored" because interpret(...) returns undefined for
both; change `#parse` to first check module presence explicitly (e.g., call
ensureModule(this, 'ParseModule', true) or equivalent) and only call
`#fallbackParse` when the module is actually absent, and if the module is present
but interpret(...) returns undefined treat it as a parse error (reflect
ParseEngine.parse errored/state.errored) — log the error with Tempo.#dbg.error,
and either return undefined (when this.#local.config.catch === true) or throw a
descriptive Error so downstream code does not silently use a fabricated
ZonedDateTime; references: update `#parse`, leave `#fallbackParse` for the "module
absent" path, and handle interpret(this, 'ParseModule','parse', ...) and
ParseEngine.parse/state.errored accordingly.
In `@packages/tempo/vitest.config.ts`:
- Around line 29-30: TEST_DIST-only aliases leave source-mode Vitest unable to
resolve bare imports '#tempo/parse' and '#tempo/mutate'; update the alias logic
in vitest.config.ts that checks TEST_DIST so the non-dist branch also defines
aliases for '#tempo/parse' and '#tempo/mutate' pointing to the source module
files (plugin/module/module.parse.ts and plugin/module/module.mutate.ts) used by
packages/tempo/src/tempo.index.ts and tests, ensuring both dist and source modes
map those bare subpaths.
---
Outside diff comments:
In `@packages/tempo/doc/tempo.serializers.md`:
- Around line 17-45: The docs import for the serializer helpers is incorrect:
update the example imports so stringify, objectify, and cloneify are imported
from `@magmacomputing/library` (e.g. import { stringify, objectify, cloneify }
from '@magmacomputing/library') or alternatively add re-exports for stringify,
objectify, and cloneify in the Tempo library index so those symbols become
available via the Tempo subpath (ensure the examples and library.index
re-exports consistently reference the same source).
In `@packages/tempo/src/tempo.class.ts`:
- Around line 43-46: Remove the unused helper by deleting the
throwMutateModuleNotLoaded function declaration; since .add()/.set() now call
interpret(this, 'MutateModule', ...) and missing-module errors are handled by
interpret → ensureModule, there are no remaining references to
throwMutateModuleNotLoaded, so remove its const declaration and any related
comments to clean up tempo.class.ts.
---
Nitpick comments:
In `@packages/tempo/doc/tempo.library.md`:
- Around line 58-59: Remove the trailing markdown horizontal rule at EOF (the
dangling line with only '---') in tempo.library.md so the file does not end with
an extra horizontal rule; simply delete that final '---' line.
In `@packages/tempo/package.json`:
- Line 161: The "./bundle" export in package.json under "exports" is a bare
string and lacks a paired "types" entry, so TypeScript consumers won't get
typings for `@magmacomputing/tempo/bundle`; update the "./bundle" export to the
object form and add a "types" field pointing to the bundle declaration (e.g.,
"./dist/tempo.bundle.d.ts"), or if the bundle is intentionally side-effect-only,
make that explicit by keeping the object form with "types": null and an
explanatory comment; locate the "./bundle" export in package.json and change it
accordingly.
In `@packages/tempo/plan/standalone_parse.md`:
- Around line 54-56: The three sequential sentences starting with "Phase" are
repetitive; rephrase them to vary sentence openings and improve flow by e.g.
turning one into a clause or reordering: mention refactoring ParseEngine to be
stateless (accept config/state as arguments), then describe exporting a wrapper
parse() from `#tempo/parse`, and finally explain updating Tempo.class to delegate
its `#parse` logic to that shared stateless engine; keep the same actions and
symbols (ParseEngine, parse(), Tempo.class, `#parse`) but alter sentence structure
so not all three lines begin with "Phase".
- Line 45: The Mermaid node label "Tempo Class `#parse`" (seen in the line `B -->
D[Tempo Class `#parse`]`) may break rendering due to the unescaped '#' — update
that node label to either escape the hash (e.g., replace '#' with its escape
sequence) or rename it to a safe identifier such as "Tempo Class internal parse"
or "Tempo Class parse" so the diagram renders consistently across Mermaid
renderers; ensure you update the exact node text within the `B --> D[...]` token
so references to the Tempo Class parse node remain correct.
In `@packages/tempo/src/plugin/module/module.duration.ts`:
- Line 162: The static assignment (TempoClass as any).duration = (input: any) =>
interpret(TempoClass, 'DurationModule', 'toDuration', false, input); can surface
a raw "module not loaded" diagnostic without Tempo instance context; fix by
catching that failure and rethrowing a clearer, instance-like error or by
instantiating a minimal Tempo context before calling interpret so the error
message matches instance-method UX: modify the (TempoClass as any).duration
implementation to call interpret('DurationModule','toDuration',...) inside a
try/catch and, on error, throw a new Error with a descriptive message (or create
a Tempo instance and forward the call) so users get the same helpful diagnostic
as instance methods.
In `@packages/tempo/src/plugin/module/module.mutate.ts`:
- Around line 96-103: The helper parseInner has an unused third parameter named
module (passed the caller's term) which is misleading; remove the unused
parameter from parseInner's signature and all call sites (so parseInner only
accepts input and optional anchor), or if you expect to use it later, rename it
to a clearer name (e.g., term) consistently; update the call at the site that
currently passes term to stop passing it, and ensure logic still uses
(this.constructor as any).from(input, { ...this.config, anchor }) and
state.matches.push(...res.parse.result) and res.toDateTime() unchanged.
In `@packages/tempo/src/plugin/module/module.parse.ts`:
- Around line 284-292: The parseMatch signature wrongly allows a thunk; change
the parameter type for value from "string | number | (() => string)" to "string
| number" in the parseMatch method so callers (e.g., where trim string is
passed) aren't misled; update any related type annotations or overloads that
reference parseMatch's value parameter and keep the implementation using
value.toString(), returning groups as t.Groups unchanged.
In `@packages/tempo/src/tempo.class.ts`:
- Around line 970-995: The private fields like
`#zdt_/`#errored_/#parseDepth_/#mutateDepth_/#matches_/#anchor_ are duplicated
with no-op getters/setters
(`#zdt/`#errored/#parseDepth/#mutateDepth/#matches/#anchor), adding unnecessary
indirection; either revert to single private fields (remove the
trailing-underscore storage names and delete the trivial getters/setters) so
code accesses the real private fields directly, or keep the renamed storage but
add a concise comment explaining the intended future use (e.g., serialization or
sync with sym.$Internal()) and ensure sym.$Internal() accesses the chosen single
storage; update all references to use the single chosen symbol set to eliminate
the redundant pairs.
🪄 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: 0407a71d-c10a-48cc-8f9c-c5b6176055e1
📒 Files selected for processing (49)
packages/tempo/.vitepress/config.tspackages/tempo/LICENSEpackages/tempo/bin/setup.polyfill.tspackages/tempo/bin/temporal-polyfill.tspackages/tempo/doc/releases/index.mdpackages/tempo/doc/releases/v0.x.mdpackages/tempo/doc/releases/v1.x.mdpackages/tempo/doc/releases/v2.x.mdpackages/tempo/doc/tempo-vs-temporal.mdpackages/tempo/doc/tempo.api.mdpackages/tempo/doc/tempo.constructor.mdpackages/tempo/doc/tempo.enumerators.mdpackages/tempo/doc/tempo.layout.mdpackages/tempo/doc/tempo.library.mdpackages/tempo/doc/tempo.pattern.mdpackages/tempo/doc/tempo.pledge.mdpackages/tempo/doc/tempo.serializers.mdpackages/tempo/doc/tempo.term.mdpackages/tempo/doc/tempo.ticker.mdpackages/tempo/doc/tempo.types.mdpackages/tempo/package.jsonpackages/tempo/plan/standalone_parse.mdpackages/tempo/rollup.config.jspackages/tempo/src/plugin/module/module.duration.tspackages/tempo/src/plugin/module/module.format.tspackages/tempo/src/plugin/module/module.mutate.tspackages/tempo/src/plugin/module/module.parse.tspackages/tempo/src/plugin/plugin.util.tspackages/tempo/src/tempo.class.tspackages/tempo/src/tempo.entry.tspackages/tempo/src/tempo.index.tspackages/tempo/src/tempo.register.tspackages/tempo/src/tempo.symbol.tspackages/tempo/src/tempo.type.tspackages/tempo/src/tempo.util.tspackages/tempo/test/constructor.core.test.tspackages/tempo/test/duration.core.test.tspackages/tempo/test/duration.lazy.test.tspackages/tempo/test/fiscal-cycle.core.test.tspackages/tempo/test/instance.set.test.tspackages/tempo/test/lazy.test.tspackages/tempo/test/number-words.core.test.tspackages/tempo/test/proof.test.tspackages/tempo/test/tempo-import.test.tspackages/tempo/test/term-dispatch.core.test.tspackages/tempo/test/term-shorthand.test.tspackages/tempo/test/term_unified.test.tspackages/tempo/test/ticker.term.core.test.tspackages/tempo/vitest.config.ts
💤 Files with no reviewable changes (8)
- packages/tempo/bin/setup.polyfill.ts
- packages/tempo/doc/tempo-vs-temporal.md
- packages/tempo/doc/tempo.term.md
- packages/tempo/doc/tempo.ticker.md
- packages/tempo/rollup.config.js
- packages/tempo/doc/tempo.types.md
- packages/tempo/doc/tempo.pattern.md
- packages/tempo/doc/tempo.constructor.md
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/tempo/package.json (1)
25-33:⚠️ Potential issue | 🟡 MinorFix the
sideEffectssource globs to match the actual filenames.Line 30 and Line 31 still use suffix-style patterns, but the current files are prefix/hyphen-style names like
module.parse.tsandtemporal-polyfill.ts. That can cause required side-effect imports to be tree-shaken when bundling TS sources.Proposed package.json fix
"sideEffects": [ "**/temporal.polyfill.js", "**/module.parse.js", "**/module.mutate.js", "**/tempo.index.js", - "**/*.polyfill.ts", - "**/*.module.ts", + "**/*-polyfill.ts", + "**/module.*.ts", "src/tempo.index.ts" ],Verify with:
#!/bin/bash # Description: Compare sideEffects patterns with actual module/polyfill source filenames. jq '.sideEffects' packages/tempo/package.json fd -t f '(module\..*\.ts|.*polyfill.*\.ts)$' packages/tempo🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/package.json` around lines 25 - 33, The sideEffects array in package.json contains suffix-style globs that don't match actual source filenames; update the "sideEffects" entries (the sideEffects property and its elements like "**/temporal.polyfill.js", "**/module.parse.js", "**/module.mutate.js", "**/tempo.index.js") to use the real prefix/hyphen-style filenames and correct extensions (e.g., patterns that match "temporal-polyfill.*", "module-parse.*", "module-mutate.*", "tempo-index.*" and the TS sources such as "*.polyfill.ts" or "*.module.ts" as needed) so bundlers won't tree-shake required side-effect modules — keep the sideEffects key and replace the offending string patterns with globs that match the actual file names in the package.
🧹 Nitpick comments (1)
packages/tempo/package.json (1)
55-58: Add a public./parseexport for API consistency.
#tempo/parseis package-private. External users can access parse via@magmacomputing/tempo/module/parsethrough the wildcard pattern, but there is no top-level./parseexport like the ones available forduration,format, andmutate. Adding it would make the public API consistent.Proposed public export
"./format": { "types": "./dist/plugin/module/module.format.d.ts", "import": "./dist/plugin/module/module.format.js" }, + "./parse": { + "types": "./dist/plugin/module/module.parse.d.ts", + "import": "./dist/plugin/module/module.parse.js" + }, "./mutate": { "types": "./dist/plugin/module/module.mutate.d.ts", "import": "./dist/plugin/module/module.mutate.js" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/package.json` around lines 55 - 58, Add a public "./parse" export entry to the package exports so the API is consistent with existing top-level exports like "./duration", "./format", and "./mutate"; specifically, duplicate the "#tempo/parse" mapping under a "./parse" key with the same "development" value "./src/plugin/module/module.parse.ts" and the same "default" value "./dist/plugin/module/module.parse.js" so external consumers can import the parse module via the top-level "./parse" path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/tempo/package.json`:
- Around line 25-33: The sideEffects array in package.json contains suffix-style
globs that don't match actual source filenames; update the "sideEffects" entries
(the sideEffects property and its elements like "**/temporal.polyfill.js",
"**/module.parse.js", "**/module.mutate.js", "**/tempo.index.js") to use the
real prefix/hyphen-style filenames and correct extensions (e.g., patterns that
match "temporal-polyfill.*", "module-parse.*", "module-mutate.*",
"tempo-index.*" and the TS sources such as "*.polyfill.ts" or "*.module.ts" as
needed) so bundlers won't tree-shake required side-effect modules — keep the
sideEffects key and replace the offending string patterns with globs that match
the actual file names in the package.
---
Nitpick comments:
In `@packages/tempo/package.json`:
- Around line 55-58: Add a public "./parse" export entry to the package exports
so the API is consistent with existing top-level exports like "./duration",
"./format", and "./mutate"; specifically, duplicate the "#tempo/parse" mapping
under a "./parse" key with the same "development" value
"./src/plugin/module/module.parse.ts" and the same "default" value
"./dist/plugin/module/module.parse.js" so external consumers can import the
parse module via the top-level "./parse" path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0885f43e-70e9-4ba8-9a99-0fd95bc4598d
📒 Files selected for processing (1)
packages/tempo/package.json
…ma into feature/modular-parse
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
packages/tempo/index.md (2)
195-213: Consider adding accessibility features to the carousel.The carousel currently lacks accessibility features that would benefit keyboard users and screen reader users:
- No keyboard navigation (arrow keys to move between features)
- No ARIA attributes (
role="region",aria-label,aria-livefor auto-rotation)- No focus management for active cards
- No pause/play controls for users who need more time to read
While the current implementation works well for mouse users, consider adding:
- ARIA attributes for screen readers
- Keyboard navigation support (arrow keys, tab focus)
- Visible pause/play button for accessibility compliance (WCAG 2.2.2 - Pause, Stop, Hide)
📋 Example accessibility enhancements
<div class="tempo-carousel-container" role="region" aria-label="Tempo features" `@mouseenter`="isPaused = true" `@mouseleave`="isPaused = false" `@keydown`="handleKeydown"> <!-- Add pause/play button --> <button `@click`="isPaused = !isPaused" aria-label="Pause carousel"> {{ isPaused ? '▶️ Play' : '⏸️ Pause' }} </button> <div class="tempo-carousel-track" role="list" :aria-live="isPaused ? 'off' : 'polite'"> <!-- existing content --> </div> </div>And add keyboard handler:
function handleKeydown(e) { if (e.key === 'ArrowLeft') { activeIndex.value = Math.max(0, activeIndex.value - 1) } else if (e.key === 'ArrowRight') { activeIndex.value = Math.min(features.length - 1, activeIndex.value + 1) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/index.md` around lines 195 - 213, Add basic a11y to the carousel by adding ARIA roles/labels and keyboard/pause controls: update the tempo-carousel-container to include role="region" and aria-label="Tempo features", wire up `@keydown`="handleKeydown" and keep the existing `@mouseenter/`@mouseleave isPaused toggles; add a visible pause/play control bound to isPaused so users can stop auto-rotation; set tempo-carousel-track to role="list" and bind :aria-live to isPaused ? 'off' : 'polite'; implement a handleKeydown method that listens for ArrowLeft/ArrowRight to decrement/increment activeIndex (clamping to 0..displayFeatures.length-1) and ensure each tempo-feature-card is focusable (tabindex="0") and updates focus to the active card when activeIndex changes.
54-66: Verify HMR safeguard approach.The HMR safeguard temporarily replaces
registry.haswith a function that always returnsfalse, then restores the original method. While this appears to work, it's fragile because:
- If the original
hasmethod relied on being bound to the Map instance, restoring it asregistry.has = originalHasshould work when called asregistry.has(...)sincethiswill be bound toregistry.- However, if any code during the import process holds a reference to the unbound method, it could fail.
Consider documenting why this workaround is needed (what HMR behavior it's working around) or explore alternative approaches that don't involve runtime method replacement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/index.md` around lines 54 - 66, The HMR workaround replaces registry.has and restores it later but saves an unbound reference which can break code that expects the method to be bound; change the save to bind the original method (originalHas = registry.has.bind(registry)) before replacing it, keep the temporary replacement as registry.has = () => false, restore with registry.has = originalHas, and add an inline comment explaining this HMR workaround (use Symbol.for('$LibrarySerializerRegistry', registry, originalHas, registry.has) to locate the relevant symbols).packages/tempo/doc/releases/v2.x.md (1)
3-19: Formatting inconsistency between v2.2.3 and older entries.The v2.2.3 section uses plain paragraph lines without bullets (Lines 6-8, 11-12, 15-16, 19), whereas v2.2.2 and v2.1.x use
-bulleted lists under their subheadings. In rendered Markdown the v2.2.3 items will appear as unbroken paragraphs glued under each###, which looks different from the rest of the history page. Consider normalizing to-bullets for parity.📝 Proposed fix
### New Features - -Modular parse engine and standalone parsing support -Layout patterns with snippet-based customization -Added release notes pages and added MIT license +- Modular parse engine and standalone parsing support +- Layout patterns with snippet-based customization +- Added release notes pages and MIT license ### Bug Fixes - -Preserve parse results across cloned instances -Fixed timezone/calendar drift during complex mutations +- Preserve parse results across cloned instances +- Fixed timezone/calendar drift during complex mutations ### Documentation - -Reorganized docs navigation with Cookbook, Advanced Reference, Utility Library, Services & Support -Expanded API/constructor docs and custom-enum guidance +- Reorganized docs navigation with Cookbook, Advanced Reference, Utility Library, Services & Support +- Expanded API/constructor docs and custom-enum guidance ### Chores - -Improved module-loading diagnostics and general doc formatting cleanup +- Improved module-loading diagnostics and general doc formatting cleanup🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tempo/doc/releases/v2.x.md` around lines 3 - 19, The release notes under "## [v2.2.3] - 2026-04-20" use plain paragraphs for the sections "### New Features", "### Bug Fixes", "### Documentation", and "### Chores"; change each item line under these subheadings to a bulleted list (prefix each item with "- ") so the formatting matches older entries like v2.2.2 and v2.1.x and renders consistently in Markdown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tempo/index.md`:
- Around line 94-104: The catch block in startTicker creates an uncaptured
repeating timer via setInterval(fallback, 1000) causing leaked intervals; fix by
storing the interval ID in a module-scoped variable (e.g., fallbackIntervalId),
assign fallbackIntervalId = setInterval(fallback, 1000) after calling
fallback(), and clear any existing interval before creating a new one
(clearInterval(fallbackIntervalId)); also ensure
clearInterval(fallbackIntervalId) is called from onUnmounted and from
handleVisibility/stopTicker when the ticker is stopped so the fallback interval
is always cleaned up.
- Around line 72-75: The initPromise catch currently logs and rethrows, so every
startTicker() that awaits the same rejected initPromise will re-log; fix this by
marking the failure once and preventing subsequent awaits: in the catch where
initPromise is handled, set a sentinel (e.g., initFailed = true) and clear
initPromise (initPromise = undefined) before rethrowing; then update
startTicker() to check initFailed and return early (or skip awaiting
initPromise) to avoid re-logging the same rejection on visibility changes.
In `@packages/tempo/src/plugin/module/module.duration.ts`:
- Around line 162-170: The current static TempoClass.duration wrapper catches
errors and falls back to calling an instance method `(new
this()).duration(input)` which doesn't exist; remove the try/catch and fallback
so the function simply returns interpret(this, 'DurationModule', 'toDuration',
false, input) directly; update the TempoClass.duration factory (the function
assigned to (TempoClass as any).duration) to call interpret(...) without the
catch block so ensureModule/interpret can throw the proper "module not loaded"
diagnostic (references: TempoClass, duration, interpret, 'DurationModule').
In `@packages/tempo/src/plugin/module/module.mutate.ts`:
- Around line 170-183: The non-root "other args" branch returns a new instance
using this.config but omits ...state.options and the recursion guard
[sym.$mutateDepth]: state.mutateDepth, which drops per-instance options and
defeats the mutate-depth guard; update the constructor call in that branch (the
new (this.constructor as any)(args, { ...this.config, ...options, result:
state.matches, anchor: zdt, [sym.$errored]: state.errored })) to also spread
...state.options and set [sym.$mutateDepth]: state.mutateDepth so it matches the
other return paths (preserving state.options and the mutateDepth counter).
In `@packages/tempo/src/plugin/module/module.parse.ts`:
- Around line 460-463: The eager write `_MODULES['ParseModule'] = ParseEngine`
bypasses the clash guard in defineInterpreterModule (see modules[name] !==
logic) and should be removed here; either delete that line so ParseModule is
only registered via defineInterpreterModule('ParseModule', ParseEngine), or move
the eager-registration logic into defineInterpreterModule/install so the guard
is enforced there (update defineInterpreterModule to perform the initial
_MODULES[name] = logic write only after verifying no clash). Ensure references:
ParseModule, ParseEngine, _MODULES, defineInterpreterModule, and the
modules[name] !== logic check are handled accordingly.
In `@packages/tempo/src/plugin/plugin.util.ts`:
- Around line 41-49: interpret currently returns undefined when ensureModule
fails and methodOrFallback is undefined, which breaks
DurationModule.until()/since() callers expecting a Tempo.Duration; update the
fallback guard in interpret() so that if methodOrFallback is undefined (or a
string method name) and (t?.config?.catch === true || silent) the function
returns t (the host) instead of undefined; specifically adjust the branch that
checks isFunction(methodOrFallback) / isString(methodOrFallback) to also handle
methodOrFallback === undefined and return t, preserving existing behavior for
function/string fallbacks while preventing undefined returns for
DurationModule.until()/since().
In `@packages/tempo/src/tempo.class.ts`:
- Around line 1328-1336: until and since can return undefined when
DurationModule is missing because they pass methodOrFallback = undefined to
interpret; update both methods (until and since in Tempo class) to mirror the
safe pattern used by add/set by providing a fallback (e.g., "?? this") or switch
the interpret call to the string-dispatch form (pass 'until'/'since' as the
method string and a non-undefined fallback) so that when ensureModule fails
under config.catch the call short-circuits to a safe default instead of
returning undefined; locate the interpret(...) calls inside until and since and
add the local guard/fallback to preserve the expected
string/number/Tempo.Duration return contract.
---
Nitpick comments:
In `@packages/tempo/doc/releases/v2.x.md`:
- Around line 3-19: The release notes under "## [v2.2.3] - 2026-04-20" use plain
paragraphs for the sections "### New Features", "### Bug Fixes", "###
Documentation", and "### Chores"; change each item line under these subheadings
to a bulleted list (prefix each item with "- ") so the formatting matches older
entries like v2.2.2 and v2.1.x and renders consistently in Markdown.
In `@packages/tempo/index.md`:
- Around line 195-213: Add basic a11y to the carousel by adding ARIA
roles/labels and keyboard/pause controls: update the tempo-carousel-container to
include role="region" and aria-label="Tempo features", wire up
`@keydown`="handleKeydown" and keep the existing `@mouseenter/`@mouseleave isPaused
toggles; add a visible pause/play control bound to isPaused so users can stop
auto-rotation; set tempo-carousel-track to role="list" and bind :aria-live to
isPaused ? 'off' : 'polite'; implement a handleKeydown method that listens for
ArrowLeft/ArrowRight to decrement/increment activeIndex (clamping to
0..displayFeatures.length-1) and ensure each tempo-feature-card is focusable
(tabindex="0") and updates focus to the active card when activeIndex changes.
- Around line 54-66: The HMR workaround replaces registry.has and restores it
later but saves an unbound reference which can break code that expects the
method to be bound; change the save to bind the original method (originalHas =
registry.has.bind(registry)) before replacing it, keep the temporary replacement
as registry.has = () => false, restore with registry.has = originalHas, and add
an inline comment explaining this HMR workaround (use
Symbol.for('$LibrarySerializerRegistry', registry, originalHas, registry.has) to
locate the relevant symbols).
🪄 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: 3d90d524-e30c-449c-8989-1584893cc3f3
📒 Files selected for processing (18)
packages/tempo/doc/releases/index.mdpackages/tempo/doc/releases/v0.x.mdpackages/tempo/doc/releases/v2.x.mdpackages/tempo/doc/tempo.api.mdpackages/tempo/doc/tempo.layout.mdpackages/tempo/doc/tempo.library.mdpackages/tempo/index.mdpackages/tempo/package.jsonpackages/tempo/plan/standalone_parse.mdpackages/tempo/plan/ticker-reactive.mdpackages/tempo/src/library.index.tspackages/tempo/src/plugin/extend/extend.ticker.tspackages/tempo/src/plugin/module/module.duration.tspackages/tempo/src/plugin/module/module.mutate.tspackages/tempo/src/plugin/module/module.parse.tspackages/tempo/src/plugin/plugin.util.tspackages/tempo/src/tempo.class.tspackages/tempo/vitest.config.ts
✅ Files skipped from review due to trivial changes (7)
- packages/tempo/doc/tempo.library.md
- packages/tempo/doc/tempo.api.md
- packages/tempo/vitest.config.ts
- packages/tempo/plan/ticker-reactive.md
- packages/tempo/doc/releases/v0.x.md
- packages/tempo/src/plugin/extend/extend.ticker.ts
- packages/tempo/doc/tempo.layout.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tempo/plan/standalone_parse.md
- packages/tempo/package.json
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tempo/package.json`:
- Around line 165-168: The "./bundle" export currently points "import" to
dist/tempo.bundle.js which is an IIFE—replace it by either producing an ESM
build (change your bundler to output format 'es' and point "./bundle"."import"
to that ESM file, e.g., dist/tempo.bundle.esm.js) or split the exports: keep
"./bundle" as an ESM entry (with "types" and "import" pointing to the ESM
output) and add a new "./global" (or "./browser") export for the IIFE consumer
that includes "types", "import" (if needed) and a "script" field pointing to the
existing dist/tempo.bundle.js; update package.json exports accordingly so
consumers get a proper ESM module from "./bundle" and IIFE consumers use the new
"./global" entry.
In `@packages/tempo/README.md`:
- Around line 83-98: The README's importmap example incorrectly points to the
package export './bundle' (dist/tempo.bundle.js), which is an IIFE build and not
ESM; update the importmap to reference the package's ESM entry (the root ESM
export) so `import Tempo from '@magmacomputing/tempo'` works in browsers, or
alternatively produce a separate ESM bundle and change the README to point to
that new ESM artifact; ensure references to './bundle' / dist/tempo.bundle.js
are removed from the importmap example and replaced with the proper ESM export
name.
In `@packages/tempo/src/plugin/module/module.parse.ts`:
- Around line 177-186: The early return for non-primitive types in
ParseEngine.result prevents Tempo instances from being converted; move the
isTempo(value) branch above the primitive-type check so a Tempo passed into
conform() is converted to Temporal.ZonedDateTime first. Specifically, in the
function containing ParseEngine.result.call(this, arg, { match: type }); and the
isTempo(value) block, check isTempo(value) and run (value as any).toDateTime(),
update state.config.timeZone and state.config.calendar, and Object.assign(arg, {
type: 'Temporal.ZonedDateTime', value: res }) before performing the type !==
'String' && type !== 'Number' && ... early return. Ensure no other logic depends
on the previous order.
In `@packages/tempo/src/plugin/plugin.util.ts`:
- Around line 27-29: The error hint builds a module import suggestion using
module.toLowerCase(), which yields names like "parsemodule" for exported
specifiers that are actually "parse"; update the hint construction in the block
that checks (!isDefined(hostLogic) && !isTermsLoaded) so it strips a trailing
"Module" suffix from the module identifier (e.g., derive a baseName =
module.endsWith("Module") ? module.slice(0, -6) : module) before calling
toLowerCase(), and use that baseName when composing the message passed to
host[sym.$logError](t?.config, msg) so the suggested import specifier becomes
"#tempo/<baseName>" (or maintain an explicit map if preferred).
🪄 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: 53f44ad6-220e-4096-9d01-9d011cb68399
📒 Files selected for processing (14)
package.jsonpackages/library/package.jsonpackages/tempo/CHANGELOG.mdpackages/tempo/README.mdpackages/tempo/doc/releases/v2.x.mdpackages/tempo/importmap.jsonpackages/tempo/index.mdpackages/tempo/package.jsonpackages/tempo/src/plugin/extend/extend.ticker.tspackages/tempo/src/plugin/module/module.duration.tspackages/tempo/src/plugin/module/module.mutate.tspackages/tempo/src/plugin/module/module.parse.tspackages/tempo/src/plugin/plugin.util.tspackages/tempo/src/tempo.class.ts
✅ Files skipped from review due to trivial changes (4)
- packages/library/package.json
- package.json
- packages/tempo/importmap.json
- packages/tempo/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/tempo/src/plugin/extend/extend.ticker.ts
- packages/tempo/doc/releases/v2.x.md
- packages/tempo/index.md
This was referenced Apr 20, 2026
Merged
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First step to making the ParseEngine useable outside of Tempo.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores