Feature/ai api - #68
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTempo 3.11.1 adds RRULE utilities, AI recurrence and scheduling APIs, remote provider manifests, asynchronous cache adapters, timezone parsing updates, generated LLM documentation, release checks, and documentation deployment synchronization. ChangesAI plugin enhancements
Tempo and library functionality
Documentation and release automation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/plugins/ai/src/core/init.ts (1)
21-31: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftApply the manifest before provider defaults are resolved.
initAIstartsloadRemoteManifest()and immediately callsgetResolvedProviderDefaults(). The resolver therefore sees an empty manifest on the first initialization. The manifest result never updates the already stored providers.Make initialization await manifest resolution, or apply resolved defaults after the load completes through an explicit async API. Add a regression test that does not preload the manifest.
packages/plugins/ai/src/core/init.ts#L21-L31: resolve the manifest before storing manifest-derived provider defaults.packages/plugins/ai/test/manifest.test.ts#L94-L121: call the supported initialization flow before manifest resolution, then verify that the remote model is applied.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/core/init.ts` around lines 21 - 31, The initialization flow in packages/plugins/ai/src/core/init.ts lines 21-31 must resolve loadRemoteManifest before getResolvedProviderDefaults stores provider values; update initAI or expose an explicit async path that guarantees manifest completion first. Add a regression test in packages/plugins/ai/test/manifest.test.ts lines 94-121 that uses the supported initialization flow without preloading the manifest and verifies the remote model is applied.
🧹 Nitpick comments (1)
packages/tempo/public/esm_sh.index.html (1)
231-231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPin the Tempo URL in
packages/tempo/public/esm_sh.index.htmlto the package version.This page still loads
https://esm.sh/@magmacomputing/tempo@3, butpackages/tempo/package.jsondeclares version3.11.1. Use the exact Tempo version, or generate the URL during release, so the check runs against the same release code.🤖 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/public/esm_sh.index.html` at line 231, Update the `@magmacomputing/tempo` URL in the esm.sh import configuration to use the exact version declared by the Tempo package, 3.11.1, or wire it to the existing release-generation mechanism so it stays synchronized with packages/tempo/package.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/library/README.md`:
- Line 1: Update the README heading image source to use a repository-relative
path or absolute public URL that resolves correctly when rendered from
packages/library, while preserving the existing logo and heading presentation.
In `@packages/plugins/.bin/check-branch-diff.sh`:
- Around line 30-35: Update the changed-plugin validation in
check-branch-diff.sh to parse and require branch_version to be strictly greater
than main_version as a valid semantic version, rejecting equal, downgraded, or
otherwise invalid versions. Preserve the success status only for valid forward
bumps, and ensure any failure—including “MODIFIED WITHOUT VERSION BUMP!”—sets a
nonzero exit status for the overall check.
In `@packages/plugins/ai/doc/rate-limits.md`:
- Around line 11-12: Update the “Request-Locked Instance Metadata” section for
parseAI so the dt.ai.limits snapshot is guaranteed only for provider-backed
results where the selected provider returns rate-limit headers; explicitly state
that native and cache results may omit limits.
- Around line 145-146: Update the Redis adapter example’s delete and clear
methods to honor the prefix argument and remove namespaced/salted cache keys,
ensuring clear(prefix) performs the corresponding prefix-scoped deletion.
Replace the current no-op clear implementation with a concrete prefix-aware
example, and make delete use the same namespacing scheme so clearAiCache cannot
leave salted entries readable.
In `@packages/plugins/ai/src/core/init.ts`:
- Around line 47-75: Update clearAiCache in
packages/plugins/ai/src/core/init.ts#L47-L75 to return Promise<void>, clear
Tempo.cache for no-input calls, and await every adapter clear and delete
operation before returning while preserving the existing per-input eviction
behavior. Update packages/plugins/ai/CHANGELOG.md#L21-L21 to state the release
behavior only with the corrected completion contract, qualifying it if eviction
is not synchronous.
- Around line 21-26: Harden the provider-resolution flow around
loadRemoteManifest and getResolvedProviderDefaults so manifest-supplied URLs are
accepted only from signed or trusted manifests and match an approved HTTPS
provider origin. Ensure fetchFromProvider rejects redirects for credentialed
requests while preserving the caller API key behavior, and reject invalid
manifest-derived endpoints before they can be resolved or used.
In `@packages/plugins/ai/src/core/manifest.ts`:
- Around line 31-36: Update the manifest initialization flow around
_cachedManifest and _fetchPromise to key cached results and in-flight requests
by the canonical remoteConfigUrl. Only reuse either value when its stored URL
matches the current canonical URL; otherwise fetch and cache the manifest for
the new origin while preserving existing reuse behavior for matching URLs.
- Around line 43-86: Update the initAI initialization flow to await
loadRemoteManifest() before calling getResolvedProviderDefaults(), ensuring the
first initialization uses the fetched manifest when available. Preserve fallback
behavior when loading fails, and update the manifest documentation to describe
that remote defaults are applied during initialization after the manifest load
completes.
In `@packages/plugins/ai/src/core/types.ts`:
- Around line 130-131: Update initAI’s provider-resolution flow, including
getResolvedProviderDefaults, to invoke the AiConfig.fetchDefaults hook for the
requested provider ID and incorporate its returned options before producing
resolved defaults; otherwise remove fetchDefaults from AiConfig. Ensure the
hook’s null result remains valid and existing manifest-based defaults continue
to work.
In `@packages/plugins/ai/src/functions/parse.ts`:
- Around line 247-260: Apply resolvedTtl to the built-in Tempo.cache write in
the parse flow, or otherwise configure that cache tier to enforce the same TTL
precedence as adapter.set. Update packages/plugins/ai/CHANGELOG.md at line 18 to
describe the precedence only for stores that enforce TTL, and update
packages/plugins/ai/README.md at lines 54-55 to clarify the built-in cache’s
separate TTL behavior if it remains independently configured.
In `@packages/tempo/CHANGELOG.md`:
- Line 13: Update the “AI Context & IDE Integration (llms.txt)” changelog entry
to remove the unsupported “zero-hallucination code generation” guarantee,
replacing it with wording that accurately describes providing project context
and improving code-generation accuracy.
In `@packages/tempo/doc/1-getting-started/ai-integration.md`:
- Around line 56-74: Update the AI integration examples to use the documented
Tempo.init({ registry: { layouts: ... } }) layout-registration contract instead
of Tempo.config. Apply this in
packages/tempo/doc/1-getting-started/ai-integration.md lines 56-74 and
packages/tempo/doc/3-extending-tempo/tempo.layout.md lines 104-113, then
regenerate the corresponding sections in packages/tempo/public/llms-full.txt
lines 65-84 and 2461-2470.
In `@packages/tempo/public/esm_sh.index.html`:
- Around line 107-118: Increase the contrast of the .subtitle text by replacing
its current inherited or dark purple text color with a lighter color/token that
achieves at least a 4.5:1 contrast ratio against the dark card background, while
preserving the existing background and layout styling.
- Around line 21-29: Update the body CSS overflow declaration to allow vertical
scrolling while continuing to clip horizontal overflow, so short or zoomed
viewports can reach the card result and footer.
In `@packages/tempo/public/llms-full.txt`:
- Around line 7101-7112: Update the canonical cache documentation in
ai.rate-limits.md to describe support for asynchronous cache adapter methods,
removing the claim that adapters must implement only synchronous Map operations.
Then regenerate llms-full.txt so its Extensible Caching section reflects the
updated async-capable contract and examples.
In `@packages/tempo/src/support/support.cache.ts`:
- Around line 192-194: Update BoundedCache.toJSON() to prevent lossy key
conversion by constraining cache keys to strings or explicitly rejecting
non-string keys before Object.fromEntries(); preserve all valid string-keyed
entries. Add a regression test covering distinct keys such as 1 and "1" so
serialization cannot silently merge entries.
---
Outside diff comments:
In `@packages/plugins/ai/src/core/init.ts`:
- Around line 21-31: The initialization flow in
packages/plugins/ai/src/core/init.ts lines 21-31 must resolve loadRemoteManifest
before getResolvedProviderDefaults stores provider values; update initAI or
expose an explicit async path that guarantees manifest completion first. Add a
regression test in packages/plugins/ai/test/manifest.test.ts lines 94-121 that
uses the supported initialization flow without preloading the manifest and
verifies the remote model is applied.
---
Nitpick comments:
In `@packages/tempo/public/esm_sh.index.html`:
- Line 231: Update the `@magmacomputing/tempo` URL in the esm.sh import
configuration to use the exact version declared by the Tempo package, 3.11.1, or
wire it to the existing release-generation mechanism so it stays synchronized
with packages/tempo/package.json.
🪄 Autofix
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 Plus
Run ID: 0885cb86-da74-4060-bc0c-9211a082a2be
⛔ Files ignored due to path filters (3)
packages/library/img/library-logo.svgis excluded by!**/*.svgpackages/tempo/img/library-logo.svgis excluded by!**/*.svgpackages/tempo/public/library-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (43)
.github/workflows/deploy-docs.ymlpackage.jsonpackages/library/README.mdpackages/library/package.jsonpackages/plugins/.bin/check-branch-diff.shpackages/plugins/.bin/check-versions.shpackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/README.mdpackages/plugins/ai/doc/architecture.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/rate-limits.mdpackages/plugins/ai/package.jsonpackages/plugins/ai/plan/v0.3.0-roadmap.mdpackages/plugins/ai/src/core/init.tspackages/plugins/ai/src/core/manifest.tspackages/plugins/ai/src/core/support.tspackages/plugins/ai/src/core/types.tspackages/plugins/ai/src/functions/parse.tspackages/plugins/ai/src/index.tspackages/plugins/ai/test/cache.test.tspackages/plugins/ai/test/index.spec.tspackages/plugins/ai/test/manifest.test.tspackages/tempo/.vitepress/config.tspackages/tempo/.vitepress/theme/data/catalog.jsonpackages/tempo/CHANGELOG.mdpackages/tempo/bin/expand-typedoc.mjspackages/tempo/bin/generate-llms-txt.mjspackages/tempo/doc/1-getting-started/ai-integration.mdpackages/tempo/doc/1-getting-started/installation.mdpackages/tempo/doc/3-extending-tempo/tempo.layout.mdpackages/tempo/doc/6-utility-library/tempo.library.mdpackages/tempo/package.jsonpackages/tempo/public/bundle.index.htmlpackages/tempo/public/esm_core.index.htmlpackages/tempo/public/esm_full.index.htmlpackages/tempo/public/esm_sh.index.htmlpackages/tempo/public/llms-full.txtpackages/tempo/public/llms.txtpackages/tempo/public/providers.v1.jsonpackages/tempo/public/script.index.htmlpackages/tempo/src/support/support.cache.tspackages/tempo/src/tempo.version.tspackages/tempo/test/support/cache.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/ai/src/core/init.ts`:
- Around line 45-65: The remote manifest flow currently rebuilds providers from
config.providers and discards fetchDefaults results. Update the initialization
logic around the fetchDefaults provider mapping and loadRemoteManifest so the
hook-merged providers are retained through resolveSyncProviders, either by
loading the manifest before applying hookOptions or by passing the merged
provider collection into the final resolution.
- Around line 44-65: Track a configuration revision for each initAI invocation
and capture its value before asynchronous provider/default and remote-manifest
resolution begins. In initAI, guard the assignments to _state.config.providers
around the asyncProviders result and resolveSyncProviders so they apply only
when the captured revision remains current, preventing an older invocation from
overwriting newer provider state.
🪄 Autofix
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 Plus
Run ID: 85c79ff5-f2e7-462f-9a8a-aa724d51ed91
📒 Files selected for processing (19)
packages/library/README.mdpackages/plugins/.bin/check-branch-diff.shpackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/README.mdpackages/plugins/ai/doc/architecture.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/rate-limits.mdpackages/plugins/ai/src/core/init.tspackages/plugins/ai/src/core/manifest.tspackages/plugins/ai/src/core/support.tspackages/plugins/ai/test/manifest.test.tspackages/tempo/CHANGELOG.mdpackages/tempo/bin/update-version.mjspackages/tempo/doc/1-getting-started/ai-integration.mdpackages/tempo/doc/3-extending-tempo/tempo.layout.mdpackages/tempo/public/esm_sh.index.htmlpackages/tempo/public/llms-full.txtpackages/tempo/src/support/support.cache.tspackages/tempo/test/support/cache.test.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/plugins/ai/test/manifest.test.ts
- packages/plugins/.bin/check-branch-diff.sh
- packages/tempo/src/support/support.cache.ts
- packages/tempo/doc/1-getting-started/ai-integration.md
- packages/plugins/ai/doc/architecture.md
- packages/library/README.md
- packages/plugins/ai/CHANGELOG.md
- packages/plugins/ai/src/core/manifest.ts
- packages/tempo/test/support/cache.test.ts
- packages/tempo/doc/3-extending-tempo/tempo.layout.md
- packages/plugins/ai/README.md
- packages/plugins/ai/doc/rate-limits.md
- packages/tempo/CHANGELOG.md
- packages/tempo/public/llms-full.txt
- packages/plugins/ai/doc/index.md
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tempo/public/llms-full.txt (1)
2923-2933: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign Tempo documentation with the actual initialization lifecycle.
Tempo.init()is idempotent after the first global initialization, so the refresh claim is misleading. Also, reset hooks do not replay arbitrary side-effect registrations, so re-callingTempo.init()will not activate late@magmacomputing/tempo-plugin-tickerimports. Update the public examples to useTempo.init({ plugins: [...] })orTempo.extend(...), then regeneratepackages/tempo/public/llms-full.txt.🤖 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/public/llms-full.txt` around lines 2923 - 2933, Update the public Tempo initialization examples at packages/tempo/public/llms-full.txt lines 2923-2933, 2605-2616, and 6552-6577 to use Tempo.init({ plugins: [...] }) or Tempo.extend(...) for plugin registration, and remove the claim that re-calling Tempo.init() refreshes dynamically imported plugins. Regenerate packages/tempo/public/llms-full.txt from the updated source documentation so all affected examples reflect the actual idempotent initialization lifecycle.
🧹 Nitpick comments (1)
packages/tempo/public/llms-full.txt (1)
7145-7148: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplace
KEYSin the production Redis example.
clear()performs a fullKEYSscan and then deletes all matching keys. Redis documentsKEYSas anO(N)dangerous command and recommendsSCANor an indexed set for application code. Use cursor-based scanning with bounded delete batches, or use namespace versioning. Then update the canonical source document and regenerate this bundle. (redis.io)🤖 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/public/llms-full.txt` around lines 7145 - 7148, Replace the redis.keys call in clear with cursor-based SCAN iteration and bounded delete batches while preserving the existing prefix pattern and clearing behavior. Update the canonical source document containing this Redis example, then regenerate packages/tempo/public/llms-full.txt so the bundled example matches the source.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/ai/doc/init.md`:
- Around line 85-87: Update the cache configuration documentation in init.md to
distinguish the cache property, which accepts Map<string, string>, from
cacheAdapter, which accepts AiCacheAdapter. Ensure the example and surrounding
description direct custom distributed-storage adapters to AiConfig.cacheAdapter
rather than AiConfig.cache.
In `@packages/plugins/ai/doc/recurrence.md`:
- Around line 7-14: Update the recurrenceAI example to call initAI before
submitting natural-language input to recurrenceAI, configuring the provider as
required while preserving the existing locale and count options.
In `@packages/plugins/ai/src/functions/recurrence.ts`:
- Around line 111-126: Update fetchFromProvider and both recurrence call sites
in the mode branches to accept and use a recurrence-specific system prompt or
request payload, ensuring recurrenceAI sends only the recurrence schema rather
than the helper’s default date-parser and iso instructions. Extend the
recurrence tests with a request-body assertion verifying the provider receives
the recurrence-specific schema without conflicting requirements.
- Around line 111-126: Update the recurrence request flow around
fetchFromProvider so TempoRecurrenceOptions.mode and minConfidence are honored
like parseAI: apply the configured confidence threshold before accepting
fallback responses, execute providers in parallel for race mode, and aggregate
responses using consensus mode. Preserve provider and raw-content assignment
only from an accepted result, and retain the existing debug logging for failed
providers.
- Around line 53-69: Replace the consecutive-day generation in the recurrence
builder, including take and createIterator, with RFC 5545 evaluation of the
RRULE from the anchor, applying the after and before window. Derive isFinite,
size, take results, and iterator output from the expanded occurrence set so
weekly rules, UNTIL limits, and other RRULE constraints are honored.
---
Outside diff comments:
In `@packages/tempo/public/llms-full.txt`:
- Around line 2923-2933: Update the public Tempo initialization examples at
packages/tempo/public/llms-full.txt lines 2923-2933, 2605-2616, and 6552-6577 to
use Tempo.init({ plugins: [...] }) or Tempo.extend(...) for plugin registration,
and remove the claim that re-calling Tempo.init() refreshes dynamically imported
plugins. Regenerate packages/tempo/public/llms-full.txt from the updated source
documentation so all affected examples reflect the actual idempotent
initialization lifecycle.
---
Nitpick comments:
In `@packages/tempo/public/llms-full.txt`:
- Around line 7145-7148: Replace the redis.keys call in clear with cursor-based
SCAN iteration and bounded delete batches while preserving the existing prefix
pattern and clearing behavior. Update the canonical source document containing
this Redis example, then regenerate packages/tempo/public/llms-full.txt so the
bundled example matches the source.
🪄 Autofix
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 Plus
Run ID: 40c265af-63e7-49d3-af3b-acb487dbd38e
📒 Files selected for processing (13)
packages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/init.mdpackages/plugins/ai/doc/parse.mdpackages/plugins/ai/doc/recurrence.mdpackages/plugins/ai/src/core/init.tspackages/plugins/ai/src/core/support.tspackages/plugins/ai/src/core/types.tspackages/plugins/ai/src/functions/parse.tspackages/plugins/ai/src/functions/recurrence.tspackages/plugins/ai/src/index.tspackages/plugins/ai/test/parse.test.tspackages/plugins/ai/test/recurrence.test.tspackages/tempo/public/llms-full.txt
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/plugins/ai/src/index.ts
- packages/plugins/ai/src/core/support.ts
- packages/plugins/ai/src/functions/parse.ts
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
packages/plugins/ai/test/recurrence.test.ts (2)
12-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the
fetchspy after each test.
vi.clearAllMocks()clears recorded calls. It does not remove the implementations installed byvi.spyOn(globalThis, 'fetch')at Lines 61, 102, and 136. The mock at Line 103 usesmockImplementation, so it stays active for every later test in the file and for any suite that shares the same global.Call
vi.restoreAllMocks()so each test starts from the realfetch.♻️ Proposed change
afterEach(() => { - vi.clearAllMocks(); + vi.restoreAllMocks(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/recurrence.test.ts` around lines 12 - 14, Update the afterEach cleanup near vi.clearAllMocks() to call vi.restoreAllMocks(), ensuring fetch spies and their implementations are removed after every test and the real global fetch is restored before the next test.
101-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the distinguishing behavior of race and consensus modes.
Both providers return the same payload, so
resRace.rruleandresConsensus.rrulepass even if the mode branch is wrong. The test cannot separateracefromconsensusexcept through Line 132.Add assertions that identify each mode. For
race, give the two providers different latencies and differentrrulevalues, then assert the faster value wins and that the slower request receives an abort signal. Forconsensus, add a case where the providers disagree and assert that the highest-confidence result is selected and thatconfidenceis not raised to1.0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/recurrence.test.ts` around lines 101 - 133, Strengthen the test `should support provider race and consensus execution modes in recurrenceAI` so each mode has distinguishable behavior: configure race providers with different delays and rrule values, assert the faster result is returned, and verify the slower request receives an abort signal. Add a disagreement case for consensus providers, then assert the highest-confidence result and its original confidence value are selected rather than being elevated to 1.0.packages/plugins/ai/src/functions/recurrence.ts (1)
157-170: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
take()re-expands the whole series on every call.Line 162 expands
offsetCursor + actualCountoccurrences and then discards the leadingoffsetCursorentries. Paging through a series therefore costs O(n²)Tempoconstructions. Line 150 adds a further 1000-occurrence expansion for finite rules withoutCOUNT.Cache the expanded occurrences on the closure and extend the cache only when the cursor passes its end.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/functions/recurrence.ts` around lines 157 - 170, The take function should cache expanded occurrences in its closure instead of rebuilding and discarding the entire prefix on every call. Add an occurrences cache, extend it only when offsetCursor + actualCount exceeds the cached length, and slice the requested batch from that cache while preserving finite size limits and cursor advancement; also avoid redundant expansion of the finite-rule 1000-occurrence baseline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/ai/src/functions/recurrence.ts`:
- Around line 83-117: Update parseRRule and expandOccurrences to support every
BYDAY, BYHOUR, and BYMINUTE value by generating the cartesian product for each
recurrence period rather than using index 0. Add consistent BYMONTH and BYSETPOS
parsing and expansion so generated occurrences match the returned rrule, or
remove those keys from the provider prompt and document the supported subset. In
the monthly BYDAY calculation, correctly resolve negative ordinals such as -1FR
as the last matching weekday instead of treating them as the first.
- Around line 172-181: The createIterator generator currently expands only one
batch and truncates infinite or bounded recurrence series. Refactor
createIterator to lazily request successive occurrence pages, yielding each page
before fetching the next, and continue until the configured size limit or
before/UNTIL boundary is reached. Ensure COUNT rules are not prematurely
truncated by batchSize and preserve the documented lazy-generator behavior of
TempoRecurrenceResult.
- Around line 380-383: Update the rrule handling in the recurrence parsing flow
to require parsedData.rrule to be a non-empty string after trimming; otherwise
throw a TempoAiError instead of defaulting to FREQ=DAILY. Preserve the existing
trimmed rrule value for valid input and leave confidence handling unchanged.
- Around line 46-61: Update the recurrence parser around UNTIL, COUNT, BYHOUR,
and BYMINUTE to support RFC 5545 date-only UNTIL values by constructing a valid
date boundary without empty time components, while preserving full datetime
handling. Validate every parsed numeric field and avoid assigning NaN: use the
existing safe default behavior for INTERVAL and establish appropriate
finite-value handling for COUNT, BYHOUR, and BYMINUTE so expandOccurrences and
createRecurrenceResult never receive NaN.
- Around line 304-325: Add a no-op rejection handler to every promise created in
the Race branch’s availableProviders.map callback before passing the collection
to Promise.race. Keep the existing Promise.race result and abort behavior
unchanged, while ensuring slower provider promises cannot produce unhandled
rejections after parentController.abort().
- Around line 74-128: Update the recurrence loop around maxToFetch,
countProduced, and the afterTempo filter so rule.count limits occurrences
generated from the anchor rather than occurrences returned after filtering.
Increment the COUNT tracking for every valid generated candidate before applying
afterTempo/beforeTempo window filters, while preserving window filtering and
termination behavior; ensure the COUNT-based series ends at the correct
occurrence regardless of after.
- Around line 375-379: Validate the mode before the successfulResult
destructuring in the recurrence execution flow, covering both _state.config.mode
and options.mode inputs. Reject any value outside Fallback, Race, or Consensus
with a clear contextual error, and only destructure successfulResult after that
validation guarantees it is non-null.
In `@packages/plugins/ai/test/recurrence.test.ts`:
- Around line 153-169: Strengthen the recurrenceAI window test by replacing the
non-empty length assertion and redundant bounds loop with an exact expectation
of three occurrences: 2026-08-03, 2026-08-04, and 2026-08-05, each at 09:00.
Keep the isFinite assertion and use the existing items result to verify both
count and dates, covering the COUNT=10 and after-window interaction.
In `@packages/tempo/doc/3-extending-tempo/tempo.modularity.md`:
- Around line 120-123: Document one consistent plugin-registration lifecycle: in
packages/tempo/doc/3-extending-tempo/tempo.modularity.md lines 120-123, align
the Tempo.init() note with the guide’s “initial discovery” wording, clarifying
when automatic discovery and explicit registration occur; in
packages/tempo/doc/3-extending-tempo/tempo.plugin.md lines 107-108, reconcile
the Tempo.extend() guidance with the automatic-registration statement so users
know which operation applies at startup versus runtime.
---
Nitpick comments:
In `@packages/plugins/ai/src/functions/recurrence.ts`:
- Around line 157-170: The take function should cache expanded occurrences in
its closure instead of rebuilding and discarding the entire prefix on every
call. Add an occurrences cache, extend it only when offsetCursor + actualCount
exceeds the cached length, and slice the requested batch from that cache while
preserving finite size limits and cursor advancement; also avoid redundant
expansion of the finite-rule 1000-occurrence baseline.
In `@packages/plugins/ai/test/recurrence.test.ts`:
- Around line 12-14: Update the afterEach cleanup near vi.clearAllMocks() to
call vi.restoreAllMocks(), ensuring fetch spies and their implementations are
removed after every test and the real global fetch is restored before the next
test.
- Around line 101-133: Strengthen the test `should support provider race and
consensus execution modes in recurrenceAI` so each mode has distinguishable
behavior: configure race providers with different delays and rrule values,
assert the faster result is returned, and verify the slower request receives an
abort signal. Add a disagreement case for consensus providers, then assert the
highest-confidence result and its original confidence value are selected rather
than being elevated to 1.0.
🪄 Autofix
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 Plus
Run ID: 3e34fed2-66aa-402f-80c9-3b4c95d09998
📒 Files selected for processing (10)
packages/plugins/ai/doc/init.mdpackages/plugins/ai/doc/rate-limits.mdpackages/plugins/ai/doc/recurrence.mdpackages/plugins/ai/src/core/support.tspackages/plugins/ai/src/core/types.tspackages/plugins/ai/src/functions/recurrence.tspackages/plugins/ai/test/recurrence.test.tspackages/tempo/doc/3-extending-tempo/tempo.modularity.mdpackages/tempo/doc/3-extending-tempo/tempo.plugin.mdpackages/tempo/public/llms-full.txt
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/plugins/ai/doc/recurrence.md
- packages/plugins/ai/doc/init.md
- packages/plugins/ai/doc/rate-limits.md
- packages/plugins/ai/src/core/support.ts
- packages/plugins/ai/src/core/types.ts
- packages/tempo/public/llms-full.txt
|
@CodeRabbit full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/plugins/ai/test/parse.test.ts (1)
10-24: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAwait
initAIand disable the remote manifest in the tests.
initAInow returns a promise and startsloadRemoteManifestwheneverremoteConfigUrlis neither set norfalse(packages/plugins/ai/src/core/init.tslines 47-52). Two consequences apply here:
- The hook does not return the promise, so each test begins while the manifest load is still pending. That pending load can overwrite
_state.config.providersmid-test through init.ts lines 70-74. The assertion at line 46 on the compiled default model is exposed to this race.- The manifest load calls
fetch. When a test installs afetchspy withmockResolvedValueOnce, the manifest request can consume the queued response that the test intended for a provider call.Set
remoteConfigUrl: falseand return the promise from the hook.🛠️ Proposed fix
- beforeEach(() => { + beforeEach(async () => { vi.spyOn(console, 'warn').mockImplementation(() => {}); vi.spyOn(console, 'error').mockImplementation(() => {}); vi.spyOn(console, 'log').mockImplementation(() => {}); if (isLiveTest) { - initAI({ - providers: [{ id: liveProviderId, key: liveApiKey! }] - }); + await initAI({ + providers: [{ id: liveProviderId, key: liveApiKey! }], + remoteConfigUrl: false + }); } else { - initAI({ - providers: [{ id: 'groq', key: 'mock-key-for-unit-testing' }] - }); + await initAI({ + providers: [{ id: 'groq', key: 'mock-key-for-unit-testing' }], + remoteConfigUrl: false + }); } });Apply the same
remoteConfigUrl: falseto theinitAIcall at line 31 and to the other in-testinitAIcalls.Also applies to: 30-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/parse.test.ts` around lines 10 - 24, Update the beforeEach hook and all other initAI calls in this test to await initialization by returning its promise, and pass remoteConfigUrl: false in each call. Ensure no test starts before initAI completes and remote manifest loading is disabled so fetch mocks remain reserved for provider requests.packages/plugins/ai/src/index.ts (1)
35-36: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stale
scheduleAIscaffold entry.Line 15 exports
scheduleAIfrom./functions/schedule.js. The commented scaffold at Lines 35-36 still listsscheduleAIunder "Upcoming AI Function Exports". The comment now contradicts the active export.🧹 Proposed cleanup
-// /** Resolves natural language scheduling prompts into optimal Tempo intervals */ -// export { scheduleAI, type TempoInterval } from './functions/schedule.js'; -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/index.ts` around lines 35 - 36, Remove the commented-out scheduleAI scaffold entry under “Upcoming AI Function Exports” in the package index, while leaving the active scheduleAI export unchanged.
🟡 Minor comments (23)
packages/tempo/public/llms.txt-27-27 (1)
27-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a base-safe link for
llms-full.txt.Line 27 uses a root-relative URL. The documentation site uses
base: '/magma/', so this link resolves outside the deployed site path. Use a relative link such asllms-full.txt. (raw.githubusercontent.com)Proposed fix
-- [Full Documentation Concatenation](/llms-full.txt): Complete raw markdown documentation for RAG ingestion. +- [Full Documentation Concatenation](llms-full.txt): Complete raw markdown documentation for RAG ingestion.🤖 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/public/llms.txt` at line 27, Update the Full Documentation Concatenation link in llms.txt to use the base-safe relative target llms-full.txt instead of the root-relative /llms-full.txt path.packages/plugins/ai/README.md-44-45 (1)
44-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAwait asynchronous cache eviction.
clearAiCache()returnsPromise<void>so a customAiCacheAdaptercan finish eviction asynchronously. This example starts eviction but does not wait for completion. Useawait clearAiCache(...)or show explicit promise handling.Proposed fix
-clearAiCache("The penultimate Tuesday before Thanksgiving in 2026"); +await clearAiCache("The penultimate Tuesday before Thanksgiving in 2026");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/README.md` around lines 44 - 45, Update the README example around clearAiCache to await its returned Promise, using await clearAiCache(...) so asynchronous eviction completes before execution continues.packages/plugins/ai/doc/recurrence.md-81-106 (1)
81-106: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument
reasoninginTempoRecurrenceResult.The public result type includes optional
reasoning, but this interface block omits it. The guide presents the block as the result contract. Add the field or state that the block is partial.Proposed addition
/** Provider ID responsible for processing or 'rrule-parser' */ provider: string; + + /** Reasoning / explanation of the recurrence pattern */ + reasoning?: string; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/doc/recurrence.md` around lines 81 - 106, Add the optional reasoning field to the TempoRecurrenceResult interface alongside the other result metadata, matching the public result type’s existing type and documentation; do not leave the documented contract incomplete.packages/plugins/ai/plan/v0.3.0-roadmap.md-7-30 (1)
7-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the roadmap to match the v0.3.0 contract.
The document describes v0.3.0 work as future implementation, although the changelog marks
scheduleAIandrecurrenceAIas released. The recurrence signature is also stale: the current contract usesPromise<TempoRecurrenceResult>and.take(count), notPromise<TempoRecurrenceRule>and.next(count). Mark completed handlers or move the remaining requirements to a future roadmap.Proposed recurrence corrections
-### 1.5 ✅ `recurrenceAI(prompt: string, options?: AiOptions): Promise<TempoRecurrenceRule>` +### 1.5 ✅ `recurrenceAI(prompt: string, options?: TempoRecurrenceOptions): Promise<TempoRecurrenceResult>` -* Translates complex natural language repeating schedule descriptions into standard RRULE strings and `Tempo` instance date generators (`rule.next(count)`). +* Translates complex natural language repeating schedule descriptions into standard RRULE strings and paged `Tempo` batches (`result.take(count)`).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/plan/v0.3.0-roadmap.md` around lines 7 - 30, Update the v0.3.0 roadmap to reflect the released status of scheduleAI and recurrenceAI, marking completed handlers accordingly or moving unfinished requirements to a later roadmap. Correct recurrenceAI to return Promise<TempoRecurrenceResult> and describe recurrence generation with rule.take(count) instead of Promise<TempoRecurrenceRule> and rule.next(count), while preserving accurate entries for remaining handlers.packages/plugins/ai/doc/init.md-73-90 (1)
73-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winComplete the
AiConfigreference.This block omits
remoteConfigUrl, whichinitAI()consumes andarchitecture.mddocuments. It also omitsttl, which the cache configuration example uses inrate-limits.md. Add these fields and verify the remaining exported options, or label this block as a partial excerpt.Proposed additions
export interface AiConfig { providers?: AiProvider[]; mode?: 'fallback' | 'race' | 'consensus'; timeout?: number; debug?: boolean; cache?: Map<string, string>; cacheAdapter?: AiCacheAdapter; + ttl?: number; + remoteConfigUrl?: string | false; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/doc/init.md` around lines 73 - 90, Complete the AiConfig reference by adding the exported remoteConfigUrl and ttl options consumed by initAI() and used in the cache configuration example. Verify the interface against the remaining documented/exported AiConfig fields, or explicitly label the block as a partial excerpt if it is not intended to be exhaustive.packages/plugins/ai/doc/rate-limits.md-141-144 (1)
141-144: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve a zero Redis TTL in the adapter example.
AiCacheAdapter.set()accepts an optional numericttlMs, and the parser calls it with the resolved TTL.if (ttlMs)skips0, soredis.set()stores the key withoutpx. UsettlMs !== undefinedwhen0means a valid non-expiring value.Proposed fix
- if (ttlMs) await redis.set(`tempo:ai:${key}`, value, { px: ttlMs }); + if (ttlMs !== undefined) await redis.set(`tempo:ai:${key}`, value, { px: ttlMs });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/doc/rate-limits.md` around lines 141 - 144, Update the AiCacheAdapter.set example to check ttlMs !== undefined instead of relying on truthiness, so a resolved TTL of 0 is passed to redis.set via the px option while an omitted TTL still uses the no-options call.packages/plugins/ai/src/core/init.ts-56-75 (1)
56-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRead
fetchDefaultsfrom merged state, not only from the current call.Line 56 tests
config.fetchDefaults. The merge at lines 37-41 storesfetchDefaultson_state.config, but no code reads it. A caller that registers the hook once and supplies providers in a laterinitAIcall silently skips the hook:await initAI({ fetchDefaults: myHook }); // hook stored, no providers await initAI({ providers: [{ id: 'groq', key }] }); // hook ignoredResolve the hook from the merged state so the behavior matches the persisted configuration.
🛠️ Proposed fix
- if (config.fetchDefaults && config.providers) { + const fetchDefaults = config.fetchDefaults ?? _state.config.fetchDefaults; + if (fetchDefaults && config.providers) { const asyncProviders = await Promise.all(config.providers.map(async p => { const normalizedId = p.id?.toLowerCase() ?? ''; const defaults = getResolvedProviderDefaults(normalizedId, remoteUrl, config.debug ?? _state.config.debug); let hookOptions: Partial<AiProvider> | null = null; try { - hookOptions = await config.fetchDefaults!(normalizedId); + hookOptions = await fetchDefaults(normalizedId); } catch { }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/core/init.ts` around lines 56 - 75, Update the provider initialization branch in initAI to read fetchDefaults from the merged _state.config rather than only the current config argument. Use that persisted hook when providers are supplied in a later call, while preserving the existing defaults resolution, error handling, and revision checks.packages/plugins/ai/src/core/support.ts-148-153 (1)
148-153: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBound the provider error body before it is placed in the error message.
await response.text()reads the whole response body with no size limit, and the full text is embedded in theTempoAiErrormessage. A misbehaving or hostile endpoint can return a very large body, which is then buffered into a string and propagated to every caller and log sink. Provider error bodies can also echo submitted prompt content.Truncate the text before you build the message.
🛠️ Proposed fix
if (!response.ok) { - const errorText = await response.text(); + const rawText = await response.text().catch(() => ''); + const errorText = rawText.length > 512 ? `${rawText.slice(0, 512)}…[truncated]` : rawText; const resetTime = limits?.resetAt ?? undefined; _state.limits = limits; throw new TempoAiError(`Provider ${provider.id} failed with status ${response.status}. Details: ${errorText}`, response.status, resetTime); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/core/support.ts` around lines 148 - 153, Bound the provider error body in the non-OK response branch of the request flow before constructing TempoAiError. Truncate errorText to a reasonable maximum while preserving the existing status, resetTime, and error-message context, and use the bounded text in the exception message.packages/plugins/ai/src/functions/schedule.ts-243-255 (1)
243-255: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReturn the first successful provider in race mode, not the first settled one.
Promise.racesettles with the first promise that settles, including a rejection. If the fastest provider returns a 500, the whole call fails while slower providers are still able to succeed. The catch block then reportsAll providers failed in race mode, which does not match what happened.Use
Promise.any, which resolves with the first fulfillment and rejects with anAggregateErroronly when every provider fails.🛠️ Proposed fix
} else if (mode === AiMode.Race || mode === 'race') { const parentController = new AbortController(); try { const promises = availableProviders.map(p => executeProviderCall(p, parentController.signal)); promises.forEach(p => p.catch(() => { })); - selectedResult = await Promise.race(promises); + selectedResult = await Promise.any(promises); parentController.abort(); } catch (aggregateErr: any) { parentController.abort(); + const firstErr = aggregateErr instanceof AggregateError ? aggregateErr.errors[0] : aggregateErr; - throw aggregateErr instanceof TempoAiError - ? aggregateErr - : new TempoAiError(`All providers failed in race mode: ${aggregateErr.message}`, 502); + throw firstErr instanceof TempoAiError + ? firstErr + : new TempoAiError(`All providers failed in race mode: ${firstErr?.message}`, 502); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/functions/schedule.ts` around lines 243 - 255, Update the race-mode branch around executeProviderCall and parentController to use Promise.any instead of Promise.race, so selectedResult receives the first successful provider and failures are aggregated only after all providers reject. Preserve abort behavior on both success and failure, and adapt error handling to safely report the AggregateError when every provider fails.packages/plugins/ai/src/functions/schedule.ts-23-28 (1)
23-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard against an
undefinedtitle.Line 27 tests only for key presence. If an event carries
title: undefined,String(undefined)assigns the literal string'undefined'. Theb.title || 'Busy'fallbacks at line 69 and line 308 do not catch a non-empty string, so'undefined'reaches the LLM prompt and the user-facing conflict message.🛠️ Proposed fix
- if ('title' in evt) title = String((evt as any).title); - else if ('label' in evt) title = String((evt as any).label); + const rawTitle = (evt as any).title ?? (evt as any).label; + if (rawTitle !== undefined && rawTitle !== null) title = String(rawTitle);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/functions/schedule.ts` around lines 23 - 28, Update the title extraction in the event parsing branch around parsePoint so title: undefined does not become the literal string "undefined"; require a defined, usable title value before converting and assigning it, while preserving the label fallback and allowing the existing "Busy" fallbacks to apply when neither is usable.packages/plugins/ai/src/functions/parse.ts-12-12 (1)
12-12: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStrip the AI-only options from
coreOptions.Line 12 removes
force,debug,mode,providers,minConfidence,softErrors,cache, andtimeout. It leavesanchor,ttl, andcacheAdapterincoreOptions.coreOptionsis then spread into theTempoconstructor at lines 30, 69, 84, 231, and 264.That passes a
cacheAdapterobject and an AI cachettlinto the core parser, and it passesanchoralongside the already-resolvedanchorStrat line 30.AiParseOptionsalso declares[key: string]: any, so any extra AI option reachesTempoas well.🛠️ Proposed fix
- const { force, debug, mode: aiMode, providers, minConfidence, softErrors, cache: aiCacheOption, timeout: callTimeout, ...coreOptions } = options || {}; + const { force, debug, mode: aiMode, providers, minConfidence, softErrors, cache: aiCacheOption, + timeout: callTimeout, anchor: _anchor, ttl: _ttl, cacheAdapter: _cacheAdapter, ...coreOptions } = 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/plugins/ai/src/functions/parse.ts` at line 12, Update the options destructuring in the parse flow so coreOptions contains only options supported by Tempo, removing anchor, ttl, cacheAdapter, and any other AI-specific fields before coreOptions is spread into each Tempo constructor. Preserve the existing extraction of AI options and ensure the resolved anchorStr remains the sole anchor value passed to Tempo.packages/plugins/ai/src/core/manifest.ts-44-53 (1)
44-53: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear the abort timer on every path.
clearTimeout(timer)runs only afterfetchresolves. Iffetchrejects, or ifresponse.json()throws, the timer stays pending fortimeoutMs. Each failed call leaks a timer handle and keeps the Node event loop alive. Move the declaration outside thetryand clear it in afinallyblock.🛠️ Proposed fix
const fetchPromise = (async () => { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); try { - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), timeoutMs); - const response = await fetch(targetUrl, { signal: controller.signal, headers: { Accept: 'application/json' } }); - clearTimeout(timer); - if (!response.ok) {Then add the clear to the existing
finallyblock:} finally { + clearTimeout(timer); _fetchPromiseMap.delete(targetUrl); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/core/manifest.ts` around lines 44 - 53, Update the fetch flow in the surrounding manifest function so the timer declared alongside the AbortController is always cleared, including when fetch or response.json rejects. Move timer cleanup into the existing finally block and remove the success-only clearTimeout call, preserving the current timeout and response handling.packages/plugins/ai/src/functions/recurrence.ts-312-312 (1)
312-312: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
reasoningis dropped unlessdebugis set.Line 312 passes
reasoningonly whenisDebugis true. The native RRULE path at Line 130 always passes a reasoning string.TempoRecurrenceResult.reasoningdocuments the field as the explanation of the recurrence pattern, with no debug condition.A caller that reads
result.reasoninggets a value for raw RRULE input andundefinedfor the same request routed to a provider. Either passreasoningunconditionally, or document the debug requirement on the type and gate the native path the same way.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/functions/recurrence.ts` at line 312, Update the provider recurrence result construction around TempoRecurrenceResult so reasoning is passed unconditionally, matching the native RRULE path and the type’s documented contract; remove the isDebug gate from the reasoning field while preserving any separate debug-only fields.packages/plugins/ai/test/recurrence.test.ts-228-235 (1)
228-235: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the exact occurrence count before indexing.
Line 230 checks only
length > 0. Lines 232 and 234 then indexitems[0]anditems[1]. If the expansion returns one occurrence, the test fails with aTypeErroronundefined.formatinstead of a clear count mismatch.
FREQ=MONTHLY;BYDAY=-1FR;UNTIL=20261231from the2026-08-01anchor produces the last Friday of August through December, sotake(5)should return 5 items. Assert that count, and assertsize.💚 Proposed change
expect(result.isFinite).toBe(true); const items = result.take(5); - expect(items.length).toBeGreaterThan(0); + expect(items).toHaveLength(5);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/recurrence.test.ts` around lines 228 - 235, Update the recurrence test around result.take(5) to assert that items.length equals 5 instead of only being greater than zero, and assert the recurrence result’s size is 5 before indexing items[0] through items[4].packages/plugins/ai/test/schedule.test.ts-48-61 (1)
48-61: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winExpose
Interval<Tempo>in thescheduleAIresult typing.
scheduleAIreturns anInterval<Tempo>wrapper, whileTempoScheduleResultonly exposes the{ start: Tempo; end: Tempo }shape andTempoInterval[]alternatives. Callers that useIntervalmethods on the slot or alternatives need an unsafe cast. Declare the result and alternatives in terms ofInterval<Tempo>, or document the concreteIntervalcontract clearly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/schedule.test.ts` around lines 48 - 61, Update the scheduleAI result typing and its TempoScheduleResult contract to expose slot and alternatives as Interval<Tempo> rather than plain start/end and TempoInterval[] shapes. Preserve the existing runtime Interval behavior so callers can use Interval methods without casts, including the alternatives collection.packages/plugins/ai/src/functions/recurrence.ts-32-40 (1)
32-40: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCompute
sizefrom the expanded window whenafterorbeforeis supplied.
expandRRuleEpochsstops the series after producingCOUNTgenerated occurrences from the anchor, then appliesafterMs/beforeMsfiltering before incrementingresultsCount. ForFREQ=DAILY;COUNT=10with an after/before window,result.sizecurrently reports 10 even though the windowed size is smaller. Use the expansion length when a window is supplied, while still respectingrule.countas the series limit if the window does not narrow 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/plugins/ai/src/functions/recurrence.ts` around lines 32 - 40, Update the size calculation in the recurrence expansion flow so supplying options.after or options.before derives size from the filtered expandOccurrences result, while retaining rule.count as the series limit when the window does not reduce it. Preserve the existing finite-rule and infinite-rule handling, including Number.POSITIVE_INFINITY for unbounded expansions.packages/tempo/public/esm_sh.index.html-217-219 (1)
217-219: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose the result as a live region.
The page changes
#resultafter module execution, but the element is a plaindiv. Addrole="status"oraria-live="polite"so screen readers announce the result and error message.Proposed fix
- <div id="result" class="result pulse-loading">Initializing Temporal...</div> + <div id="result" class="result pulse-loading" role="status" aria-live="polite">Initializing Temporal...</div>🤖 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/public/esm_sh.index.html` around lines 217 - 219, Update the result element in the output panel to expose dynamic content as an accessible live region by adding role="status" or aria-live="polite"; preserve its existing id, classes, and initialization text.packages/tempo/public/esm_sh.index.html-236-238 (1)
236-238: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle static module imports outside the instantiation
try.The
import '@js-temporal/polyfill'andimport { Tempo } from '@magmacomputing/tempo'statements run before thetryblock. If either module or its import map fails to resolve/load, that exception does not enter thecatch, so the page can stay atInitializing Temporal...while the console error remains unhandled by the UI.Use top-level
try/catcharoundawait import(...), or add awindow.addEventListener('error', ...)handler for module-load failures, so this path always updates the visible result.🤖 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/public/esm_sh.index.html` around lines 236 - 238, Move the static imports for `@js-temporal/polyfill` and Tempo out of the module’s top-level declarations and dynamically import them within the existing initialization try/catch. Ensure resolution or loading failures reach the catch handler so the visible result is updated instead of remaining at “Initializing Temporal...”.packages/tempo/doc/1-getting-started/ai-integration.md-56-56 (1)
56-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse one documented month token in both AI guides.
Both changed prompts tell AI assistants to use
{mon}, but the available snippet table defines the month token as{mm}. This inconsistency can produce layouts that do not match the documented grammar.
packages/tempo/doc/1-getting-started/ai-integration.md#L56-L56: replace{mon}with{mm}in the named-token list.packages/tempo/doc/3-extending-tempo/tempo.layout.md#L108-L109: replace{mon}with{mm}in the AI prompt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/doc/1-getting-started/ai-integration.md` at line 56, Replace the undocumented {mon} month token with the documented {mm} token in the named-token list in packages/tempo/doc/1-getting-started/ai-integration.md:56-56 and in the AI prompt in packages/tempo/doc/3-extending-tempo/tempo.layout.md:108-109, keeping both guides consistent with the snippet table.packages/tempo/doc/2-core-concepts/tempo.parse.md-130-143 (1)
130-143: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the
{tzd}reference aligned with the implementation.
{tzd}already accepts registered timezone abbreviations such asAESTandPST, so this parsing example is valid. Updatepackages/tempo/doc/3-extending-tempo/tempo.layout.mdso the token reference describes{tzd}as accepting offset designators and registered timezone abbreviations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/doc/2-core-concepts/tempo.parse.md` around lines 130 - 143, Update the `{tzd}` token reference in the layout documentation to state that it accepts both timezone offset designators and registered timezone abbreviations such as AEST and PST. Keep the description aligned with the existing parser behavior and avoid changing the parsing example.packages/tempo/doc/1-getting-started/ai-integration.md-24-25 (1)
24-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not list
.cursorrulesin the VS Code/Copilot setup.This is Copilot Chat configuration, and
.github/copilot-instructions.mdis the VS Code Copilot workspace instruction path..cursorrulesis a Cursor/legacy file, so keep this guide focused on Copilot or move the Cursor file to the Cursor section.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/doc/1-getting-started/ai-integration.md` around lines 24 - 25, Update the “VS Code & GitHub Copilot” section to mention only .github/copilot-instructions.md as the workspace instruction file; remove .cursorrules from this setup guidance, leaving any Cursor-specific guidance to its appropriate section.packages/tempo/src/module/module.parse.ts-263-264 (1)
263-264: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLimit the date-prefix guard bypass.
The bypass accepts inputs like
2024-99-99and2024-01-01abc, allowing invalid or partially matching text to reach layout parsing. Require valid month/day ranges and enforce a boundary after the date component.🤖 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/module/module.parse.ts` around lines 263 - 264, Update the date-prefix check in the module parsing guard so it only bypasses the guard for valid calendar-shaped prefixes: restrict month and day to valid ranges and require the date to end at a boundary rather than accepting trailing letters or other partial text. Keep the existing guard assignment behavior for genuinely valid date prefixes.packages/tempo/src/support/support.default.ts-37-37 (1)
37-37: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire
[+-]in the short timezone-offset branch.
Match.offsetis embedded intoToken.tzd, and the short-offset branch allows a missing sign. BecauseGMT 10:30andUTC 1030are classified as timezone input, setMatch.offsetto require+or-for numeric short offsets so clock values cannot collide here. Also applies to line 71.🤖 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/support/support.default.ts` at line 37, Update the short numeric-offset branch of Match.offset in the offset definitions at both referenced locations to require an explicit + or - sign before the hour, while preserving the existing colon and four-digit offset formats and GMT/UTC prefix handling.
🧹 Nitpick comments (11)
packages/plugins/ai/test/manifest.test.ts (1)
10-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset the shared
_statebetween tests.
resetManifestCache()clears only the manifest maps.initAImutates the module-level_stateinpackages/plugins/ai/src/core/init.ts, and that object persists across tests and across test files in the same worker. The test at line 161 leaves_state.config.remoteConfigUrlset tohttps://tempo.magmacomputing.com.au/manifest-2.json. Any later test that callsinitAIwithoutremoteConfigUrlinherits that URL through line 22 ofinit.tsand resolves defaults from the wrong cache key.Reset the configuration in
beforeEachso each test starts from a known baseline.♻️ Proposed change
beforeEach(() => { resetManifestCache(); vi.restoreAllMocks(); + // isolate the shared module-level _state between tests + initAI({ providers: [], remoteConfigUrl: false, fetchDefaults: undefined }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/manifest.test.ts` around lines 10 - 18, Update the test setup around beforeEach in manifest.test.ts to reset the shared _state configuration mutated by initAI, including clearing config.remoteConfigUrl, before each test. Keep resetManifestCache() and mock restoration intact so every test starts with the default configuration and cannot inherit a prior remote manifest URL.packages/plugins/ai/test/schedule.test.ts (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the commented-out spy.
Line 10 leaves a disabled
console.errorspy in place. Lines 9 and 11 keep the other two spies active. Delete the line, or restore it so the suite silencesconsole.errorconsistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/schedule.test.ts` at line 10, Remove the commented-out console.error spy near the existing active spies in the schedule test, leaving the active spy setup unchanged.packages/plugins/ai/src/types/schedule.type.ts (1)
74-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated AI metadata shape into a named interface.
The
aiobject literal is declared twice with identical members, inTempoScheduleResult(Lines 75-82) andTempoScheduleMeta(Lines 103-110). A named interface keeps the two declarations in sync.♻️ Proposed refactor
+/** + * ## TempoScheduleAiMeta + * Extended AI execution metadata attached to scheduling results. + */ +export interface TempoScheduleAiMeta { + provider: string; + confidence: number; + conflictBumped?: boolean | undefined; + originalSlot?: TempoInterval | undefined; + reasoning?: string | undefined; + [key: string]: any; +}/** Extended AI execution metadata */ - ai?: { - provider: string; - confidence: number; - conflictBumped?: boolean | undefined; - originalSlot?: TempoInterval | undefined; - reasoning?: string | undefined; - [key: string]: any; - } | undefined; + ai?: TempoScheduleAiMeta | undefined;/** Extended AI execution metadata */ - ai: { - provider: string; - confidence: number; - conflictBumped?: boolean | undefined; - originalSlot?: TempoInterval | undefined; - reasoning?: string | undefined; - [key: string]: any; - }; + ai: TempoScheduleAiMeta;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/types/schedule.type.ts` around lines 74 - 111, Extract the duplicated ai metadata object shape into a named interface in the schedule type definitions, then replace the inline ai declarations in TempoScheduleResult and TempoScheduleMeta with that interface while preserving their existing optionality.packages/plugins/ai/test/cache.test.ts (1)
8-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAwait
initAIin the tests.
initAIreturnsPromise<void>. Every call site here ignores the returned promise. The synchronous part ofinitAIsets_state.config, so the assertions still pass today, but the async tail keeps running after the test body continues and afterafterEachrestores mocks. Awaiting removes the floating promise and the cross-test ordering dependency.♻️ Proposed change (apply the same pattern to each call site)
- beforeEach(() => { + beforeEach(async () => { vi.restoreAllMocks(); Tempo.cache.clear(); - initAI({ + await initAI({ providers: [{ id: 'groq', key: 'mock-test-key' }], remoteConfigUrl: false }); });Also applies to: 37-41, 74-78, 105-108, 130-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/test/cache.test.ts` around lines 8 - 11, Await every initAI call in the tests, including the call sites around the existing test setup and the additional reported locations, so each test completes initialization before assertions or cleanup run. Mark the containing test or setup callbacks async as needed while preserving the current initialization arguments and test behavior.packages/plugins/ai/src/functions/recurrence.ts (1)
46-58: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
ensureCachedre-expands the whole series on every page.Lines 53-54 discard
cachedOccurrencesand rebuild it from the anchor each time the needed count grows. Paged reads throughtake()therefore cost O(n²) expansions across n pages. Each call also re-runsexpandRRuleEpochsfrom the anchor.Grow the cache instead of rebuilding it, for example by expanding in geometric steps and appending only the new tail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/functions/recurrence.ts` around lines 46 - 58, Update ensureCached to preserve cachedOccurrences when neededCount grows instead of clearing and rebuilding from anchorTempo. Expand only the missing tail, using geometric growth as appropriate, append new occurrences, and set fullyExpanded when expansion returns fewer items than requested while preserving the existing after/before filtering behavior.packages/library/README.md (1)
18-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the RRULE module to the key modules table.
This release adds
rrule.libraryto the public barrel. The table omits it, so the new utilities are not discoverable from the README.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/library/README.md` around lines 18 - 28, Update the key modules table in the README to add an RRULE entry for the newly public rrule.library utilities, including a concise description consistent with the existing module rows.packages/library/CHANGELOG.md (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueList all new public exports.
rrule.libraryalso exportsexpandRRuleEpochsandisFiniteRRule. Both are part of the public surface throughcommon.index.ts. Add them to the entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/library/CHANGELOG.md` at line 11, Update the RRULE Support changelog entry to list the complete public export set, adding expandRRuleEpochs and isFiniteRRule alongside the existing rrule.library utilities exposed through common.index.ts.packages/tempo/test/discrete/standalone_parse.test.ts (1)
84-97: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the resolved offset for the named zones.
The named-zone cases assert
timeZoneIdonly.PSTdenotes a fixed-08:00abbreviation, butAmerica/Los_Angeleson 6 August resolves to-07:00because daylight saving is active. The test cannot detect whether the parser preserves the literal abbreviation offset or applies the IANA zone rules.Add
offsetassertions for both named-zone cases so the intended semantics are pinned.💚 Proposed additions
expect(zdtAest.timeZoneId).toBe('Australia/Sydney'); + expect(zdtAest.offset).toBe('+10:00'); const zdtPst = parse('Aug 6, 16:16 PST'); expect(zdtPst.month).toBe(8); expect(zdtPst.day).toBe(6); expect(zdtPst.hour).toBe(16); expect(zdtPst.minute).toBe(16); expect(zdtPst.timeZoneId).toBe('America/Los_Angeles'); + expect(zdtPst.offset).toBe('-07:00'); // DST active on 6 August🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/test/discrete/standalone_parse.test.ts` around lines 84 - 97, Extend the named-zone assertions in the parse test for zdtAest and zdtPst to verify their resolved offset values, covering both the AEST abbreviation and PST’s fixed -08:00 offset rather than relying only on timeZoneId.packages/tempo/test/plugins/extend.recurrence.test.ts (1)
4-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a rule that yields no occurrence.
getNextRRuleEpochfalls back to a fixed one-day shift when the rule is exhausted. That branch is a silent, non-conforming result. Add a test with an expiredUNTILso the fallback behaviour is explicit and intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/test/plugins/extend.recurrence.test.ts` around lines 4 - 16, Extend the extend.recurrence test suite with a Tempo.prototype.nextOccurrence case using an RRULE containing an expired UNTIL, and assert the current fixed one-day fallback result explicitly. Keep the existing string and rrule-object coverage unchanged.packages/library/src/common/rrule.library.ts (1)
192-192: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
options.countis ignored when the rule declaresCOUNT.
maxToFetchprefersrule.countoveroptions.count.getNextRRuleEpochrequests one occurrence, but forFREQ=DAILY;COUNT=500the expansion generates up to 500 candidates before returning the first. Use the minimum of the two bounds.♻️ Proposed change
- const maxToFetch = isDefined(rule.count) ? rule.count : (options?.count ?? 100); + const requested = options?.count ?? 100; + const maxToFetch = isDefined(rule.count) ? Math.min(rule.count, requested) : requested;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/library/src/common/rrule.library.ts` at line 192, Update the maxToFetch calculation in the rule expansion flow to use the smaller of rule.count and options.count when both are defined, while retaining the existing default of 100 when neither bound is provided. This ensures getNextRRuleEpoch and other callers never expand beyond either configured limit.packages/library/test/common/rrule_library.test.ts (1)
40-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the untested RRULE paths.
The suite covers
DAILYexpansion only. The following behaviour is unverified: Sunday handling inWEEKLYandMONTHLY(BYDAY=SU),MONTHLYwithnthselectors,YEARLYwithBYMONTH,UNTILandCOUNTtermination,BYSETPOS, andisFiniteRRule.A Sunday case would expose the
DAY_MAP.SUNdefect flagged inpackages/library/src/common/rrule.library.tsat Line 214. Add at least that case with the fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/library/test/common/rrule_library.test.ts` around lines 40 - 55, Extend the tests around expandRRuleEpochs and getNextRRuleEpoch to cover Sunday BYDAY handling for WEEKLY and MONTHLY, including correcting the DAY_MAP.SUN mapping in the RRULE implementation. Add coverage for MONTHLY nth selectors, YEARLY BYMONTH, UNTIL and COUNT termination, BYSETPOS, and isFiniteRRule, preserving expected occurrence ordering and termination behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9217fb79-1cb3-4494-b4e8-8b00f62cebbd
⛔ Files ignored due to path filters (3)
packages/library/img/library-logo.svgis excluded by!**/*.svgpackages/tempo/img/library-logo.svgis excluded by!**/*.svgpackages/tempo/public/library-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (73)
.github/workflows/deploy-docs.ymlpackage.jsonpackages/library/CHANGELOG.mdpackages/library/README.mdpackages/library/package.jsonpackages/library/src/common.index.tspackages/library/src/common/rrule.library.tspackages/library/test/common/rrule_library.test.tspackages/plugins/.bin/check-branch-diff.shpackages/plugins/.bin/check-versions.shpackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/README.mdpackages/plugins/ai/doc/architecture.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/init.mdpackages/plugins/ai/doc/parse.mdpackages/plugins/ai/doc/rate-limits.mdpackages/plugins/ai/doc/recurrence.mdpackages/plugins/ai/package.jsonpackages/plugins/ai/plan/v0.3.0-roadmap.mdpackages/plugins/ai/src/core/config.tspackages/plugins/ai/src/core/init.tspackages/plugins/ai/src/core/manifest.tspackages/plugins/ai/src/core/support.tspackages/plugins/ai/src/functions/parse.tspackages/plugins/ai/src/functions/recurrence.tspackages/plugins/ai/src/functions/schedule.tspackages/plugins/ai/src/index.tspackages/plugins/ai/src/types/common.type.tspackages/plugins/ai/src/types/index.tspackages/plugins/ai/src/types/parse.type.tspackages/plugins/ai/src/types/recurrence.type.tspackages/plugins/ai/src/types/schedule.type.tspackages/plugins/ai/test/cache.test.tspackages/plugins/ai/test/manifest.test.tspackages/plugins/ai/test/parse.test.tspackages/plugins/ai/test/recurrence.test.tspackages/plugins/ai/test/schedule.test.tspackages/tempo/.vitepress/config.tspackages/tempo/.vitepress/theme/data/catalog.jsonpackages/tempo/CHANGELOG.mdpackages/tempo/bin/expand-typedoc.mjspackages/tempo/bin/generate-llms-txt.mjspackages/tempo/bin/update-version.mjspackages/tempo/doc/1-getting-started/ai-integration.mdpackages/tempo/doc/1-getting-started/installation.mdpackages/tempo/doc/2-core-concepts/tempo.parse.mdpackages/tempo/doc/3-extending-tempo/tempo.layout.mdpackages/tempo/doc/3-extending-tempo/tempo.modularity.mdpackages/tempo/doc/3-extending-tempo/tempo.plugin.mdpackages/tempo/doc/6-utility-library/tempo.library.mdpackages/tempo/package.jsonpackages/tempo/public/bundle.index.htmlpackages/tempo/public/esm_core.index.htmlpackages/tempo/public/esm_full.index.htmlpackages/tempo/public/esm_sh.index.htmlpackages/tempo/public/llms-full.txtpackages/tempo/public/llms.txtpackages/tempo/public/providers.v1.jsonpackages/tempo/public/script.index.htmlpackages/tempo/src/engine/engine.composer.tspackages/tempo/src/engine/engine.lexer.tspackages/tempo/src/interval.class.tspackages/tempo/src/module/module.parse.tspackages/tempo/src/plugin/extend/extend.recurrence.tspackages/tempo/src/support/support.cache.tspackages/tempo/src/support/support.default.tspackages/tempo/src/support/support.enum.tspackages/tempo/src/tempo.class.tspackages/tempo/src/tempo.version.tspackages/tempo/test/discrete/standalone_parse.test.tspackages/tempo/test/plugins/extend.recurrence.test.tspackages/tempo/test/support/cache.test.ts
| uses: peter-evans/repository-dispatch@v3 | ||
| with: | ||
| token: ${{ secrets.TEMPO_WORKSPACE_DISPATCH_TOKEN }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Resolve the current v3 tag before selecting and reviewing the commit to pin.
git ls-remote https://github.com/peter-evans/repository-dispatch.git refs/tags/v3
# Find all current uses for a consistent pinning policy.
rg -n 'peter-evans/repository-dispatch@' .github/workflowsRepository: magmacomputing/magma
Length of output: 298
Pin peter-evans/repository-dispatch to a reviewed commit SHA.
peter-evans/repository-dispatch@v3 is a mutable tag and receives secrets.TEMPO_LOCK; pin this third-party action to a full commit SHA and keep the release tag in a comment for updates.
🤖 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 @.github/workflows/deploy-docs.yml around lines 82 - 84, Update the
repository-dispatch action reference in the workflow to use the reviewed full
commit SHA instead of the mutable v3 tag, and retain the corresponding release
tag in an inline comment for future updates. Leave the existing
TEMPO_WORKSPACE_DISPATCH_TOKEN configuration unchanged.
| if (rule.byDay && rule.byDay.length > 0) { | ||
| periodBases = rule.byDay.map(bd => { | ||
| const targetDay = DAY_MAP[bd.day] ?? 1; | ||
| const currentDow = baseDate.getUTCDay() === 0 ? DAY_MAP.SUN : baseDate.getUTCDay(); | ||
| const diff = (targetDay - currentDow + DAYS_IN_WEEK) % DAYS_IN_WEEK; | ||
| const targetDate = new Date(baseDate.getTime()); | ||
| targetDate.setUTCDate(targetDate.getUTCDate() + diff); | ||
| return targetDate; | ||
| }); | ||
| } else { | ||
| periodBases = [baseDate]; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
DAY_MAP.SUN is undefined and produces NaN occurrences.
DAY_MAP defines the key SU, not SUN. At Line 214 DAY_MAP.SUN evaluates to undefined whenever baseDate falls on a Sunday. diff then becomes NaN, setUTCDate(NaN) creates an Invalid Date, and expandRRuleEpochs pushes NaN into results. getNextRRuleEpoch then returns NaN, and Tempo.prototype.nextOccurrence constructs a Tempo from NaN.
The same key error exists at Line 238 in the MONTHLY branch. There, dow becomes undefined for every Sunday, so BYDAY=SU never matches and monthly Sunday rules yield no candidates.
Use DAY_MAP.SU at both sites.
🐛 Proposed fix
- const currentDow = baseDate.getUTCDay() === 0 ? DAY_MAP.SUN : baseDate.getUTCDay();
+ const currentDow = baseDate.getUTCDay() === 0 ? DAY_MAP.SU : baseDate.getUTCDay();And in the MONTHLY branch at Line 238:
- const dow = d.getUTCDay() === 0 ? DAY_MAP.SUN : d.getUTCDay();
+ const dow = d.getUTCDay() === 0 ? DAY_MAP.SU : d.getUTCDay();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/library/src/common/rrule.library.ts` around lines 211 - 222, Replace
the invalid DAY_MAP.SUN references with DAY_MAP.SU in both the weekly
period-base calculation and the monthly BYDAY matching logic, preserving all
other recurrence behavior.
| case 'MONTHLY': { | ||
| baseDate.setUTCMonth(baseDate.getUTCMonth() + step * rule.interval); | ||
| const year = baseDate.getUTCFullYear(); | ||
| const month = baseDate.getUTCMonth() + 1; | ||
| const daysInMonth = getDaysInMonth(year, month); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
setUTCMonth overflows for anchors after day 28.
setUTCMonth keeps the day-of-month. For an anchor on 31 January, step = 1 produces 3 March, not 28 February. The RFC 5545 rule is to skip months that do not contain the anchor day. This affects every MONTHLY rule without BYDAY.
Clamp or skip explicitly. One approach is to build the date from Date.UTC(year, monthIndex, day, ...) and discard the period when the resulting month index differs from the intended one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/library/src/common/rrule.library.ts` around lines 225 - 229, Update
the MONTHLY branch in the recurrence calculation to avoid setUTCMonth overflow
for anchors after day 28. Construct each candidate using the intended year/month
and original day, then skip the period when the resulting month differs from the
intended month, preserving valid anchor dates and existing BYDAY handling.
| case 'YEARLY': { | ||
| baseDate.setUTCFullYear(baseDate.getUTCFullYear() + step * rule.interval); | ||
| periodBases = [baseDate]; | ||
| break; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
YEARLY does not expand BYMONTH or BYDAY.
The YEARLY branch produces one base per step. Line 271 then filters that base by BYMONTH. For an anchor in August and FREQ=YEARLY;BYMONTH=1, every base is filtered out. The loop runs the full 1000 steps and returns an empty array. getNextRRuleEpoch then falls back to a one-day shift, which is not an occurrence of the rule.
Expand BYMONTH into one base per listed month within the target year, instead of using it only as a filter for YEARLY. The test at packages/library/test/common/rrule_library.test.ts Line 36 parses such a rule but does not expand it, so this gap is not covered.
Also applies to: 271-272
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/library/src/common/rrule.library.ts` around lines 258 - 262, The
YEARLY branch in the rule expansion logic must create a base date for each month
listed by BYMONTH in the target year, rather than creating only one base and
filtering it later. Update the YEARLY handling around periodBases and the
BYMONTH/BYDAY filtering flow so rules such as FREQ=YEARLY;BYMONTH=1 produce
January candidates and preserve existing behavior when BYMONTH is absent.
| is_gt=$(node --input-type=module -e ' | ||
| function parseSemver(v) { | ||
| const m = String(v).trim().match(/^v?(\d+)\.(\d+)\.(\d+)(?:-(.+))?$/); | ||
| if (!m) return null; | ||
| return { major: parseInt(m[1], 10), minor: parseInt(m[2], 10), patch: parseInt(m[3], 10), prerelease: m[4] || "" }; | ||
| } | ||
| function compare(a, b) { | ||
| const pa = parseSemver(a), pb = parseSemver(b); | ||
| if (!pa || !pb) return false; | ||
| if (pa.major !== pb.major) return pa.major > pb.major; | ||
| if (pa.minor !== pb.minor) return pa.minor > pb.minor; | ||
| if (pa.patch !== pb.patch) return pa.patch > pb.patch; | ||
| if (!pa.prerelease && pb.prerelease) return true; | ||
| if (pa.prerelease && !pb.prerelease) return false; | ||
| return pa.prerelease > pb.prerelease; | ||
| } | ||
| console.log(compare(process.argv[1], process.argv[2]) ? "true" : "false"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a SemVer-compliant prerelease comparator.
Line 47 compares the full prerelease string lexicographically. The check accepts 1.0.0-alpha.2 as newer than 1.0.0-alpha.10, although it is older under Semantic Version precedence. This can approve a prerelease downgrade.
Replace parseSemver and compare with a SemVer-compliant comparator. It must compare each prerelease identifier by SemVer rules.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/plugins/.bin/check-branch-diff.sh` around lines 33 - 49, Replace the
inline parseSemver and compare functions in the Node version check with
SemVer-compliant prerelease comparison: split prerelease values into
dot-separated identifiers, compare numeric identifiers numerically, compare
nonnumeric identifiers lexically, rank numeric identifiers below nonnumeric
ones, and treat a shorter equal-prefix sequence as lower precedence. Preserve
existing core-version and stable-versus-prerelease ordering and return the same
boolean output.
| "groq": { | ||
| "url": "https://api.groq.com/openai/v1/chat/completions", | ||
| "model": "llama-3.3-70b-versatile", | ||
| "tokenParam": "max_tokens" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching providers.v1.json:\n'
fd -a 'providers\.v1\.json$' . || true
printf '\nFile content:\n'
if [ -f packages/tempo/public/providers.v1.json ]; then
cat -n packages/tempo/public/providers.v1.json
else
echo "packages/tempoPublic/providers.v1.json not found"
fi
printf '\nReferences to providers.v1 and provider model selection:\n'
rg -n "providers\.v1|providers\.v1\.json|llama-3\.3-70b-versatile|tokenParam|max_tokens|model" packages/tempo -S || trueRepository: magmacomputing/magma
Length of output: 6648
🌐 Web query:
Groq deprecations llama-3.3-70b-versatile August 16 2026
💡 Result:
The Groq model llama-3.3-70b-versatile is scheduled for deprecation and will shut down on August 16, 2026 [1][2][3]. This deprecation primarily impacts usage on Groq's free and developer tiers; committed-spend enterprise contracts are generally exempt from these specific model retirements [4][5]. Groq officially recommends that developers migrate their applications to alternative models, specifically suggesting openai/gpt-oss-120b or qwen/qwen3.6-27b as replacements [1][3][5]. Users are advised to update their API integrations before the shutdown deadline to avoid service interruptions [6][5].
Citations:
- 1: langchain-ai/langchain@946fbbb
- 2: https://modelpricewatch.com/models/groq-llama-33-70b-versatile/
- 3: https://markaicode.com/benchmarks/groq-production-benchmark-latency/
- 4: https://vorplabs.com/models/deprecations
- 5: https://www.linkedin.com/posts/cbvk28_groq-ai-llms-activity-7476700000266182656-r4K7
- 6: https://vuink.com/post/console-d-dgroq-d-dcom
Replace the retiring Groq default model.
llama-3.3-70b-versatile shuts down on August 16, 2026 for free and developer usage. Groq recommends switching to openai/gpt-oss-120b or qwen/qwen3.6-27b; update this public manifest and test the replacement request format before release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tempo/public/providers.v1.json` around lines 5 - 8, Update the groq
provider entry in providers.v1.json to replace the retiring
llama-3.3-70b-versatile default with Groq’s recommended openai/gpt-oss-120b or
qwen/qwen3.6-27b model, and verify the replacement request format remains
compatible before release.
| if (zone && /^([+-]\d{1,2})$/.test(zone)) { | ||
| const sign = zone[0]; | ||
| const num = Math.abs(parseInt(zone, 10)); | ||
| zone = `${sign}${num.toString().padStart(2, '0')}:00`; | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg 'packages/tempo/src/(engine/engine\.lexer\.ts|support/support\.default\.ts|.*timezone|.*match).*' || true
echo "== lexer section =="
sed -n '260,330p' packages/tempo/src/engine/engine.lexer.ts
echo "== support offset refs =="
rg -n "parseZone|Match\.offset|offset|timeZone|zone" packages/tempo/src -S
echo "== package deps temporal =="
for f in package.json packages/tempo/package.json pnpm-lock.yaml package-lock.json yarn.lock; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n "`@js-temporal/polyfill`|Temporal|moment|dayjs|luxon" "$f" -S || true
fi
done
echo "== behavioral parser probe =="
python3 - <<'PY'
import re
zones = ["+5", "5", "+05", "+5:30", "+530", "-4", "UTC", "+UTC", "GMT", "+10:30", "+0:30"]
for z in zones:
old = z
if z and re.match(r'^([+-]\d{1,2})$', z):
sign = z[0]
num = abs(int(z,10))
old = z
z = f'{sign}{str(num).zfill(2)}:00'
print(f"{z!r:12} -> {old if z==z else z!r}")
PYRepository: magmacomputing/magma
Length of output: 30745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg 'packages/tempo/src/(engine/engine\.lexer\.ts|support/support\.default\.ts|.*timezone|.*match).*' || true
echo "== lexer section =="
sed -n '260,330p' packages/tempo/src/engine/engine.lexer.ts
echo "== support offset refs =="
rg -n "parseZone|Match\.offset|offset|timeZone|zone" packages/tempo/src -S
echo "== package deps temporal =="
for f in package.json packages/tempo/package.json pnpm-lock.yaml package-lock.json yarn.lock; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n "`@js-temporal/polyfill`|Temporal|moment|dayjs|luxon" "$f" -S || true
fi
done
echo "== behavioral parser probe =="
python3 - <<'PY'
import re
zones = ["+5", "5", "+05", "+5:30", "+530", "-4", "UTC", "+UTC", "GMT", "+10:30", "+0:30"]
for z in zones:
old = z
if z and re.match(r'^([+-]\d{1,2})$', z):
sign = z[0]
num = abs(int(z,10))
z = f'{sign}{str(num).zfill(2)}:00'
matched_short = bool(re.match(r'^([+-]?)\d{1,2}(?::?\d{2})?$', old))
print(f"input={z!r:8} previously_normalized_to_short_offset={matched_short}")
PYRepository: magmacomputing/magma
Length of output: 31139
🌐 Web query:
Temporal ZonedDateTime toPlainDateTime toZonedDateTime TimeZone offset valid ID examples @js-temporal/polyfill``
💡 Result:
In the Temporal API (supported by @js-temporal/polyfill), Temporal.ZonedDateTime is a timezone-aware, calendar-aware date/time object [1][2]. Conversion Methods toPlainDateTime: Converts a ZonedDateTime to a PlainDateTime by removing the time zone and offset information, leaving only the wall-clock date and time [3][4]. toZonedDateTime(timeZone): This is a method on PlainDateTime (and other types) that creates a ZonedDateTime from a local wall-clock time by applying the specified time zone [5]. Time Zone and Offset Identifiers Time zones in Temporal are typically IANA time zone identifiers (e.g., 'America/New_York', 'Asia/Kolkata') [6]. Offset-based time zones are also supported when an IANA ID is not strictly required or when dealing with fixed-offset scenarios. Valid formats include '+HH:mm', '-HH:mm', '+HHmm', or '-HH' (e.g., '+05:30', '-08:00', '+0600') [3][6]. The 'Z' offset (UTC) is supported but behaves differently than '+00:00'; 'Z' indicates the UTC time is known but the offset is unknown, whereas '+00:00' is treated as a specific local offset [3]. You may optionally add a critical flag to an identifier, such as '[!America/New_York]', to signal that the system must support this specific zone or throw an error [3]. Disambiguation When converting from a wall-clock time (PlainDateTime) to a ZonedDateTime, the time might be ambiguous or non-existent due to daylight saving time transitions. Temporal allows you to configure this using the disambiguation option (e.g., 'compatible', 'earlier', 'later', 'reject') [3][5]. Example Usage // Converting ZonedDateTime to PlainDateTime const zdt = Temporal.ZonedDateTime.from('2026-08-08T12:00:00+00:00[UTC]'); const pdt = zdt.toPlainDateTime; // '2026-08-08T12:00:00' // Converting PlainDateTime to ZonedDateTime const dt = Temporal.PlainDateTime.from('2026-08-08T12:00:00'); const zdtFromDt = dt.toZonedDateTime('America/Los_Angeles'); // '2026-08-08T12:00:00-07:00[America/Los_Angeles]'
Citations:
- 1: https://tc39.es/proposal-temporal/docs/
- 2: https://github.com/tc39/proposal-temporal/blob/main/docs/README.md
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/ZonedDateTime
- 4: https://github.com/js-temporal/temporal-polyfill/blob/main/lib/zoneddatetime.ts
- 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/PlainDateTime/toZonedDateTime
- 6: https://github.com/tc39/proposal-temporal/blob/main/docs/zoneddatetime.md
Normalize short timezone offsets with minutes.
Match.offset accepts forms like +5:30, but parseZone only expands hour-only offsets. Unnormalized offsets such as +5:30 can be passed to Temporal and rejected, so the parser keeps the prior timezone instead of applying the requested offset. Normalize hour and minute offsets before toZonedDateTime.
🤖 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/engine/engine.lexer.ts` around lines 304 - 309, Update the
timezone normalization logic in parseZone to also normalize signed
hour-and-minute offsets such as +5:30, padding the hour and preserving the
minutes in the canonical ±HH:MM form before toZonedDateTime. Keep existing
hour-only offset normalization unchanged and ensure normalized offsets are
applied instead of retaining the prior timezone.
| const resolvedZone = enums.TIMEZONE[zone.toLowerCase()] ?? zone; | ||
| if (config) config.timeZone = resolvedZone; | ||
| try { | ||
| dateTime = zdt.toPlainDateTime().toZonedDateTime(resolvedZone); | ||
| } catch { | ||
| logWarn(`Unrecognized or invalid timezone identifier: '${zone}'`, config); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the referenced lexer implementation and surrounding timezone handling.
git ls-files | rg 'packages/tempo/src/engine/engine\.lexer\.ts$' || true
wc -l packages/tempo/src/engine/engine.lexer.ts
sed -n '280,340p' packages/tempo/src/engine/engine.lexer.ts
# Search for config.timeZone consumers/usages to assess later composition behavior.
rg -n "config\.timeZone|timeZone" packages/tempo/src packages -g '*.ts' | head -n 120Repository: magmacomputing/magma
Length of output: 15996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: use Luxon semantics if available to verify that invalid timezone
# conversion throws and leaves the pre-call value unchanged.
node - <<'JS'
try {
const { DateTime, ZonedDateTime, Settings } = require('luxon');
console.log('luxon version', require('luxon/package.json').version);
const now = DateTime.local();
const zdt = now.setZone('local');
let caughtTime;
try {
const result = zdt.toPlainDateTime().toZonedDateTime('Invalid/Zone');
caughtTime = result;
} catch (e) {
caughtTime = e && e.code !== undefined ? `${e.code}: ${e.message}` : String(e);
}
console.log(JSON.stringify({
lexicalZone: 'Invalid/Zone',
thrownByToZonedDateTime: caughtTime,
zdtAfterTryCatch: zdt.zone.zoneName
}), 'success');
} catch (e) {
console.log('luxon dependency unavailable in sandbox:', e && (e.code || e.message));
}
JSRepository: magmacomputing/magma
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the composition path that consumes config.timeZone and the Temporal call sites.
sed -n '1,120p' packages/tempo/src/engine/engine.composer.ts
sed -n '1128,1148p' packages/tempo/src/tempo.class.ts
sed -n '1660,1760p' packages/tempo/src/tempo.class.ts
sed -n '888,906p' packages/tempo/src/tempo.class.ts
sed -n '32,50p' packages/tempo/src/engine/engine.term.tsRepository: magmacomputing/magma
Length of output: 11542
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Static verifier: check parseZone assigns config.timeZone before the toZonedDateTime call,
# and that the catch block only calls logWarn without reverting config.timeZone.
python3 - <<'PY'
from pathlib import Path
p = Path('packages/tempo/src/engine/engine.lexer.ts')
text = p.read_text().splitlines()
try:
i = next(idx for idx, line in enumerate(text) if 'const resolvedZone = enums.TIMEZONE' in line)
except StopIteration:
raise SystemExit('parseZone assignment not found')
try:
catch_idx = next(idx for idx in range(i+1, min(i+15, len(text))) if '{' in text[idx] and idx > i)
except StopIteration:
raise SystemExit('try block not found')
print('resolvedZone assignment line:', i+1)
print(text[i].strip())
print('try/zdt line:', catch_idx+1)
print(text[catch_idx].strip())
print('catch assignment lines:', [(n+1, text[n].strip()) for n in range(catch_idx+1, catch_idx+5) if 'config.timeZone' in text[n]])
PYRepository: magmacomputing/magma
Length of output: 308
Update config.timeZone only after toZonedDateTime succeeds.
parseZone stores resolvedZone before calling toZonedDateTime, so an invalid timezone leaves config.timeZone set to the bad identifier after the catch. Later Temporal calls that reuse this.#local.config.timeZone can then throw. Move config.timeZone = resolvedZone into the successful conversion path.
Proposed fix
- if (config) config.timeZone = resolvedZone;
try {
- dateTime = zdt.toPlainDateTime().toZonedDateTime(resolvedZone);
+ dateTime = zdt.toPlainDateTime().toZonedDateTime(resolvedZone);
+ if (config) config.timeZone = resolvedZone;
} catch {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const resolvedZone = enums.TIMEZONE[zone.toLowerCase()] ?? zone; | |
| if (config) config.timeZone = resolvedZone; | |
| try { | |
| dateTime = zdt.toPlainDateTime().toZonedDateTime(resolvedZone); | |
| } catch { | |
| logWarn(`Unrecognized or invalid timezone identifier: '${zone}'`, config); | |
| } | |
| const resolvedZone = enums.TIMEZONE[zone.toLowerCase()] ?? zone; | |
| try { | |
| dateTime = zdt.toPlainDateTime().toZonedDateTime(resolvedZone); | |
| if (config) config.timeZone = resolvedZone; | |
| } catch { | |
| logWarn(`Unrecognized or invalid timezone identifier: '${zone}'`, config); | |
| } |
🤖 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/engine/engine.lexer.ts` around lines 318 - 324, In
parseZone, move the config.timeZone assignment from before the conversion into
the successful toZonedDateTime path, after it completes without throwing. Keep
invalid timezone handling in the catch without mutating config.timeZone.
| const rruleStr = isString(rrule) ? rrule : rrule.rrule; | ||
| const nextMs = getNextRRuleEpoch(rruleStr, this.epoch.ms); | ||
| return new Tempo(nextMs, this.config); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 10 '\b(getNextRRuleEpoch|expandRRuleEpochs)\b' packages/library packages/tempo
rg -n -C 8 '\btimeStamp\b|new Tempo\(' packages/tempo/srcRepository: magmacomputing/magma
Length of output: 43835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- tempo.class constructor signature ---\n'
ast-grep outline packages/tempo/src/tempo.class.ts --match Tempo --view expanded || true
printf '\n--- relevant constructor area ---\n'
sed -n '89,180p' packages/tempo/src/tempo.class.ts | cat -n
printf '\n--- epoch getters / resolve area ---\n'
sed -n '1520,1660p' packages/tempo/src/tempo.class.ts | cat -n
printf '\n--- parse numeric value handling ---\n'
sed -n '300,385p' packages/tempo/src/module/module.parse.ts | cat -n
printf '\n--- compose argument area ---\n'
sed -n '90,140p' packages/tempo/src/engine/engine.composer.ts | cat -n
printf '\n--- timestamp parser area ---\n'
sed -n '220,320p' packages/tempo/src/engine/engine.parser.ts | cat -n
printf '\n--- search explicit numeric timestamp parser ---\n'
rg -n -C 5 "epochMilliseconds|epochSeconds|fromEpoch|toZonedDateTime|Temporal|value type|Number|timestamp" packages/tempo/src/engine/engine.parser.ts packages/tempo/src/engine/engine.composer.tsRepository: magmacomputing/magma
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tempo.class constructor signature ---'
ast-grep outline packages/tempo/src/tempo.class.ts --match Tempo --view expanded || true
echo
echo '--- relevant constructor area ---'
sed -n '89,180p' packages/tempo/src/tempo.class.ts | cat -n
echo
echo '--- epoch getters / resolve area ---'
sed -n '1520,1660p' packages/tempo/src/tempo.class.ts | cat -n
echo
echo '--- parse numeric value handling ---'
sed -n '300,385p' packages/tempo/src/module/module.parse.ts | cat -n
echo
echo '--- compose argument area ---'
sed -n '90,140p' packages/tempo/src/engine/engine.composer.ts | cat -n
echo
echo '--- parser timestamp handling ---'
sed -n '220,320p' packages/tempo/src/engine/engine.parser.ts | cat -n
echo
echo '--- search parser numeric timestamps ---'
rg -n -C 5 "epochMilliseconds|epochSeconds|fromEpoch|toZonedDateTime|Temporal|value type|Number|timestamp" packages/tempo/src/engine/engine.parser.ts packages/tempo/src/engine/engine.composer.tsRepository: magmacomputing/magma
Length of output: 32430
🏁 Script executed:
#!/bin/bash
set -eu o pipefail
echo '--- constructor implementation ---'
sed -n '1309,1735p' packages/tempo/src/tempo.class.ts | cat -n
echo
echo '--- compose implementation ---'
sed -n '70,180p' packages/tempo/src/engine/engine.composer.ts | cat -n
echo
echo '--- unit lookup ---'
rg -n -C 4 "UNIT_LOOKUP|scale|timeStamp" packages/tempo/src/engine packages/tempo/src/module packages/tempo/src/tempo.class.ts
echo
echo '--- behavioral probe for numeric epoch vs timeStamp ---'
node - <<'JS'
const anchor = BigInt(Date.UTC(2026, 7, 7, 0, 0, 0, 0));
const nextMs = anchor + 86_400_000n;
console.log('anchor ms:', Number(anchor));
console.log('next ms:', Number(nextMs));
console.log('tempo 1000 in ms:', Number(Temporal.Instant.fromEpochMilliseconds(anchor + 1000n).epochMilliseconds));
console.log('tempo 1000 in ss:', Temporal.Instant.fromEpochNanoseconds(anchor / 1000n * 1000n).epochMilliseconds);
JSRepository: magmacomputing/magma
Length of output: 37390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node --experimental-temporal - <<'JS'
const anchor = BigInt(Date.UTC(2026, 7, 7, 0, 0, 0, 0));
const nextMs = anchor + 86_400_000n;
console.log('anchor ms:', Number(anchor));
console.log('next ms:', Number(nextMs));
const { scale } = { ss: 1_000_000_000n, ms: 1_000_000n, us: 1_000n, ns: 1n }?.ms;
console.log('tempo(1000) in ms:', Number(Temporal.Instant.fromEpochNanoseconds(1000n * scale).epochMilliseconds));
console.log('tempo(next) in ms:', Number(Temporal.Instant.fromEpochNanoseconds(nextMs * scale).epochMilliseconds));
console.log('tempo(next) in ss:', Number(Temporal.Instant.fromEpochNanoseconds(nextMs * 1_000_000_000n).epochMilliseconds));
JSRepository: magmacomputing/magma
Length of output: 200
Preserve the receiver class and millisecond epoch.
getNextRRuleEpoch returns a millisecond epoch, but numeric Tempo construction applies this.config.timeStamp; with timeStamp: 'ss', the returned instant will be shifted by the seconds-to-nanoseconds scale. Use this.constructor and an explicit millisecond instant so the recurrence result is not reinterpreted and decorated subclasses are preserved.
🤖 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/extend/extend.recurrence.ts` around lines 17 - 19,
Update the recurrence result construction after getNextRRuleEpoch in the extend
method to instantiate via this.constructor, preserving decorated subclasses, and
explicitly mark the nextMs value as a millisecond instant so
this.config.timeStamp cannot reinterpret it. Keep the existing recurrence
calculation and configuration propagation unchanged.
| test('Tempo.prototype.nextOccurrence returns next date matching RRULE string', () => { | ||
| const start = new Tempo('2026-08-07T00:00:00.000Z'); | ||
| const next = start.nextOccurrence('FREQ=DAILY;INTERVAL=1'); | ||
| expect(next.format('{yyyy}-{mm}-{dd}')).toBe('2026-08-08'); | ||
| }); | ||
|
|
||
| test('Tempo.prototype.nextOccurrence accepts object with rrule property', () => { | ||
| const start = new Tempo('2026-08-07T00:00:00.000Z'); | ||
| const next = start.nextOccurrence({ rrule: 'FREQ=DAILY;INTERVAL=2' }); | ||
| expect(next.format('{yyyy}-{mm}-{dd}')).toBe('2026-08-09'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
These assertions depend on the runner timezone.
getNextRRuleEpoch computes in UTC, but format('{yyyy}-{mm}-{dd}') renders in the timezone of the Tempo instance. The anchor is midnight UTC. In any negative-offset timezone the formatted date is the previous calendar day, so '2026-08-08' becomes '2026-08-07' and the test fails.
Pin the timezone for these instances, or assert on next.epoch.ms against an explicit Date.UTC value.
💚 Proposed fix using an explicit UTC assertion
test('Tempo.prototype.nextOccurrence returns next date matching RRULE string', () => {
const start = new Tempo('2026-08-07T00:00:00.000Z');
const next = start.nextOccurrence('FREQ=DAILY;INTERVAL=1');
- expect(next.format('{yyyy}-{mm}-{dd}')).toBe('2026-08-08');
+ expect(next.epoch.ms).toBe(Date.UTC(2026, 7, 8, 0, 0, 0, 0));
});
test('Tempo.prototype.nextOccurrence accepts object with rrule property', () => {
const start = new Tempo('2026-08-07T00:00:00.000Z');
const next = start.nextOccurrence({ rrule: 'FREQ=DAILY;INTERVAL=2' });
- expect(next.format('{yyyy}-{mm}-{dd}')).toBe('2026-08-09');
+ expect(next.epoch.ms).toBe(Date.UTC(2026, 7, 9, 0, 0, 0, 0));
});📝 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.
| test('Tempo.prototype.nextOccurrence returns next date matching RRULE string', () => { | |
| const start = new Tempo('2026-08-07T00:00:00.000Z'); | |
| const next = start.nextOccurrence('FREQ=DAILY;INTERVAL=1'); | |
| expect(next.format('{yyyy}-{mm}-{dd}')).toBe('2026-08-08'); | |
| }); | |
| test('Tempo.prototype.nextOccurrence accepts object with rrule property', () => { | |
| const start = new Tempo('2026-08-07T00:00:00.000Z'); | |
| const next = start.nextOccurrence({ rrule: 'FREQ=DAILY;INTERVAL=2' }); | |
| expect(next.format('{yyyy}-{mm}-{dd}')).toBe('2026-08-09'); | |
| }); | |
| test('Tempo.prototype.nextOccurrence returns next date matching RRULE string', () => { | |
| const start = new Tempo('2026-08-07T00:00:00.000Z'); | |
| const next = start.nextOccurrence('FREQ=DAILY;INTERVAL=1'); | |
| expect(next.epoch.ms).toBe(Date.UTC(2026, 7, 8, 0, 0, 0, 0)); | |
| }); | |
| test('Tempo.prototype.nextOccurrence accepts object with rrule property', () => { | |
| const start = new Tempo('2026-08-07T00:00:00.000Z'); | |
| const next = start.nextOccurrence({ rrule: 'FREQ=DAILY;INTERVAL=2' }); | |
| expect(next.epoch.ms).toBe(Date.UTC(2026, 7, 9, 0, 0, 0, 0)); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tempo/test/plugins/extend.recurrence.test.ts` around lines 5 - 15,
Update the nextOccurrence tests to avoid runner-timezone dependence by asserting
next.epoch.ms against explicit Date.UTC values, or by constructing the Tempo
instances with a pinned UTC timezone. Apply this to both RRULE string and
rrule-object cases while preserving their expected daily intervals.
1 participant
Summary by CodeRabbit