Feature/doc tidy#58
Conversation
📝 WalkthroughWalkthroughTempo 3.8.0 updates TypeScript 7 build tooling, adds the tree-shakeable ChangesPlatform and Utilities
Plugin Type System and Runtime
Documentation Restructure and Content
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 19
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)
560-584: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winType-tagged branch doesn't verify
installis a function before calling it.The
type === 'plugin' | 'namespace' | 'module'checks don't also requireisFunction(item.install), unlike the trailingname+installclause. A malformed object with atypetag but no (or non-function)installwill pass this condition and then throw an unguardedTypeErrorat(item as TempoPlugin).install.call(...)(line 583), whereas theisFunction(arg)branch above catches similar misidentification errors.🛡️ Suggested fix
- else if (isObject(item) && ((item as any).type === 'plugin' || (item as any).type === 'namespace' || (item as any).type === 'module' || (isString((item as any).name) && isFunction((item as any).install)))) { + else if (isObject(item) && isFunction((item as any).install) && (['plugin', 'namespace', 'module'].includes((item as any).type) || isString((item as any).name))) {🤖 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 560 - 584, Update the object-form condition in the plugin handling branch to require isFunction(item.install) for every accepted type-tagged object (plugin, namespace, or module), while preserving the existing name-and-install fallback. This prevents malformed tagged objects from reaching the unguarded install.call invocation in the surrounding plugin registration logic.
🧹 Nitpick comments (5)
packages/tempo-fns/test/isSameFiscalQuarter.test.ts (1)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFiscal quarter labels in test comments may be inaccurate.
The comments
// Q1and// Q2assume calendar-aligned fiscal quarters (Jan–Mar = Q1). If the default fiscal year start differs (e.g., April), these labels are wrong even though the assertions still hold. Consider removing the inline quarter labels or making them generic to avoid confusion.🤖 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 17 - 18, Remove the potentially inaccurate `// Q1` and `// Q2` comments from the `t1` and `t2` date declarations in the `isSameFiscalQuarter` test, or replace them with neutral descriptions that do not assume calendar-aligned fiscal quarters.packages/tempo/doc/2-core-concepts/tempo.registry.md (1)
64-76: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAlign the registry override and safe-merge contracts.
This page documents
Tempo.extend()as allowing overrides, while the Soft Freeze documentation says shared-state merges are additive-only and protect existing root keys. Clarify that additive-only behavior applies only to discovery/plugin merges, or document the precedence rules forTempo.extend().🤖 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.registry.md` around lines 64 - 76, Clarify the conflicting registry merge contracts in the “Other Internal Registries” documentation: distinguish additive-only, root-key-preserving behavior for discovery/plugin merges from the override behavior of Tempo.extend(), and explicitly document Tempo.extend() precedence and safe-merge rules. Update the surrounding registry guidance so both contracts are consistent and unambiguous.packages/tempo/src/plugin/term/term.util.ts (1)
20-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider avoiding mutation of the input
termobject.
Object.assign(term, { type: 'term' })mutates the caller's original object in place. While the function already freezes the input (so callers can't use it afterward anyway), mutating an input parameter is a code smell that can surprise callers during debugging or logging. If reference identity doesn't matter to downstream consumers, consider spreading into a new object:♻️ Optional refactor
export const defineTerm = <T extends TermPlugin>(term: T): T & { type: 'term' } => { if (!term.version) term.version = TEMPO_VERSION; - const result = Object.assign(term, { type: 'term' } as const); + const result = Object.assign({}, term, { type: 'term' } as const) as T & { type: 'term' }; registerTerm(result); - return deepFreeze(result) as T & { type: 'term' }; + return deepFreeze(result); }Note: this changes reference identity (
result !== term), so verify no callers depend ondefineTermreturning the same reference as the input.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/src/plugin/term/term.util.ts` around lines 20 - 24, Update defineTerm to avoid mutating the caller-provided term: create result as a new object containing term and type instead of using Object.assign(term, ...), while preserving version defaulting, registerTerm, and deepFreeze behavior. Verify callers do not require the returned object to retain the same reference identity as the input.packages/tempo/doc/3-extending-tempo/tempo.term.md (1)
28-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a blank line after the table to avoid markdown table-parsing issues.
The paragraph at line 32 immediately follows the table (lines 28-31) with no blank line separator, which is why markdownlint flags missing pipes/wrong column count (MD055/MD056) — some renderers may misparse this line as a malformed extra table row.
📝 Suggested fix
| `tempo.term.<key>` | A short identifier string (e.g. `'qtr'`, `'szn'`, `'zdc'`) | | `tempo.term.<scope>` | The full matching range object, with all metadata fields (e.g. `key`, `day`, `month`, `year`, `sphere`, etc.) | + The `<key>` and `<scope>` are defined by the plugin author, where the intent of the `<key>` is to provide a short identifier value, and the intent of the `<scope>` is to provide the full matching range object.🤖 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.term.md` around lines 28 - 32, Add a blank line between the table ending with the `tempo.term.<scope>` row and the following paragraph beginning “The `<key>` and `<scope>` are defined” in the term documentation.Source: Linters/SAST tools
packages/tempo/src/plugin/plugin.util.ts (1)
239-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNamespace cache leaks as an enumerable own property on the instance.
this[cacheKey] = delegate(...)is a plain bracket assignment, which creates an enumerable, writable, configurable own property directly on theTempoinstance (e.g._myNamespace). Every other lazy-cache path in this codebase (#setLazyintempo.class.ts) usesObject.definePropertywith an explicitenumerableflag, and the prototype getter installed here even setsenumerable: falsefor itself — but the cached value it stores bypasses that discipline. This meansObject.keys(instance),for...in, and object-spread will unexpectedly surface this internal cache key, breaking the "lean instance" invariant the rest of the class relies on private fields to preserve.♻️ Suggested fix
get() { const cacheKey = isSymbol(config.name) ? config.name : `_${String(config.name)}`; if (!this[cacheKey]) { const target = Object.create(null); - this[cacheKey] = delegate(target, (key) => { - const resolver = config.resolvers[key as keyof typeof config.resolvers]; - if (resolver) return resolver(this); - return undefined; - }, true); + Object.defineProperty(this, cacheKey, { + value: delegate(target, (key) => { + const resolver = config.resolvers[key as keyof typeof config.resolvers]; + if (resolver) return resolver(this); + return undefined; + }, true), + enumerable: false, configurable: true, writable: false + }); } return 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 239 - 254, Store the delegated namespace cache as a non-enumerable own property instead of using direct assignment in the prototype getter. Update the caching logic in the Object.defineProperty callback for TempoClass to use Object.defineProperty(this, cacheKey, ...) with appropriate writable and configurable settings, while preserving symbol and string cache-key support and returning the cached value.
🤖 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/README.md`:
- Line 45: Update the README example for TempoFns.isFirstDayOfMonth to pass a
Tempo instance created with new Tempo() instead of the timestamp returned by
Tempo.now(), ensuring the documented CDN usage provides the expected .day
property.
In `@packages/tempo/doc/2-core-concepts/tempo.format.md`:
- Around line 190-195: The “Complex Native Intl Formatting” documentation
overstates Tempo’s suppression of Intl errors. Update the section to explain
that Intl.DateTimeFormat still validates locales and option values; Tempo only
retries Temporal mismatch failures using zdt.epochMilliseconds, injects timeZone
and calendar, and swallows withTimeZone/withCalendar failures. Remove the claim
that native RangeError and TypeError exceptions are bypassed wholesale.
In `@packages/tempo/doc/2-core-concepts/tempo.registry.md`:
- Around line 38-59: Update the timezone registry documentation to explicitly
state that aliases map to DST-aware regional IANA zones, not fixed UTC offsets;
clarify that entries such as gmt → Europe/London and est → America/New_York may
become BST or EDT seasonally, and advise callers needing literal GMT/EST offsets
to use appropriate fixed-offset zones.
In `@packages/tempo/doc/3-extending-tempo/tempo.plugin.md`:
- Around line 144-147: Replace the invalid isLeapYear import in the
documentation snippet with a helper that is actually exported by
`@magmacomputing/tempo-fns`, or export isLeapYear from that package before using
it with Tempo.
In `@packages/tempo/doc/4-advanced-reference/sandbox-factory.md`:
- Around line 46-51: Resolve the contradiction in the “Hierarchy of Resolution”
section by making its ordering explicit: label the list as highest-to-lowest
priority if Local through Global Defaults reflects actual precedence, or reverse
the entries to match a true “Last One Wins” interpretation. Ensure the wording
and ordering consistently describe collision resolution.
In `@packages/tempo/doc/5-architecture-and-internals/architecture.md`:
- Around line 61-67: Qualify the “Zero-Cost” claims in the Status section by
stating that the ~523µs and ~359µs figures are steady-state benchmark results
after warm-up under the documented setup. Clarify that constructor and guard
costs can depend on input scanning and registry-mutation-driven wordlist
rebuilds, so they are not universally input- or registry-independent; update the
“guarantee” wording in the Lazy Evaluation and Master Guard discussion
accordingly.
In `@packages/tempo/doc/5-architecture-and-internals/lazy-evaluation-pattern.md`:
- Around line 11-36: Update the lazy-evaluation example around `setLazy` so it
no longer memoizes by redefining properties on a frozen target. Use a mutable
sidecar/private cache keyed by target and property name, or explicitly document
that the object owning the lazy properties must remain unfrozen; ensure the
implementation and explanation do not claim a Proxy can bypass non-configurable
property invariants.
In `@packages/tempo/doc/5-architecture-and-internals/soft_freeze_strategy.md`:
- Around line 57-66: Resolve the contradiction in the enumify example: its
default frozen=true is passed as the lock argument to proxify, producing Hard
Freeze rather than the documented Soft Freeze. Prefer changing enumify’s default
to false and update the inline comments to accurately describe the resulting
extensible-registry behavior; otherwise revise the surrounding documentation to
consistently describe Hard Freeze by default.
In `@packages/tempo/doc/6-utility-library/tempo.decorators.md`:
- Line 6: Correct the inconsistent product-name capitalization in the
documentation text by changing “Javascript” to “JavaScript” in the paragraph
describing native ECMAScript decorators and TypeScript transpilation.
In `@packages/tempo/doc/6-utility-library/tempo.library.md`:
- Line 13: Correct the documentation typos: change “iteratable” to “iterable” in
the enumify description and capitalize “Javascript” as “JavaScript” at the later
occurrence.
In `@packages/tempo/doc/6-utility-library/tempo.pledge.md`:
- Around line 84-93: Update the “Automatic Cleanup (Symbol.dispose)” section to
clarify that disposal occurs only through a `using` declaration or explicit
`Symbol.dispose` invocation, not ordinary block exit or garbage collection.
Revise the `Pledge` description and example comments to avoid implying pending
promises are automatically rejected when leaving scope.
- Line 3: Address the runtime compatibility issue in the Pledge implementation:
update the constructor or resolver initialization in pledge.class.ts to use a
compatible fallback when Promise.withResolvers() is unavailable, or consistently
raise the project’s minimum Node.js version to 22+ and update related
documentation and metadata. Ensure new Pledge instances work on the supported
runtime.
In `@packages/tempo/doc/6-utility-library/tempo.serializers.md`:
- Around line 54-59: Correct the structuredClone comparison in the “Fallback for
structuredClone” section: replace the claim that it “drops functions” with
wording that accurately states structuredClone throws on functions or cannot
clone them, while preserving the surrounding comparison.
In `@packages/tempo/doc/7-ecosystem/extending-temporal.md`:
- Line 15: Change the four section headings in the document from level-three
(`###`) to level-two (`##`), including “Parsing: Strict vs. Intelligent” and the
sections at the locations corresponding to lines 35, 56, and 92, so they follow
the document’s top-level title hierarchy.
- Around line 106-109: Make the relative-time example deterministic by replacing
Temporal.Now.plainDateISO() with a fixed reference date that produces the
documented three-day difference, or remove the hard-coded output assertion.
Update the surrounding example consistently while retaining the
Temporal.PlainDate.until and Intl.RelativeTimeFormat usage.
In `@packages/tempo/doc/7-ecosystem/vision.md`:
- Around line 14-18: Fix the punctuation in the value proposition section: close
the em-dash parenthetical in the “Business Intelligence via Plugin” paragraph,
and add a final period to the “Lean but Robust” paragraph.
In `@packages/tempo/doc/8-project-and-support/releases/v2.x.md`:
- Line 153: Fix the v2.5.0 release heading by adding a space after the hyphen
before the date, matching the formatting used by the other entries in the
release document.
In `@packages/tempo/doc/8-project-and-support/releases/v4.x.md`:
- Around line 3-14: The planned v4.0.0 release notes use past tense for
unreleased changes; update the entries under “Major Breaking Changes,” “API
Removals,” and “Internal Architecture” to future tense, using wording such as
“will be removed” and “will enforce” while preserving the migration guidance.
In `@packages/tempo/src/tempo.class.ts`:
- Around line 518-524: Update isOptionsArg in Tempo.extend so valid t.Options
objects containing intl, planner, ignore, monthDay, or plugins are recognized as
options rather than discovery input; remove or narrow the DISCOVERY-key
rejection and use an explicit distinction that preserves genuine discovery
arguments. Verify the resulting argument classification still handles plugin
configuration correctly.
---
Outside diff comments:
In `@packages/tempo/src/tempo.class.ts`:
- Around line 560-584: Update the object-form condition in the plugin handling
branch to require isFunction(item.install) for every accepted type-tagged object
(plugin, namespace, or module), while preserving the existing name-and-install
fallback. This prevents malformed tagged objects from reaching the unguarded
install.call invocation in the surrounding plugin registration logic.
---
Nitpick comments:
In `@packages/tempo-fns/test/isSameFiscalQuarter.test.ts`:
- Around line 17-18: Remove the potentially inaccurate `// Q1` and `// Q2`
comments from the `t1` and `t2` date declarations in the `isSameFiscalQuarter`
test, or replace them with neutral descriptions that do not assume
calendar-aligned fiscal quarters.
In `@packages/tempo/doc/2-core-concepts/tempo.registry.md`:
- Around line 64-76: Clarify the conflicting registry merge contracts in the
“Other Internal Registries” documentation: distinguish additive-only,
root-key-preserving behavior for discovery/plugin merges from the override
behavior of Tempo.extend(), and explicitly document Tempo.extend() precedence
and safe-merge rules. Update the surrounding registry guidance so both contracts
are consistent and unambiguous.
In `@packages/tempo/doc/3-extending-tempo/tempo.term.md`:
- Around line 28-32: Add a blank line between the table ending with the
`tempo.term.<scope>` row and the following paragraph beginning “The `<key>` and
`<scope>` are defined” in the term documentation.
In `@packages/tempo/src/plugin/plugin.util.ts`:
- Around line 239-254: Store the delegated namespace cache as a non-enumerable
own property instead of using direct assignment in the prototype getter. Update
the caching logic in the Object.defineProperty callback for TempoClass to use
Object.defineProperty(this, cacheKey, ...) with appropriate writable and
configurable settings, while preserving symbol and string cache-key support and
returning the cached value.
In `@packages/tempo/src/plugin/term/term.util.ts`:
- Around line 20-24: Update defineTerm to avoid mutating the caller-provided
term: create result as a new object containing term and type instead of using
Object.assign(term, ...), while preserving version defaulting, registerTerm, and
deepFreeze behavior. Verify callers do not require the returned object to retain
the same reference identity as the input.
🪄 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: df91decf-40c1-41ea-a73a-b1ce81e1cf3f
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpackages/tempo-fns/img/tempo-fns-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (67)
package.jsonpackages/library/package.jsonpackages/tempo-fns/doc/CONTRIBUTING.mdpackages/tempo-fns/doc/README.mdpackages/tempo-fns/doc/preview.htmlpackages/tempo-fns/package.jsonpackages/tempo-fns/rollup.config.jspackages/tempo-fns/src/index.tspackages/tempo-fns/src/isFirstDayOfMonth.tspackages/tempo-fns/src/isSameFiscalQuarter.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/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/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.ts
💤 Files with no reviewable changes (2)
- packages/tempo/doc/architecture.md
- packages/tempo/doc/tempo-vs-temporal.md
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 (19)
packages/tempo-fns/doc/README.md (1)
45-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Search for a static `now` method on the Tempo class rg -n 'static\s+now\b' packages/tempo/src --type=ts -C3Repository: magmacomputing/magma
Length of output: 987
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== README excerpt ==" sed -n '20,60p' packages/tempo-fns/doc/README.md | cat -n echo echo "== Search isFirstDayOfMonth ==" rg -n "isFirstDayOfMonth" packages/tempo-fns packages/tempo/src -C 3Repository: magmacomputing/magma
Length of output: 7188
Use
new Tempo()here instead ofTempo.now().Tempo.now()returns a timestamp, butisFirstDayOfMonthexpects aTempo/Temporal-like value with a.dayproperty, so this CDN example won’t work as written.🤖 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` at line 45, Update the README example for TempoFns.isFirstDayOfMonth to pass a Tempo instance created with new Tempo() instead of the timestamp returned by Tempo.now(), ensuring the documented CDN usage provides the expected .day property.packages/tempo/doc/2-core-concepts/tempo.format.md (1)
190-195: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the documented section and the formatter implementation. git ls-files packages/tempo/doc/2-core-concepts/tempo.format.md printf '\n--- tempo.format.md excerpt ---\n' sed -n '170,220p' packages/tempo/doc/2-core-concepts/tempo.format.md printf '\n--- search for Intl.DateTimeFormatOptions handling ---\n' rg -n "DateTimeFormatOptions|Intl\.DateTimeFormat|RangeError|TypeError|locale" packages/tempo -g '!**/dist/**' -g '!**/build/**' printf '\n--- candidate source files ---\n' fd -a "format" packages/tempoRepository: magmacomputing/magma
Length of output: 50378
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n--- packages/tempo/src/module/module.format.ts (around Intl formatting path) ---\n' sed -n '1,160p' packages/tempo/src/module/module.format.ts printf '\n--- search for getDTF definition ---\n' rg -n "function getDTF|getDTF\s*=" packages/tempo/src -g '!**/dist/**' -g '!**/build/**' printf '\n--- packages/tempo/src/support/support.init.ts (locale/timezone normalization hooks) ---\n' sed -n '200,260p' packages/tempo/src/support/support.init.tsRepository: magmacomputing/magma
Length of output: 6639
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n--- getDTF and related Intl helpers ---\n' fd -a "international.library" packages/tempo/src printf '\n' rg -n "export .*getDTF|function getDTF|const getDTF|new Intl\.DateTimeFormat|DateTimeFormat\(" packages/tempo/src -g '*international*' -g '!**/dist/**' -g '!**/build/**' printf '\n--- relevant slice from international library if present ---\n' f=$(fd -a "international.library" packages/tempo/src | head -n 1 || true) if [ -n "${f:-}" ]; then sed -n '1,260p' "$f" fiRepository: magmacomputing/magma
Length of output: 200
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find the Intl helper implementation and any tests around formatter fallback. rg -n "getDTF|format\(zdt\)|fallbackOptions|withTimeZone|withCalendar" packages/tempo/src packages/tempo/test -g '!**/dist/**' -g '!**/build/**' # Show the formatter path with line numbers for precise context. cat -n packages/tempo/src/module/module.format.ts | sed -n '90,130p'Repository: magmacomputing/magma
Length of output: 8482
🏁 Script executed:
#!/bin/bash set -euo pipefail cat -n packages/tempo/test/instance/instance.format.test.ts | sed -n '45,80p'Repository: magmacomputing/magma
Length of output: 1527
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the Intl helper implementation and inspect its error behavior. fd -a "international.library" packages/tempo/src packages/tempo -t f fd -a "international" packages/tempo/src packages/tempo -t f printf '\n--- possible helper definitions ---\n' rg -n "export .*getDTF|const getDTF|function getDTF|new Intl\.DateTimeFormat" packages/tempo/src -g '!**/dist/**' -g '!**/build/**' -g '!**/node_modules/**'Repository: magmacomputing/magma
Length of output: 195
Clarify the Intl fallback behavior
Intl.DateTimeFormatstill validates locales and option values; Tempo only softens the Temporal mismatch path by retrying withzdt.epochMillisecondsand injectingtimeZone/calendar, whilewithTimeZone/withCalendarfailures are swallowed. Describe those constraints explicitly instead of saying nativeRangeErrorandTypeErrorare bypassed wholesale.🤖 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` around lines 190 - 195, The “Complex Native Intl Formatting” documentation overstates Tempo’s suppression of Intl errors. Update the section to explain that Intl.DateTimeFormat still validates locales and option values; Tempo only retries Temporal mismatch failures using zdt.epochMilliseconds, injects timeZone and calendar, and swallows withTimeZone/withCalendar failures. Remove the claim that native RangeError and TypeError exceptions are bypassed wholesale.packages/tempo/doc/2-core-concepts/tempo.registry.md (1)
38-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify that these aliases represent regional zones, not fixed offsets.
Mappings such as
gmt → Europe/Londonandest → America/New_Yorkchange offset seasonally (BST/EDT). If callers expect literal GMT/EST offsets, these aliases are incorrect; otherwise state explicitly that they select DST-aware regions.🤖 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.registry.md` around lines 38 - 59, Update the timezone registry documentation to explicitly state that aliases map to DST-aware regional IANA zones, not fixed UTC offsets; clarify that entries such as gmt → Europe/London and est → America/New_York may become BST or EDT seasonally, and advise callers needing literal GMT/EST offsets to use appropriate fixed-offset zones.packages/tempo/doc/3-extending-tempo/tempo.plugin.md (1)
144-147: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n 'isLeapYear|tempo/core|"\./core"' \ packages/tempo-fns/src \ packages/tempo-fns/package.json \ packages/tempo/package.jsonRepository: magmacomputing/magma
Length of output: 258
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n## packages/tempo/package.json\n' sed -n '1,240p' packages/tempo/package.json printf '\n## packages/tempo-fns/package.json\n' sed -n '1,220p' packages/tempo-fns/package.json printf '\n## tempo-fns exports / source hits\n' rg -n 'isLeapYear|export .*isLeapYear|from .*tempo-fns' packages/tempo-fns -SRepository: magmacomputing/magma
Length of output: 9890
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n## file list\n' git ls-files 'packages/tempo-fns/**' 'packages/tempo/**' | sed -n '1,200p' printf '\n## package entry points\n' fd -a -t f 'index\.(ts|js|mjs|cjs)$' packages/tempo-fns packages/tempo | sed -n '1,120p' printf '\n## ast-grep outline for likely export files\n' for f in $(fd -a -t f 'index\.ts$' packages/tempo-fns packages/tempo 2>/dev/null); do echo "### $f" ast-grep outline "$f" --view expanded || true doneRepository: magmacomputing/magma
Length of output: 14520
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n## core subpath references\n' rg -n '"\./core"|`#tempo/core`|tempo/core' packages/tempo/package.json packages/tempo/src packages/tempo-fns/src -S printf '\n## isLeapYear definition / export chain\n' rg -n 'isLeapYear' packages/tempo-fns/src packages/tempo-fns -g '!**/*.map' -SRepository: magmacomputing/magma
Length of output: 592
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n## repo-wide isLeapYear search\n' rg -n 'isLeapYear' . -S printf '\n## tempo-fns source exports\n' cat -n packages/tempo-fns/src/index.ts printf '\n## tempo-fns source files with export declarations\n' for f in packages/tempo-fns/src/*.ts; do echo "### $f" rg -n 'export ' "$f" -n -S || true doneRepository: magmacomputing/magma
Length of output: 1529
Replace the missing
isLeapYearimport
@magmacomputing/tempo/coreis exported, butisLeapYearis not exported from@magmacomputing/tempo-fns, so this snippet won’t run as written. Use an exported helper here or add the missing export.🤖 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.plugin.md` around lines 144 - 147, Replace the invalid isLeapYear import in the documentation snippet with a helper that is actually exported by `@magmacomputing/tempo-fns`, or export isLeapYear from that package before using it with Tempo.packages/tempo/doc/4-advanced-reference/sandbox-factory.md (1)
46-51: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the sandbox precedence contradiction.
The section says “Last One Wins,” but the hierarchy lists Local before Sandbox, Plugins, and Global Defaults. That wording implies Global Defaults override local aliases, contradicting the apparent precedence order and potentially misleading collision resolution. Label the list “highest to lowest priority” or reverse it.
🤖 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/sandbox-factory.md` around lines 46 - 51, Resolve the contradiction in the “Hierarchy of Resolution” section by making its ordering explicit: label the list as highest-to-lowest priority if Local through Global Defaults reflects actual precedence, or reverse the entries to match a true “Last One Wins” interpretation. Ensure the wording and ordering consistently describe collision resolution.packages/tempo/doc/5-architecture-and-internals/architecture.md (1)
61-67: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the cited sections with surrounding context. file='packages/tempo/doc/5-architecture-and-internals/architecture.md' wc -l "$file" sed -n '50,80p' "$file" printf '\n----\n' sed -n '120,150p' "$file" printf '\n----\n' sed -n '1,220p' "$file" | rg -n "O\\(1\\)|523µs|359µs|zero-cost|fast-fail|master guard|wordlist|registry|scan|benchmark|steady-state|input"Repository: magmacomputing/magma
Length of output: 9763
Qualify the “zero-cost” claim. The wording reads like a universal latency guarantee, but the benchmark is explicitly steady-state after warm-up, and the guard still depends on input scanning and registry-mutation-driven wordlist rebuilds. Clarify that
~523µsreflects the documented setup, not input- or registry-independent performance.🤖 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 61 - 67, Qualify the “Zero-Cost” claims in the Status section by stating that the ~523µs and ~359µs figures are steady-state benchmark results after warm-up under the documented setup. Clarify that constructor and guard costs can depend on input scanning and registry-mutation-driven wordlist rebuilds, so they are not universally input- or registry-independent; update the “guarantee” wording in the Lazy Evaluation and Master Guard discussion accordingly.packages/tempo/doc/5-architecture-and-internals/lazy-evaluation-pattern.md (1)
11-36: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The memoization example still mutates the frozen target.
After
Object.freeze(target), the accessor cannot be redefined because it is non-configurable. The laterObject.defineProperty(target, name, { value, ... })therefore throws aTypeError; a Proxy or private field cannot bypass this property invariant. Replace the example with a mutable sidecar/private cache, or explicitly avoid freezing the object that owns the lazy properties.🤖 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/lazy-evaluation-pattern.md` around lines 11 - 36, Update the lazy-evaluation example around `setLazy` so it no longer memoizes by redefining properties on a frozen target. Use a mutable sidecar/private cache keyed by target and property name, or explicitly document that the object owning the lazy properties must remain unfrozen; ensure the implementation and explanation do not claim a Proxy can bypass non-configurable property invariants.packages/tempo/doc/5-architecture-and-internals/soft_freeze_strategy.md (1)
57-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the contradictory
enumifydefault.
enumify(list, frozen = true)passeslock=truetoproxify, which is the documented Hard Freeze path—not Soft Freeze. Either change the default tofalseor update the surrounding text to describe hard-freeze-by-default behavior.Proposed alignment if extensible registries should be the default
-export function enumify(list, frozen = true) { +export function enumify(list, frozen = false) { const target = Object.create(proto, descriptors); - // Default to Soft Freeze (frozen=true, lock=false) - // if 'frozen' is passed as false, it signals 'extensible library registry' + // Default to Soft Freeze: the proxy is read-only while the target remains mutable. return proxify(target, true, frozen); }📝 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.export function enumify(list, frozen = false) { const target = Object.create(proto, descriptors); // Default to Soft Freeze: the proxy is read-only while the target remains mutable. return proxify(target, true, frozen);🤖 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/soft_freeze_strategy.md` around lines 57 - 66, Resolve the contradiction in the enumify example: its default frozen=true is passed as the lock argument to proxify, producing Hard Freeze rather than the documented Soft Freeze. Prefer changing enumify’s default to false and update the inline comments to accurately describe the resulting extensible-registry behavior; otherwise revise the surrounding documentation to consistently describe Hard Freeze by default.packages/tempo/doc/6-utility-library/tempo.decorators.md (1)
6-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix "Javascript" capitalization to "JavaScript".
Line 6 uses "Javascript" while the rest of the document correctly uses "TypeScript" and "ECMAScript". Consistent capitalization is important in user-facing documentation.
✏️ Proposed fix
-> Currently, the TypeScript compiler target for Tempo is set to ES2022. Because native ECMAScript decorators are still maturing across Javascript engines, TypeScript transpiles these decorators away into standard function calls in the emitted Javascript code. Our long-term aim is to bring these decorators to first-class, native Javascript items once the ECMAScript decorator proposal is fully mature and widely implemented in engines without requiring down-level transpilation. +> Currently, the TypeScript compiler target for Tempo is set to ES2022. Because native ECMAScript decorators are still maturing across JavaScript engines, TypeScript transpiles these decorators away into standard function calls in the emitted JavaScript code. Our long-term aim is to bring these decorators to first-class, native JavaScript items once the ECMAScript decorator proposal is fully mature and widely implemented in engines without requiring down-level transpilation.📝 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.> Currently, the TypeScript compiler target for Tempo is set to ES2022. Because native ECMAScript decorators are still maturing across JavaScript engines, TypeScript transpiles these decorators away into standard function calls in the emitted JavaScript code. Our long-term aim is to bring these decorators to first-class, native JavaScript items once the ECMAScript decorator proposal is fully mature and widely implemented in engines without requiring down-level transpilation.🤖 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.decorators.md` at line 6, Correct the inconsistent product-name capitalization in the documentation text by changing “Javascript” to “JavaScript” in the paragraph describing native ECMAScript decorators and TypeScript transpilation.packages/tempo/doc/6-utility-library/tempo.library.md (1)
13-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix typo and capitalization in documentation.
Line 13: "iteratable" should be "iterable". Line 41: "Javascript" should be "JavaScript" (capital S).
📝 Proposed fixes
-Tempo uses a custom utility called `enumify` to create heavily-protected, iteratable enum-like objects instead of relying on native TypeScript enums. +Tempo uses a custom utility called `enumify` to create heavily-protected, iterable enum-like objects instead of relying on native TypeScript enums.-Because Tempo's build target is currently ES2022, this decorator functionality is transpiled away into standard Javascript functions by the compiler rather than using native ECMAScript decorators. Our aim is to transition these to first-class native features once Javascript engines natively support the ECMAScript decorator proposal. +Because Tempo's build target is currently ES2022, this decorator functionality is transpiled away into standard JavaScript functions by the compiler rather than using native ECMAScript decorators. The goal is to transition these to first-class native features once JavaScript engines natively support the ECMAScript decorator proposal.Also applies to: 41-41
🧰 Tools
🪛 LanguageTool
[grammar] ~13-~13: Ensure spelling is correct
Context: ...enumifyto create heavily-protected, iteratable enum-like objects instead of relying on...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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.library.md` at line 13, Correct the documentation typos: change “iteratable” to “iterable” in the enumify description and capitalize “Javascript” as “JavaScript” at the later occurrence.packages/tempo/doc/6-utility-library/tempo.pledge.md (2)
3-3: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n 'Promise\.withResolvers|class Pledge|engines|target' packages package.jsonRepository: magmacomputing/magma
Length of output: 30168
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== package.json ==\n' cat -n package.json | sed -n '1,40p' printf '\n== packages/tempo/package.json ==\n' cat -n packages/tempo/package.json | sed -n '1,40p' printf '\n== packages/library/src/common/pledge.class.ts ==\n' cat -n packages/library/src/common/pledge.class.ts | sed -n '1,140p' printf '\n== packages/tempo/doc/6-utility-library/tempo.pledge.md ==\n' cat -n packages/tempo/doc/6-utility-library/tempo.pledge.md | sed -n '1,140p'Repository: magmacomputing/magma
Length of output: 12195
🌐 Web query:
Node.js Promise.withResolvers supported version Node 20 release💡 Result:
The Promise.withResolvers method is not natively supported in Node.js 20 [1][2]. It was officially introduced as a stable feature in Node.js version 22.0.0 [1]. While some versions of Node.js 21 (specifically v21.7.1 and later) allowed the use of Promise.withResolvers when executed with the --js-promise-with-resolvers flag, this functionality was not backported to the Node.js 20 release line [3][2][4]. If you are using Node.js 20, you must use a polyfill to implement this functionality [5][3].
Citations:
- 1: https://lirantal.com/blog/promise-with-resolvers-in-nodejs-tests
- 2: elizaOS/eliza#76
- 3: https://techinsights.manisuec.com/javascript/promise-with-resolvers/
- 4: https://mikevdv.dev/blog/promise-withresolvers-in-javascript/
- 5: nodejs/node#54836
Raise the runtime floor or add a
Promise.withResolvers()fallback
packages/library/src/common/pledge.class.ts:71callsPromise.withResolvers()directly, but the repo’s Node floor is>=20.0.0. That API is only available unflagged in newer Node releases, sonew Pledge()can fail on the documented runtime. Either add a fallback/polyfill or bump the required Node version to 22+.🤖 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.pledge.md` at line 3, Address the runtime compatibility issue in the Pledge implementation: update the constructor or resolver initialization in pledge.class.ts to use a compatible fallback when Promise.withResolvers() is unavailable, or consistently raise the project’s minimum Node.js version to 22+ and update related documentation and metadata. Ensure new Pledge instances work on the supported runtime.
84-93: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not imply that ordinary scope exit disposes a
Pledge.
Symbol.disposeis invoked byusingor explicit disposal; a normal block ending or garbage collection does not automatically reject a pending promise. Clarify this to prevent callers from assuming that unresolved operations are cleaned up withoutusingor an explicit dispose call.🤖 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.pledge.md` around lines 84 - 93, Update the “Automatic Cleanup (Symbol.dispose)” section to clarify that disposal occurs only through a `using` declaration or explicit `Symbol.dispose` invocation, not ordinary block exit or garbage collection. Revise the `Pledge` description and example comments to avoid implying pending promises are automatically rejected when leaving scope.packages/tempo/doc/6-utility-library/tempo.serializers.md (1)
54-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix:
structuredClonedoes not "drop" functions — it throws.Line 57 states
structuredClone"still drops functions." In reality,structuredClone(() => {})throws aDataCloneError; it does not silently drop them. This distinction matters for users evaluating error-handling strategies. Consider rewording to "still throws on functions" or "still cannot clone functions."📝 Proposed fix
-* **Fallback for `structuredClone`:** While `structuredClone` is great, it is not universally available in older environments and still drops functions. `cloneify` gives you a guaranteed deep-clone that resurrects your precise types reliably. +* **Fallback for `structuredClone`:** While `structuredClone` is great, it is not universally available in older environments and still throws on functions. `cloneify` gives you a guaranteed deep-clone that resurrects your precise types reliably.📝 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.Why not just use `JSON.parse(JSON.stringify(obj))` or `structuredClone()`? * **Preservation of Rich Types:** `JSON.stringify()` instantly throws a `TypeError` if it encounters a `BigInt`. It silently strips `undefined`, reduces `Date` to a dumb string, and mangles `Set`/`Map` into empty `{}` objects. Tempo's `stringify` preserves all of these. * **Fallback for `structuredClone`:** While `structuredClone` is great, it is not universally available in older environments and still throws on functions. `cloneify` gives you a guaranteed deep-clone that resurrects your precise types reliably. * **Safe Encoding:** Strings are natively URI-encoded for control characters, ensuring safe stashing in fragile WebStorage or cache environments without injection/corruption vulnerabilities. * **Class Registration:** User-defined classes can be resurrected after stringification if they are decorated with Tempo's `@Serialize()` registry.🤖 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.serializers.md` around lines 54 - 59, Correct the structuredClone comparison in the “Fallback for structuredClone” section: replace the claim that it “drops functions” with wording that accurately states structuredClone throws on functions or cannot clone them, while preserving the surrounding comparison.packages/tempo/doc/7-ecosystem/extending-temporal.md (2)
15-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use level-two headings for the four sections.
These
###headings jump directly from the document’s#title, producing an invalid hierarchy and incorrect generated navigation.Proposed fix
-### 1. Parsing: Strict vs. Intelligent +## 1. Parsing: Strict vs. IntelligentApply the same change to Lines 35, 56, and 92.
Also applies to: 35-35, 56-56, 92-92
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
🤖 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` at line 15, Change the four section headings in the document from level-three (`###`) to level-two (`##`), including “Parsing: Strict vs. Intelligent” and the sections at the locations corresponding to lines 35, 56, and 92, so they follow the document’s top-level title hierarchy.Source: Linters/SAST tools
106-109: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the relative-time example deterministic.
Temporal.Now.plainDateISO()changes over time, so the hard-coded"3 days ago"result will eventually be false. Use a fixed reference date or omit the exact output.Proposed fix
-const now = Temporal.Now.plainDateISO(); +const now = Temporal.PlainDate.from('2026-07-09');🤖 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 106 - 109, Make the relative-time example deterministic by replacing Temporal.Now.plainDateISO() with a fixed reference date that produces the documented three-day difference, or remove the hard-coded output assertion. Update the surrounding example consistently while retaining the Temporal.PlainDate.until and Intl.RelativeTimeFormat usage.packages/tempo/doc/7-ecosystem/vision.md (1)
14-18: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the punctuation in the value proposition section.
Line 15 leaves the em-dash parenthetical unclosed, and Line 18 lacks a final period.
-Tempo extends beyond simple date arithmetic through its **Terms** system. By providing declarative access to complex calculations—such as fiscal quarters, meteorological seasons, and zodiac signs, Tempo moves domain-specific logic out of the application code and into a reusable, extensible plugin architecture. +Tempo extends beyond simple date arithmetic through its **Terms** system. By providing declarative access to complex calculations—such as fiscal quarters, meteorological seasons, and zodiac signs—Tempo moves domain-specific logic out of the application code and into a reusable, extensible plugin architecture. ... -Tempo is designed to be a thin, highly capable layer. It prioritizes a lightweight API surface for the developer while maintaining robust internal logic to handle the complexities of timezones, calendars, and durations +Tempo is designed to be a thin, highly capable layer. It prioritizes a lightweight API surface for the developer while maintaining robust internal logic to handle the complexities of timezones, calendars, and durations.📝 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.### 3. Business Intelligence via Plugin Tempo extends beyond simple date arithmetic through its **Terms** system. By providing declarative access to complex calculations—such as fiscal quarters, meteorological seasons, and zodiac signs—Tempo moves domain-specific logic out of the application code and into a reusable, extensible plugin architecture. ### 4. Lean but Robust Tempo is designed to be a thin, highly capable layer. It prioritizes a lightweight API surface for the developer while maintaining robust internal logic to handle the complexities of timezones, calendars, and durations.🤖 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/vision.md` around lines 14 - 18, Fix the punctuation in the value proposition section: close the em-dash parenthetical in the “Business Intelligence via Plugin” paragraph, and add a final period to the “Lean but Robust” paragraph.packages/tempo/doc/8-project-and-support/releases/v2.x.md (1)
153-153: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the v2.5.0 date separator.
The heading is missing a space before the date, so it should use
- 2026-04-25like the other release entries.Proposed fix
-## [v2.5.0] -2026-04-25 +## [v2.5.0] - 2026-04-25📝 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.## [v2.5.0] - 2026-04-25🤖 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/v2.x.md` at line 153, Fix the v2.5.0 release heading by adding a space after the hyphen before the date, matching the formatting used by the other entries in the release document.packages/tempo/doc/8-project-and-support/releases/v4.x.md (1)
3-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use future tense for planned v4 changes.
This section is marked
Planned, but says changes were “Removed” and “has been completely removed,” which can mislead users about the current API. Use “will be removed”/“will enforce,” or move this content after the 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/doc/8-project-and-support/releases/v4.x.md` around lines 3 - 14, The planned v4.0.0 release notes use past tense for unreleased changes; update the entries under “Major Breaking Changes,” “API Removals,” and “Internal Architecture” to future tense, using wording such as “will be removed” and “will enforce” while preserving the migration guidance.packages/tempo/src/tempo.class.ts (1)
518-524: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect the DISCOVERY Set definition and confirm overlap with legitimate Options keys rg -n -B2 -A10 'DISCOVERY\s*=' packages/tempo/src rg -n 'DISCOVERY\.has' packages/tempo/srcRepository: magmacomputing/magma
Length of output: 1446
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- tempo.class.ts around isOptionsArg / argument handling ---' sed -n '500,690p' packages/tempo/src/tempo.class.ts | cat -n echo '--- search for Tempo.extend / plugin.install signatures ---' rg -n "extend\(|install\(" packages/tempo/src -g '!**/dist/**' echo '--- search for Options type definition ---' rg -n "interface Options|type Options|export .*Options" packages/tempo/src packages/tempo -g '!**/dist/**'Repository: magmacomputing/magma
Length of output: 19129
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- tempo.type.ts around BaseOptions / Options ---' sed -n '80,180p' packages/tempo/src/tempo.type.ts | cat -n echo '--- tempo.type.ts around Internal.BaseOptions and OptionsKeep ---' sed -n '240,360p' packages/tempo/src/tempo.type.ts | cat -n echo '--- tempo.class.ts around $setDiscovery call sites and discovery handling ---' sed -n '360,460p' packages/tempo/src/tempo.class.ts | cat -nRepository: magmacomputing/magma
Length of output: 17118
isOptionsArgstill collides with validt.Optionsfields.Tempo.extend(plugin, { intl: ... }),{ planner: ... },{ ignore: ... },{ monthDay: ... }, or{ plugins: ... }will be treated as discovery input because the heuristic rejects any object with aDISCOVERYkey. That skips the plugin-config path and changesextend()behavior for legitimate 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/src/tempo.class.ts` around lines 518 - 524, Update isOptionsArg in Tempo.extend so valid t.Options objects containing intl, planner, ignore, monthDay, or plugins are recognized as options rather than discovery input; remove or narrow the DISCOVERY-key rejection and use an explicit distinction that preserves genuine discovery arguments. Verify the resulting argument classification still handles plugin configuration correctly.
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 (2)
packages/tempo/src/tempo.class.ts (2)
518-529: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
isOptionsArgexclusion list is incomplete — discovery objects withmonthDay,intl,planner,registry,plugins, orignorekeys are silently swallowed.The predicate treats any plain object without the excluded keys as an options arg. Since
monthDay,intl,planner,registry,plugins, andignoreare not in the exclusion list, a call likeTempo.extend({ monthDay: { locales: [...] } })(withoutoptions: {}) would have its sole argument popped asoptions, leavingitemsempty and triggering the earlyreturn thisat line 539. The discovery processing (includingregistryUpdateside effects) never runs.This contradicts the overload signatures (lines 503-510) where
optionsis always the second parameter, not an inferred last argument. The tests indiscovery-extend.test.tswork around this by addingoptions: {}as a discriminator, but this is a fragile API contract that will cause silent failures for users who don't know about the marker.Adding the missing keys to the exclusion list would break
Tempo.extend(plugin, { monthDay: ... })where the second arg is legitimately options. Consider an alternative approach such as only popping the last arg as options whenargs.length > 1, or requiring options to be explicitly marked.🤖 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 518 - 529, Fix isOptionsArg in Tempo.extend so a single discovery object is never inferred and removed as options. Align argument parsing with the overloads by only treating the final argument as inferred options when there is at least one preceding item (or require an explicit options marker), while preserving legitimate calls such as extend(plugin, { monthDay: ... }). Ensure single-argument discovery calls still reach discovery processing and registryUpdate side effects.
587-587: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winForward
optionsto namespace plugin installs
Pluginitself only declaresinstall(this, t), so generic plugins are fine here, butdefineNamespacePlugin()’sinstalltakes an optionaloptionsarg and this branch drops it. Namespace plugins won’t see theextend()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/src/tempo.class.ts` at line 587, Update the plugin installation branch in the relevant method to forward the available extend options as the second argument when invoking `(item as TempoPlugin).install.call(...)`. Preserve the existing context and ensure namespace plugins created by defineNamespacePlugin() receive the options while generic plugins remain compatible.
🤖 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/doc/2-core-concepts/tempo.registry.md`:
- Around line 77-83: Revise the “Registry Merge Contracts” section to remove the
claim that Tempo.extend() universally overrides all registry data. Explain that
registryUpdate() remains additive-only, while Tempo.extend() can shadow existing
entries only for enum/proxy-backed registries such as formats and locales, not
provide a blanket overwrite mechanism for every registry.
---
Outside diff comments:
In `@packages/tempo/src/tempo.class.ts`:
- Around line 518-529: Fix isOptionsArg in Tempo.extend so a single discovery
object is never inferred and removed as options. Align argument parsing with the
overloads by only treating the final argument as inferred options when there is
at least one preceding item (or require an explicit options marker), while
preserving legitimate calls such as extend(plugin, { monthDay: ... }). Ensure
single-argument discovery calls still reach discovery processing and
registryUpdate side effects.
- Line 587: Update the plugin installation branch in the relevant method to
forward the available extend options as the second argument when invoking `(item
as TempoPlugin).install.call(...)`. Preserve the existing context and ensure
namespace plugins created by defineNamespacePlugin() receive the options while
generic plugins remain compatible.
🪄 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: 54855b2d-0eed-4149-8e9a-6109b7701924
📒 Files selected for processing (26)
packages/library/src/common/pledge.class.tspackages/tempo-fns/doc/README.mdpackages/tempo-fns/test/isSameFiscalQuarter.test.tspackages/tempo/doc/2-core-concepts/tempo.format.mdpackages/tempo/doc/2-core-concepts/tempo.registry.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/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/6-utility-library/tempo.decorators.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/extending-temporal.mdpackages/tempo/doc/7-ecosystem/vision.mdpackages/tempo/doc/8-project-and-support/migration-guide.mdpackages/tempo/doc/8-project-and-support/releases/v2.x.mdpackages/tempo/doc/8-project-and-support/releases/v4.x.mdpackages/tempo/package.jsonpackages/tempo/src/plugin/plugin.util.tspackages/tempo/src/plugin/term/term.util.tspackages/tempo/src/tempo.class.tspackages/tempo/test/core/discovery-extend.test.ts
💤 Files with no reviewable changes (1)
- packages/tempo/package.json
✅ Files skipped from review due to trivial changes (15)
- packages/tempo-fns/doc/README.md
- packages/tempo/doc/3-extending-tempo/tempo.term.md
- packages/tempo/doc/6-utility-library/tempo.serializers.md
- packages/tempo/doc/3-extending-tempo/tempo.namespace.md
- packages/tempo/doc/5-architecture-and-internals/lazy-evaluation-pattern.md
- packages/tempo/doc/8-project-and-support/releases/v2.x.md
- packages/tempo/doc/5-architecture-and-internals/architecture.md
- packages/tempo/doc/4-advanced-reference/sandbox-factory.md
- packages/tempo/doc/8-project-and-support/releases/v4.x.md
- packages/tempo/doc/7-ecosystem/extending-temporal.md
- packages/tempo/doc/6-utility-library/tempo.pledge.md
- packages/tempo/doc/2-core-concepts/tempo.format.md
- packages/tempo/doc/6-utility-library/tempo.decorators.md
- packages/tempo/doc/7-ecosystem/vision.md
- packages/tempo/doc/8-project-and-support/migration-guide.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/tempo-fns/test/isSameFiscalQuarter.test.ts
- packages/tempo/src/plugin/term/term.util.ts
- packages/tempo/src/plugin/plugin.util.ts
- packages/tempo/doc/5-architecture-and-internals/soft_freeze_strategy.md
Summary by CodeRabbit
Promise.withResolversand ensured immutability after construction.