Feature/ai api - #68
Conversation
📝 WalkthroughWalkthroughThis release updates Tempo to ChangesAI plugin enhancements
Tempo and library runtime
Tempo docs, tooling, and release metadata
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🔵 Low · up to The formatter currently accepts unsupported single arguments without compile-time errors, which can allow invalid values into the API. This is a bounded, non-blocking type-safety risk that should receive owner follow-up before or after merge. Sequence Diagram(s)sequenceDiagram
participant initAI
participant executeWithMode
participant Provider
participant TempoAiError
initAI->>Provider: load remote defaults and manifests
executeWithMode->>Provider: run fallback, race, consensus, hedged, round-robin, or adaptive mode
Provider-->>executeWithMode: return candidate and rate-limit data
executeWithMode->>TempoAiError: normalize invalid or failing responses
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
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (21)
packages/plugins/.bin/check-branch-diff.sh-33-63 (1)
33-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAccept SemVer build metadata.
parseSemverrejects1.2.4+build.1. Build metadata is valid SemVer and does not change precedence. A plugin changed from1.2.3to that version reportsMODIFIED WITHOUT VERSION BUMP!.Parse and ignore an optional
+metadata suffix.Proposed fix
- const m = String(v).trim().match(/^v?(\d+)\.(\d+)\.(\d+)(?:-(.+))?$/); + const m = String(v).trim().match(/^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/);🤖 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 - 63, Update parseSemver in the inline compare script to accept an optional +build metadata suffix after the version or prerelease portion, while excluding that metadata from the parsed prerelease value and all precedence comparisons. Preserve existing version comparison behavior so metadata-only changes do not affect SemVer ordering.packages/plugins/ai/src/functions/schedule.ts-302-342 (1)
302-342: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReport an unresolved conflict instead of returning an overlapping slot.
If the loop reaches
MAX_ADJUSTMENT_ITERATIONSwhile a conflict remains, Line 342 exits and the code returns the still-conflicting slot. Line 345 then reports[Adjusted for conflict], andai.conflictBumpedistrue. The caller cannot detect that the slot still overlaps a booked event.Track the final conflict state and act on it. Throw a
TempoAiError, or expose the unresolved state in the returned metadata.Note also that the working-hours corrections at Lines 321-327 are not re-validated in the same iteration. The loop continues only when a busy conflict remains.
🤖 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 302 - 342, Track whether the final candidate remains conflicted after the adjustment loop around MAX_ADJUSTMENT_ITERATIONS and do not return it as a valid slot; instead throw a TempoAiError or expose an explicit unresolved-conflict state in the returned metadata so callers can detect it. Also re-run the working-hours/active-day validation after each correction before accepting the interval, rather than continuing only when busyEvents still overlap. Update the surrounding conflict-adjustment flow and its result reporting, including ai.conflictBumped, to reflect the final validated state.packages/plugins/ai/test/schedule.test.ts-214-224 (1)
214-224: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winThe assertion depends on the ambient time zone.
The call passes no
timeZoneand noanchor, soscheduleAIresolvestimeZonefromanchorTempo.tz, which is the runner's ambient zone. The mock returns2026-08-11T09:00:00Z, and Line 223 asserts the local rendering09:00. The test passes only when the runner runs in UTC.The consensus test at Line 263 has the same dependency.
Pass
timeZone: 'UTC'in both calls.💚 Proposed change
const resRace = await scheduleAI('Schedule 1 hour slot', { mode: 'race', + timeZone: 'UTC', providers: [🤖 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 214 - 224, Update both the race and consensus scheduleAI calls in this test to pass timeZone: 'UTC' explicitly, ensuring their mocked timestamps render consistently regardless of the runner’s ambient time zone.packages/plugins/ai/test/recurrence.test.ts-35-44 (1)
35-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that the two batches contain four distinct, consecutive occurrences.
Line 43 compares only
batch2[0]withbatch1[0].ensureCachedre-expands the rule withafter: lastOccurrencefor the second page. If theafterMsfilter inexpandRRuleEpochsis inclusive, thenbatch2[0]repeatsbatch1[1], and this assertion still passes.Assert the four dates as one sequence of consecutive Fridays. I raised the underlying paging concern on
packages/plugins/ai/src/functions/recurrence.tsLines 51-64.💚 Proposed change
const batch2 = result.take(2); expect(batch2).toHaveLength(2); - expect(batch2[0].format('{yyyy}-{mm}-{dd}')).not.toBe(batch1[0].format('{yyyy}-{mm}-{dd}')); + const dates = [...batch1, ...batch2].map(item => item.format('{yyyy}-{mm}-{dd}')); + expect(new Set(dates).size).toBe(4);🤖 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 35 - 44, Strengthen the stateful paging test in “should support stateful paged batching via .take(n)” by combining batch1 and batch2 and asserting all four occurrences are distinct, ordered consecutive Fridays. Replace the single batch2[0] comparison with explicit formatted-date expectations covering both pages, so an inclusive afterMs filter cannot pass by repeating batch1[1].packages/plugins/ai/src/functions/recurrence.ts-196-208 (1)
196-208: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRead the confidence from
winningCandidate, not fromparsedData.
executeConsensusModecan elevate the selected candidate’s confidence to1.0, but this code reads an additional rawparsedData.confidenceand uses that value for bothcreateRecurrenceResultand theminConfidencecheck. Use the normalizedwinningCandidate.confidenceas done inparse.ts.🤖 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 196 - 208, Update the confidence assignment in executeConsensusMode to use the normalized winningCandidate.confidence value rather than parsedData.confidence. Ensure this value is used consistently for the effectiveMinConfidence check and the downstream createRecurrenceResult call, preserving the candidate’s elevated confidence.packages/plugins/ai/test/recurrence.test.ts-245-261 (1)
245-261: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a supported hemisphere value for
sphere.
recurrenceAIacceptssphere?: string, but Tempo hemisphere config is documented assphere: 'north' | 'south', and hemisphere-aware terms comparesphere === 'south'or useTempo.COMPASS.South.sphere: 'southern'propagates as an unrecognized value, so this test only proves storage of an unsupported option. Usesphere: 'south'or add/reject normalization at the type boundary.🤖 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 245 - 261, Update the recurrenceAI test case to pass the supported hemisphere value sphere: 'south' instead of 'southern', and update the corresponding expected items[0].config.sphere assertion to 'south'.packages/plugins/ai/doc/index.md-48-48 (1)
48-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse one canonical recurrence result API in all documentation.
packages/plugins/ai/doc/index.md#L48-L48: remove.nextand document the supportedtake()and iterator APIs.packages/plugins/ai/plan/v0.3.0-roadmap.md#L12-L13: replacerule.take(count)with the actual returned recurrence result name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/doc/index.md` at line 48, Use one canonical recurrence result API across both documentation sites: in packages/plugins/ai/doc/index.md lines 48-48, remove the unsupported .next reference and describe the supported take() and iterator APIs; in packages/plugins/ai/plan/v0.3.0-roadmap.md lines 12-13, replace rule.take(count) with the actual returned recurrence result name.packages/plugins/ai/doc/index.md-47-47 (1)
47-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the documented
parseAIbatch contract consistent.
packages/plugins/ai/doc/index.md#L47-L47: document the declared array union, includingTempoAiErrorwhensoftErrorsis enabled, instead of documenting onlyTempo[].packages/plugins/ai/doc/parse.md#L64-L70: narrow the array result before destructuring and handleTempoAiError, or add an overload that makes the example type-safe.🤖 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/index.md` at line 47, Update packages/plugins/ai/doc/index.md:47-47 to document the full parseAI batch result union, including TempoAiError when softErrors is enabled, rather than only Tempo[]. Update packages/plugins/ai/doc/parse.md:64-70 to narrow the array result before destructuring and handle TempoAiError, or provide an overload that makes the example type-safe.packages/plugins/ai/README.md-45-50 (1)
45-50: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the scheduling example deterministic.
"next Wednesday"uses the currentTempo()anchor whenanchoris omitted. On August 10, 2026, it resolves to August 12, 2026, but it can resolve to a different date later. SetanchorandtimeZone, or use an absolute date and explicit offsets in the example.Proposed fix
-const booking = await scheduleAI("45 min sync next Wednesday afternoon", { - events: [{ start: "2026-08-12 14:00", end: "2026-08-12 15:00", title: "Team standup" }] +const booking = await scheduleAI("45 min sync on 2026-08-12 at 14:00", { + anchor: "2026-08-10T00:00:00Z", + timeZone: "UTC", + events: [{ start: "2026-08-12T14:00:00Z", end: "2026-08-12T15:00:00Z", title: "Team standup" }] });🤖 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 45 - 50, Update the scheduleAI example to avoid the relative “next Wednesday” anchor: either provide explicit anchor and timeZone options or use an absolute date with explicit offsets, while keeping the documented booking output deterministic.packages/plugins/ai/doc/recurrence.md-62-76 (1)
62-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the open-ended iterator guidance.
TempoRecurrenceResult[Symbol.iterator]yields at mostdefaultBatchSizeitems whenisFinite === false; the default is 5. The example therefore stops after five items even withoutbreak. Document the cap and direct callers to repeated.take(n)calls for additional pages.Proposed wording
-When iterating over open-ended schedules (`isFinite === false`), build a `break` termination clause into the loop: +For open-ended schedules, `for...of` yields at most the configured `count` (5 by default). Use repeated `.take(n)` calls for additional pages; a `break` is not required by the current iterator:🤖 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 62 - 76, Update the “Lazy Iteration” guidance for TempoRecurrenceResult[Symbol.iterator] to state that open-ended schedules yield at most defaultBatchSize items per iteration, with a default of 5, so the break condition is not required for that cap. Direct callers to use repeated take(n) calls to retrieve additional pages.packages/plugins/ai/doc/init.md-73-93 (1)
73-93: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winComplete the
AiConfigreference.The source type also exposes
minConfidenceandfetchDefaults, but this reference omits both. Add them so the configuration section does not suggest that these supported options are unavailable.Proposed additions
export interface AiConfig { providers?: AiProvider[]; + /** Strict minimum confidence threshold (0.0 to 1.0) */ + minConfidence?: number; mode?: 'fallback' | 'race' | 'consensus'; timeout?: number; debug?: boolean; cache?: Map<string, string>; cacheAdapter?: AiCacheAdapter; ttl?: number; remoteConfigUrl?: string | false; + /** Optional custom resolver hook for provider defaults */ + fetchDefaults?: (providerId: string) => Promise<Partial<AiProvider> | null>; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/doc/init.md` around lines 73 - 93, Update the AiConfig reference to include the supported minConfidence and fetchDefaults options alongside the existing configuration properties, using their source-defined types and descriptions so the documentation accurately reflects the complete interface.packages/plugins/ai/README.md-56-57 (1)
56-57: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAwait
clearAiCache().
clearAiCache()returnsPromise<void>. Withoutawait, a subsequent request can read stale data while asynchronous adapter eviction is still running.-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 56 - 57, Update the README example to await the Promise returned by clearAiCache(), ensuring subsequent requests only run after asynchronous cache eviction completes.packages/plugins/ai/doc/index.md-49-49 (1)
49-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the exported
TempoScheduleResultshape everywhere.
TempoScheduleResultexposesstart,end,slot,alternatives, andai.conflictBumped;startTempo/endTempoare only internal resolver bindings. Updatepackages/plugins/ai/doc/index.mdand the roadmap/API docs so public docs match the function 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/plugins/ai/doc/index.md` at line 49, Update the scheduleAI documentation entry and related roadmap/API documentation to describe the exported TempoScheduleResult fields—start, end, slot, alternatives, and ai.conflictBumped—instead of the internal startTempo/endTempo resolver bindings. Keep the public result description aligned with the actual function return shape everywhere.packages/plugins/ai/src/core/init.ts-172-190 (1)
172-190: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe snapshot is shallow, so the documentation overstates the guarantee.
The comment on Line 173 states that the return value is immutable.
Object.freezeis shallow. Each cloned provider keeps a shared reference to itsoptionsobject, and the spread on Line 187 exposes the livecacheMap and the livecacheAdapter. A caller can mutate provideroptionsor the cache through the returned snapshot.Either deep-copy the provider
options, or adjust the documentation to state that only the top level and the provider array are frozen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/core/init.ts` around lines 172 - 190, Update getAiConfig so its documentation accurately describes the shallow immutability guarantee: state that only the returned top-level object and providers array are frozen, without claiming an immutable snapshot. Preserve the existing API behavior and redaction logic.packages/plugins/ai/src/core/support.ts-68-72 (1)
68-72: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate
modelalongsideurl.Line 69 asserts
provider.model!but no check follows. Line 133 then setsmodel: model. Ifmodelis undefined,JSON.stringifyomits the field and the provider returns a generic 400 error that does not identify the missing configuration. Theurlcheck at Line 71 already establishes the pattern for a clear local error.🐛 Proposed fix
if (!url || typeof url !== 'string') throw new TempoAiError(`Provider ${provider.id} missing valid endpoint URL.`, 400); + + if (!model || typeof model !== 'string') + throw new TempoAiError(`Provider ${provider.id} missing valid model identifier.`, 400);🤖 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 68 - 72, Validate the asserted model value alongside url in the provider configuration flow, before constructing the request at the later model assignment. Add a clear TempoAiError identifying provider.id and the missing or invalid model, while preserving the existing URL validation behavior.packages/plugins/ai/src/types/common.type.ts-57-58 (1)
57-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply the resolved TTL to
Tempo.cachewrites.
Tempo.cacheis a globalBoundedCache, and itsset(...)does not accept or accept a TTL argument. The current writeTempo.cache.set(cacheKey, parsedIso)uses the global cache TTL instead ofoptions.ttl,provider.ttl, orAiConfig.ttl. UseresolvedTtlfor the global cache write as well.🤖 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/common.type.ts` around lines 57 - 58, Update the Tempo.cache write to use resolvedTtl when storing parsedIso, ensuring the resolved value from options.ttl, provider.ttl, or AiConfig.ttl is applied instead of the global cache default.packages/plugins/ai/src/core/mode.ts-168-175 (1)
168-175: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
consensusto reserved provider IDs.
RESERVED_PROVIDER_IDSonly rejectsnativeandcache, whileexecuteConsensusModecan returnproviderId: AiMode.Consensus('consensus') on unanimous results. Addconsensusto the reserved set so user-configured provider IDs cannot collide with the synthetic consensus 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/plugins/ai/src/core/mode.ts` around lines 168 - 175, Add AiMode.Consensus (`'consensus'`) to RESERVED_PROVIDER_IDS alongside the existing reserved IDs, preventing configured providers from colliding with the synthetic providerId returned by executeConsensusMode.packages/plugins/ai/src/core/mode.ts-93-96 (1)
93-96: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCompare the normalized confidence values.
Line 93 defaults a missing
confidenceto1.0. Line 95 compares the new value againstbestCandidate.confidence ?? 0, which uses the raw stored value. If the stored best candidate had noconfidence, it is treated as0and a later candidate with a lower real score replaces it. Track the normalized score alongside the candidate.🐛 Proposed fix
let lastError: any = null; let bestCandidate: ModeCandidate<T> | null = null; + let bestConfidence = -1; for (const provider of providers) { try { const candidate = await task(provider); const confidence = typeof candidate.confidence === 'number' ? candidate.confidence : 1.0; - if (!bestCandidate || confidence > (bestCandidate.confidence ?? 0)) + if (!bestCandidate || confidence > bestConfidence) { bestCandidate = candidate; + bestConfidence = confidence; + }🤖 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/mode.ts` around lines 93 - 96, Update the candidate-selection logic around confidence normalization to track the normalized confidence score alongside bestCandidate. Compare each candidate’s normalized confidence against the stored normalized best score, including when the best candidate originally lacks confidence, so missing values consistently remain 1.0.packages/tempo/doc/1-getting-started/ai-integration.md-3-5 (1)
3-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unsupported zero-hallucination guarantee.
llms.txtcan provide project context, but it cannot guarantee hallucination-free AI output. Use wording about documented context and possible accuracy improvements at both sites.
packages/tempo/doc/1-getting-started/ai-integration.md#L3-L5: replace “hallucination-free Tempo code” with a project-context or accuracy statement.packages/tempo/doc/1-getting-started/installation.md#L250-L252: replace “zero-hallucination context” with the same accurate wording.🤖 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 3 - 5, Replace the unsupported “hallucination-free” guarantee in the llms.txt description with wording that says the rules provide documented project context and may improve AI-generated code accuracy. Apply the same wording to packages/tempo/doc/1-getting-started/ai-integration.md lines 3-5 and packages/tempo/doc/1-getting-started/installation.md lines 250-252.packages/tempo/doc/1-getting-started/ai-integration.md-74-76 (1)
74-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the documented
Tempo.parse()API in the sample.
Tempo.parseis currently documented as a static parse-rule reflection object, not an overload that accepts(value, layoutName). Replace this call with the API that returns a parsedTemporal.ZonedDateTime, such asnew Tempo('Q3 2026').toDateTime()if the layout should apply automatically.🤖 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 74 - 76, Update the date-parsing example to use the documented API that returns a parsed Temporal.ZonedDateTime, replacing the invalid two-argument Tempo.parse call with the appropriate Tempo construction and toDateTime flow while preserving the fiscal-quarter input behavior.packages/tempo/doc/3-extending-tempo/tempo.modularity.md-122-123 (1)
122-123: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not call the lifecycle idempotent.
Tempo.init()is a reset-and-reinitialize flow: base runtime state is cleared, user tokens/time counters are reset, default and persisted discovery are reapplied, and explicitpluginsare reprocessed. That is not no-op/idempotent behavior. Rename the note and state thatTempo.extend(...)is the path for plugins loaded after startup.Proposed wording
- **Idempotent Initialization Lifecycle**: `Tempo.init()` is designed to establish baseline configuration once during initial startup. + **Initialization Lifecycle**: `Tempo.init()` establishes baseline configuration during startup and can reset base runtime state when called again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/doc/3-extending-tempo/tempo.modularity.md` around lines 122 - 123, Rename the “Idempotent Initialization Lifecycle” note to describe reset-and-reinitialize behavior, and revise its text to state that Tempo.init() clears and reapplies runtime state, tokens, counters, discovery, and explicit plugins rather than being idempotent. Keep Tempo.extend(...) identified as the path for registering plugins loaded after startup.
🧹 Nitpick comments (20)
packages/tempo/public/providers.v1.json (1)
5-8: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse Groq’s non-deprecated completion-limit parameter.
tokenParamis forwarded to Groq requests via the provider options, so publishingtokenParam: "max_tokens"exposes Groq’s deprecated Chat Completions parameter to callers. Groq documentsmax_completion_tokensas the replacement.Proposed change
- "tokenParam": "max_tokens" + "tokenParam": "max_completion_tokens"🤖 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 set tokenParam to the non-deprecated max_completion_tokens value, while leaving its URL and model unchanged.Source: MCP tools
packages/plugins/ai/test/recurrence.test.ts (3)
17-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that no provider request occurs for a raw RRULE input.
The test name states "without network calls", but the test does not observe
fetch. Add a spy and assert zero calls.💚 Proposed change
it('should detect raw RRULE strings and parse them natively without network calls', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch'); const rruleInput = 'FREQ=WEEKLY;BYDAY=TU;BYHOUR=15'; @@ const batch1 = result.take(3); expect(batch1).toHaveLength(3); expect(batch1[0]).toBeInstanceOf(Tempo); + expect(fetchSpy).not.toHaveBeenCalled(); });🤖 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 17 - 33, Update the raw RRULE test around recurrenceAI to spy on fetch before invoking it, then assert that the spy records zero calls after processing the input. Preserve the existing result and batch assertions, and restore the spy during test cleanup.
143-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsensus tests bind mock responses to the provider dispatch order. Both consensus tests chain
mockResolvedValueOnce, so each response maps to an invocation index. Consensus mode dispatches the providers in parallel, so the mapping depends on the dispatch order rather than on the provider identity. The race tests in both files already key the response bybody.model.
packages/plugins/ai/test/recurrence.test.ts#L143-L167: replace the chainedmockResolvedValueOncecalls with amockImplementationthat selects the response frombody.model(m1andm2).packages/plugins/ai/test/schedule.test.ts#L253-L263: apply the samemockImplementationkeyed onbody.modelform1andm2.🤖 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 143 - 167, Update the consensus test mocks in packages/plugins/ai/test/recurrence.test.ts lines 143-167 and packages/plugins/ai/test/schedule.test.ts lines 253-263: replace chained mockResolvedValueOnce calls with mockImplementation that inspects the request body’s model and returns the corresponding m1 or m2 response. Keep each provider’s expected response tied to its model rather than dispatch order.
97-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe iterator ignores the
take()cursor.Line 84 advances the internal cursor to 5. Line 98 then iterates from index 0 and yields the same five occurrences again.
takeusesoffsetCursor, andcreateIteratoruses a localindexthat always starts at 0.Two independent cursors on one result object are easy to misuse. Document the behavior on
TempoRecurrenceResultinpackages/plugins/ai/src/types/recurrence.type.ts, or share one cursor.🤖 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 97 - 99, Update TempoRecurrenceResult and its createIterator/take behavior so iteration respects the cursor advanced by take(5), avoiding separate independent cursors that restart at index 0. Prefer sharing the result’s existing offsetCursor between take and createIterator; if independent cursors are intentional, document that behavior clearly on TempoRecurrenceResult in recurrence.type.ts.packages/plugins/ai/test/schedule.test.ts (2)
38-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a case where the anchor carries no zone information.
Line 39 builds the anchor with the
-07:00offset, which already matchesAmerica/Los_Angeles.scheduleAIconstructsanchorTempobefore it resolvestimeZone, so this test cannot detect the ordering defect. Add a case that passes a zone-less anchor string together withtimeZone, and assert theReference Anchor Timeline in the prompt.I raised the root cause on
packages/plugins/ai/src/functions/schedule.tsLine 172.🤖 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 38 - 45, Add a scheduleAI test case using a zone-less anchor string with the America/Los_Angeles timeZone, ensuring it exercises anchorTempo construction before timeZone resolution. Assert that the generated prompt’s Reference Anchor Time line reflects the resolved America/Los_Angeles interpretation.
164-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd coverage for a
workingHours.timeZonethat differs fromtimeZone.This test sets no
workingHours.timeZone, sowhTzequalstimeZone. The conflict loop inpackages/plugins/ai/src/functions/schedule.tsconverts towhTzat Lines 310-311 and converts back withnew Tempo(nextZdt, { timeZone })at Lines 315 and 326. That round trip is only exercised when the two zones differ.🤖 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 164 - 180, Update the scheduleAI test around the existing Friday conflict scenario to set workingHours.timeZone to a zone different from the top-level timeZone, while preserving the same expected scheduling behavior and assertions. This should exercise the conflict loop’s conversions in scheduleAI between the working-hours zone and timeZone, including conflictBumped and originalSlot values.packages/plugins/ai/src/functions/schedule.ts (2)
188-188: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDo not pre-resolve the timeout before you pass it to
fetchFromProvider.
fetchFromProviderresolves the timeout astimeoutOverride ?? provider.timeout ?? provider.options?.timeout ?? _state.config.timeout ?? 15000. Line 188 always produces a number, sotimeoutOverrideis always set andprovider.timeoutnever applies.recurrenceAIpassesoptions?.timeoutunchanged.♻️ Proposed change
- const callTimeout = options?.timeout ?? state.config.timeout ?? 15000; + const callTimeout = options?.timeout;🤖 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` at line 188, Pass options?.timeout directly to fetchFromProvider from the schedule flow instead of assigning the pre-resolved callTimeout value. Preserve fetchFromProvider’s fallback order so provider.timeout and provider.options?.timeout remain effective, matching recurrenceAI’s unchanged timeout behavior.
352-362: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFilter non-lazy
Tempofailures before buildingIntervalalternatives.
Tempoonly re-throws non-lazy parse failures; lazy strings andstrict/autoinvalid inputs can complete construction and later resolve throughisValid/#zdt. Skip alternatives whoseTempoboundaries resolve as invalid before returning theInterval.🤖 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 352 - 362, Update the alternatives mapping around the Tempo boundary construction to validate both start and end Tempo instances before creating an Interval. After constructing them, check their resolved validity using the existing Tempo validity mechanism, return null for any invalid boundary, and retain the current catch-and-filter behavior for thrown parse failures.packages/library/test/common/number.library.test.ts (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove these tests to an
international.librarytest file, or import fromnumber.library.The file is
number.library.test.tsand the suite name isNumber Library, but the subjectformatCurrencycomes from#library/international.library.js. A reader looking forinternational.librarycoverage will not find it here.Consider also pinning the locale in
formatCurrencycalls or asserting with a locale-independent matcher. The assertions at Lines 8, 13, 25, and 30 depend on.as the decimal separator, which changes under a non-English default runtime locale.🤖 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/number.library.test.ts` around lines 1 - 4, Move the formatCurrency tests from the Number Library suite into an international.library test file, or change the import and coverage to the number.library subject if that is the intended ownership. Update the formatCurrency calls or assertions at the affected cases to use an explicit locale or locale-independent expectations so decimal formatting does not depend on the runtime default locale.packages/plugins/ai/src/core/manifest.ts (1)
54-88: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNegative results are cached for the lifetime of the process.
Every failure path stores
{}in_cachedManifestMap. Line 35 then returns that empty entry on every later call. One transient timeout or one 503 disables remote provider defaults untilresetManifestCache()runs, and long-lived processes never recover.Record a timestamp with the empty entry and allow a retry after a short interval.
[reliability]🤖 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 54 - 88, The failure paths in the manifest fetch flow cache empty results indefinitely, preventing recovery from transient errors. Update the `_cachedManifestMap` handling used by the response validation and catch branches to store a timestamp with negative results, and make the lookup logic retry after a short negative-cache interval while preserving normal caching for valid manifests and `resetManifestCache()` behavior.packages/plugins/ai/src/core/init.ts (1)
129-158: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAwait non-native thenables returned by the cache adapter.
AiCacheAdapter.clearandAiCacheAdapter.deleteare declared asPromise<void> | void. Theres instanceof Promisechecks only match native promises. Several storage clients return a custom thenable or a promise from a different realm. In that caseclearAiCachereturns before the eviction completes, and a parse started immediately after can read a stale adapter value.Use
await Promise.resolve(res)inside the existingtryblock, which handles both the synchronous and the asynchronous case.♻️ Proposed change
if (adapter?.clear) { try { - const res = adapter.clear(); - if (res instanceof Promise) await res.catch(() => { }); + await Promise.resolve(adapter.clear()); } catch { } } @@ if (adapter) { try { if (adapter.delete) { - const res1 = adapter.delete(normalized); - if (res1 instanceof Promise) await res1.catch(() => { }); - const res2 = adapter.delete(i); - if (res2 instanceof Promise) await res2.catch(() => { }); + await Promise.resolve(adapter.delete(normalized)); + await Promise.resolve(adapter.delete(i)); } if (adapter.clear) { - const resClear = adapter.clear(prefix); - if (resClear instanceof Promise) await resClear.catch(() => { }); + await Promise.resolve(adapter.clear(prefix)); } } 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 129 - 158, Update clearAiCache’s adapter.clear and adapter.delete handling to await each returned value with Promise.resolve(res) instead of checking res instanceof Promise, including the initial adapter.clear path and the per-input delete/clear operations. Preserve the existing try/catch behavior while ensuring native promises, cross-realm promises, custom thenables, and synchronous returns are all handled before clearAiCache completes.packages/plugins/ai/src/types/parse.type.ts (1)
40-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the
localetype with the parser and narrow the catch-all index signature.
parseSingleInputinpackages/plugins/ai/src/functions/parse.tshandles an array locale withArray.isArray(options!.locale) ? options!.locale[0] : .... The declared type here islocale?: string, so that branch contradicts the contract. The[key: string]: anysignature on Line 65 hides the mismatch and also accepts any misspelled option without error.TempoScheduleOptionsextends this interface, so the same looseness applies toscheduleAI.Widen
locale, and consider replacing the catch-all with an explicit passthrough for core Tempo options.♻️ Proposed change
/** Target locale override */ - locale?: string; + locale?: string | string[];Also applies to: 64-65
🤖 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/parse.type.ts` around lines 40 - 41, Update the locale property in the parse options interface to accept the array form handled by parseSingleInput, while preserving support for string locales. Remove the broad [key: string]: any index signature and replace it with an explicit passthrough type for supported core Tempo options so misspelled options are rejected; ensure TempoScheduleOptions and scheduleAI inherit the narrowed contract.packages/plugins/ai/src/types/schedule.type.ts (1)
95-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive
TempoScheduleMetafromTempoScheduleResult.Six fields are duplicated between the two interfaces:
durationMinutes,summary,reasoning,confidence,provider, andalternatives. A later change to one interface can silently diverge from the other.♻️ Proposed change
-export interface TempoScheduleMeta { - /** Target slot duration in minutes */ - durationMinutes: number; - /** Human-friendly summary of the scheduled slot */ - summary: string; - /** Reasoning / explanation of why this slot was selected */ - reasoning?: string | undefined; - /** Confidence rating from 0.0 (unparseable) to 1.0 (certain) */ - confidence: number; - /** Provider ID responsible for processing or 'native-scheduler' */ - provider: string; - /** Alternative backup intervals identified during scheduling */ - alternatives?: Interval<Tempo>[] | undefined; +export interface TempoScheduleMeta extends Pick<TempoScheduleResult, + 'durationMinutes' | 'summary' | 'reasoning' | 'confidence' | 'provider' | 'alternatives'> { /** Extended AI execution metadata */ 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 95 - 110, Update TempoScheduleMeta to derive the six shared fields from TempoScheduleResult using the existing type utilities, while keeping the ai-specific metadata field defined locally. Remove the duplicated property declarations and preserve the current optionality and types inherited from TempoScheduleResult.packages/plugins/ai/test/mode.test.ts (2)
12-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the test name with the asserted behavior.
The name states "stop on first provider meeting minConfidence". Provider-a returns 0.7, which is below the 0.8 threshold, so execution cascades and the assertions expect two calls. Rename to describe the cascade, for example "should cascade in Fallback mode until a provider meets minConfidence". A separate test already covers the short-circuit case in
packages/plugins/ai/test/parse.test.ts.🤖 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/mode.test.ts` around lines 12 - 27, Rename the test case describing executeWithMode in AiMode.Fallback so it reflects cascading through providers until one meets minConfidence, while preserving the existing assertions and test behavior.
97-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the status code that the test name states.
The test name declares status 400. The assertion only checks the error type. Add a code assertion so a regression in the status value fails the test.
💚 Proposed change
- await expect(executeWithMode('unsupported' as any, mockProviders, task)) - .rejects.toThrow(TempoAiError); + await expect(executeWithMode('unsupported' as any, mockProviders, task)) + .rejects.toMatchObject({ name: 'TempoAiError', code: 400 }); + expect(task).not.toHaveBeenCalled();🤖 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/mode.test.ts` around lines 97 - 101, Update the test around executeWithMode to assert that the rejected TempoAiError has status 400, in addition to verifying its error type. Ensure the assertion checks the actual status value so changes to the invalid-mode response code fail the test.packages/plugins/ai/src/types/recurrence.type.ts (1)
9-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a narrower type for the occurrence window.
afterandbeforeare typedany. The downstream builder passes them toexpandOccurrencesas date-window bounds. A union such asTempo | Date | string | numberdocuments the accepted inputs and catches misuse at compile time.♻️ Proposed narrowing
- /** Start date/time window for occurrence expansion */ - after?: any; - /** End date/time window for occurrence expansion */ - before?: any; + /** Start date/time window for occurrence expansion */ + after?: Tempo | Date | string | number; + /** End date/time window for occurrence expansion */ + before?: Tempo | Date | string | number;Note:
import type { Tempo }already exists, so the value is not imported at runtime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugins/ai/src/types/recurrence.type.ts` around lines 9 - 14, Replace the any types of after and before in the recurrence options type with the narrower union Tempo | Date | string | number, reusing the existing type-only Tempo import and preserving their optional date-window behavior.packages/plugins/ai/src/functions/parse.ts (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the destructured values consistently.
Line 13 destructures
ttlandcacheAdapter. Line 34 readsoptions?.cacheAdapterand line 171 readsoptions?.ttl. The behavior is identical, but the mixed access makes it harder to see which option keys are excluded fromcoreOptions.♻️ Proposed change
- const adapter = options?.cacheAdapter ?? _state.config.cacheAdapter; + const adapter = cacheAdapter ?? _state.config.cacheAdapter;- const resolvedTtl = options?.ttl ?? winningProvider?.ttl ?? _state.config.ttl ?? 3600000; + const resolvedTtl = ttl ?? winningProvider?.ttl ?? _state.config.ttl ?? 3600000;Also applies to: 34-34, 171-171
🤖 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 13, Update the code around the parse function’s destructured options to use the existing cacheAdapter and ttl variables at the references currently accessing options?.cacheAdapter and options?.ttl. Keep the behavior unchanged and consistently rely on the destructured values excluded from coreOptions.packages/plugins/ai/src/core/mode.ts (1)
146-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNormalize the consensus rejection error.
Line 160 rethrows
firstRejected.reasondirectly. The race path routes failures throughunwrapExecutionError, which preservesTempoAiErrorand wraps other values with a 500 status. A non-Errorrejection from a provider task propagates unwrapped here. Use the same helper for consistent error contracts across modes.♻️ Proposed change
if (fulfilled.length === 0) { const firstRejected = settled.find(s => s.status === 'rejected') as PromiseRejectedResult | undefined; - throw firstRejected?.reason || new TempoAiError('Consensus failed: all providers rejected.', 500); + if (firstRejected === undefined) + throw new TempoAiError('Consensus failed: all providers rejected.', 500); + throw unwrapExecutionError(firstRejected.reason, 'Provider consensus failed'); }🤖 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/mode.ts` around lines 146 - 161, Update executeConsensusMode’s all-providers-rejected path to pass firstRejected.reason through unwrapExecutionError before throwing it, while retaining the existing TempoAiError fallback when no rejection reason is available. This must preserve TempoAiError instances and normalize non-Error rejection values with the same 500-status contract used by the race path.packages/plugins/ai/test/cache.test.ts (1)
122-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the arguments passed to
deleteandclear.
clearAiCache('Easter 2026')must remove only the matching entries. The test asserts thatdeleteandclearwere called, but not with which key or prefix. A regression that clears the whole adapter for a scoped request still passes. Assert the call arguments.🤖 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 122 - 138, Strengthen the test around clearAiCache so the scoped call clearAiCache('Easter 2026') verifies mockAdapter.delete receives the matching key and mockAdapter.clear receives the expected prefix or scope argument, while retaining the existing assertions for the unscoped clearAiCache() call.packages/plugins/ai/test/manifest.test.ts (1)
162-192: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the provider-to-response mapping explicit.
The test binds the hanging response to invocation 1 and the fast response to invocation 2 through
mockImplementationOnceordering. The mapping holds only whileinitAIissues the manifest fetch synchronously in call order. Key the mock on the request URL instead. The two invocations already use distinct manifest URLs.💚 Proposed change
- fetchSpy - .mockImplementationOnce(() => manifestPromise1 as any) - .mockResolvedValueOnce(new Response(JSON.stringify({ providers: { groq: { model: 'invocation-2-model' } } }), { status: 200 })); + fetchSpy.mockImplementation((input: any) => { + const url = String(input); + if (url.endsWith('manifest-1.json')) return manifestPromise1 as any; + return Promise.resolve(new Response(JSON.stringify({ providers: { groq: { model: 'invocation-2-model' } } }), { status: 200 })); + });🤖 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 162 - 192, Update the fetch mock in the revision-tracking test to return responses based on the request URL rather than mockImplementationOnce call order. Map manifest-1.json to the hanging manifestPromise1 and manifest-2.json to the immediate invocation-2 response, preserving the existing assertions and initAI flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1bb1116a-ef78-461b-a0b8-6df50a815d62
⛔ 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 (86)
.github/workflows/deploy-docs.ymlpackage.jsonpackages/library/CHANGELOG.mdpackages/library/README.mdpackages/library/package.jsonpackages/library/src/common.index.tspackages/library/src/common/calendar.library.tspackages/library/src/common/recurrence.library.tspackages/library/test/common/calendar.library.test.tspackages/library/test/common/class.library.test.tspackages/library/test/common/number.library.test.tspackages/library/test/common/recurrence.library.test.tspackages/library/test/common/reflection.library.test.tspackages/library/test/common/serialize.library.test.tspackages/library/test/common/string.library.test.tspackages/library/test/common/temporal.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/mode.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/mode.test.tspackages/plugins/ai/test/parse.test.tspackages/plugins/ai/test/recurrence.test.tspackages/plugins/ai/test/schedule.test.tspackages/plugins/vitest.shared.tspackages/tempo/.vitepress/config.tspackages/tempo/.vitepress/theme/data/catalog.jsonpackages/tempo/CHANGELOG.mdpackages/tempo/README.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/library.index.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
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (1)
packages/library/test/common/temporal.library.test.ts (1)
24-26: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In@js-temporal/polyfill0.5.1, how do Temporal.Instant.from(isoWithZ).toZonedDateTimeISO(timeZone) and Temporal.ZonedDateTime.from(isoWithZ) differ for an ISO string ending in Z?💡 Result:
In
@js-temporal/polyfill0.5.1, the difference between these two approaches when dealing with an ISO 8601 string ending in 'Z' (e.g., "2026-08-10T10:00:00Z") comes down to their requirements, purpose, and the resulting object's metadata [1][2]. Temporal.Instant.from(isoWithZ).toZonedDateTimeISO(timeZone) This is the correct pattern for converting an exact UTC time string to a ZonedDateTime in a specific time zone [1][2]. 1. Temporal.Instant.from(isoWithZ) creates a Temporal.Instant, which represents an exact point in time (UTC) and has no concept of a time zone or calendar [3][1]. 2..toZonedDateTimeISO(timeZone) then projects that exact instant into the calendar and wall-clock time of the specified timeZone [3][4]. 3. This method is flexible: it accepts any valid time zone identifier (e.g., 'America/New_York', 'UTC', or even an offset like '+05:00') [1][4]. Temporal.ZonedDateTime.from(isoWithZ) This approach will fail for a typical ISO string ending in 'Z' [1][2]. 1. Temporal.ZonedDateTime.from requires an RFC 9557 string, which must include a time zone identifier (e.g., [America/New_York]) [1][2]. 2. Because a standard ISO 8601 string ending in 'Z' lacks this required time zone identifier, Temporal.ZonedDateTime.from will throw a RangeError (or equivalent validation error) because it cannot unambiguously determine the time zone context [1][2]. In summary, Temporal.Instant.from(isoWithZ) is used to parse "exact time" strings (which include UTC offsets or 'Z'), while Temporal.ZonedDateTime.from is used to parse strings that explicitly contain both an exact time and a specific time zone identifier [1][2].Citations:
- 1: https://tc39.es/proposal-temporal/docs/zoneddatetime.html
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/ZonedDateTime/from
- 3: https://github.com/tc39/proposal-temporal/blob/main/docs/instant.md
- 4: https://tc39.es/proposal-temporal/docs/instant.html
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== files ==" git ls-files | rg '(^|/)packages/library/test/common/temporal.library.test.ts$|temporal' | head -100 echo "== package deps ==" for f in package.json packages/library/package.json; do if [ -f "$f" ]; then echo "--- $f" node -e 'const p=require("./'"$f"'"); console.log(JSON.stringify({dependencies:p.dependencies,devDependencies:p.devDependencies,peerDependencies:p.peerDependencies},null,2))' fi done echo "== test file context ==" cat -n packages/library/test/common/temporal.library.test.ts | sed -n '1,80p' echo "== implementation references ==" rg -n "toZonedDateTime|Temporal\.ZonedDateTime\.from|Temporal\.Instant\.from" packages/library -SRepository: magmacomputing/magma
Length of output: 6910
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== temporal.library.ts context ==" cat -n packages/library/src/common/temporal.library.ts | sed -n '100,145p' echo "== local availability checks ==" node - <<'JS' try { const {Temporal} = require('`@js-temporal/polyfill`'); console.log('polyfill available'); for (const [name, fn] of Object.entries({ withTz: () => Temporal.ZonedDateTime.from('2024-01-01T12:00:00[Australia/Sydney]'), withZTz: () => Temporal.ZonedDateTime.from('2024-01-01T12:00:00Z[Australia/Sydney]'), withZ: () => Temporal.ZonedDateTime.from('2024-01-01T12:00:00Z'), viaInstant: () => Temporal.Instant.from('2024-01-01T12:00:00Z').toZonedDateTimeISO('Australia/Sydney'), })) { try { const result = fn(); console.log(name, 'ok', result.toString(), result.timeZoneId); } catch (e) { console.log(name, 'error', e.constructor.name, e.message.split('\n').slice(0,3).join('\n')); } } } catch (e) { console.log('polyfill unavailable', e.message); } JSRepository: magmacomputing/magma
Length of output: 2303
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== temporal.library.ts context ==" cat -n packages/library/src/common/temporal.library.ts | sed -n '100,145p' echo "== local availability checks ==" node - <<'JS' try { const {Temporal} = require('`@js-temporal/polyfill`'); console.log('polyfill available'); for (const [name, fn] of Object.entries({ withTz: () => Temporal.ZonedDateTime.from('20Z4-01-01T12:00:00[Australia/Sydney]'), withZTz: () => Temporal.ZonedDateTime.from('2024-01-01T12:00:00Z[Australia/Sydney]'), withZ: () => Temporal.ZonedDateTime.from('2024-01-01T12:00:00Z'), viaInstant: () => Temporal.Instant.from('2024-01-01T12:00:00Z').toZonedDateTimeISO('Australia/Sydney'), })) { try { const result = fn(); console.log(name, 'ok', result.toString(), result.timeZoneId); } catch (e) { console.log(name, 'error', e.constructor.name, e.message.split('\n').slice(0,3).join('\n')); } } } catch (e) { console.log('polyfill unavailable', e.message); } JSRepository: magmacomputing/magma
Length of output: 2303
Support
Z-designated ISO input through an instant conversion.
Zmarks a valid UTC instant, butTemporal.ZonedDateTime.from()requires the string itself to include a bracketed time-zone annotation. This test accepts a throw for valid ISO input; convertZstrings viaTemporal.Instant.from(bag).toZonedDateTimeISO(tz)and assert that the instant is preserved instead of treatingZas an existing zone annotation.🤖 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/temporal.library.test.ts` around lines 24 - 26, Update the test around toZonedDateTime for the Z-designated input so it expects successful conversion rather than an error. Convert or assert against the resulting ZonedDateTime using the UTC instant 2024-01-01T12:00:00Z and the Australia/Sydney time zone, preserving the instant while applying the requested zone.
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 (6)
packages/plugins/ai/src/functions/schedule.ts (2)
284-300: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate working-day numbers before the active-day loop.
A value such as
days: [8]ordays: [0]entersactiveDaysSet. NoTemporal.ZonedDateTime.dayOfWeekvalue can match it. If adjustment callsadvanceToNextActiveDay, Line 298 loops forever.Reject invalid values, or filter to integer values from 1 through 7 before creating
activeDaysSet.🤖 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 284 - 300, Validate active day values in activeDaysList before constructing activeDaysSet, retaining only integer numbers from 1 through 7 and applying the weekday defaults when none remain. Ensure advanceToNextActiveDay always receives a set containing values that can match Temporal.ZonedDateTime.dayOfWeek, preventing an infinite loop.
304-358: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate schedule constraints before returning a slot.
Working-hour and active-day checks run only when the original AI slot overlaps a busy event. If
busyEventsis empty, a provider result outside working hours or on an inactive day is returned unchanged.The loop also returns the last slot after 50 iterations even if Line 349 still finds a conflict. Run the same constraint and overlap validation for every initial slot. Throw a
TempoAiErrorif adjustment reaches its limit without producing a valid slot.🤖 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 304 - 358, Update the schedule validation flow around the initial proposedInterval and conflict-adjustment loop so every provider slot is validated against activeDays, working hours, and busyEvents, including when busyEvents is empty. Reuse the existing constraint-adjustment logic for initially valid and conflicting slots, and after MAX_ADJUSTMENT_ITERATIONS verify the final interval; if it still violates any constraint or overlaps a busy event, throw a TempoAiError instead of returning it.packages/plugins/ai/doc/init.md (2)
21-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect remote manifest precedence.
Line 21 implies that remote manifest values override provider configuration.
initAImerges defaults before the caller provider object. Explicitprovidersvalues take precedence over remote defaults.Proposed fix
-while `await initAI()` guarantees that remote provider manifest overrides are fetched and applied before proceeding. +while `await initAI()` guarantees that remote provider defaults are fetched and merged before proceeding. Explicit `providers` values take precedence.🤖 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` at line 21, Update the initAI documentation to state that explicit provider configuration values take precedence over remote manifest defaults, rather than implying remote values override the caller’s providers. Preserve the distinction that awaiting initAI fetches and applies remote defaults before proceeding.
49-49: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the timeout hierarchy count.
Line 49 lists four precedence levels: call-site, provider, global, and default. Change
3-tierto4-levelor remove the 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/doc/init.md` at line 49, Correct the timeout hierarchy description in the “Prevent hanging requests” documentation by changing the inaccurate “3-tier” wording to “4-level” or removing the count, while preserving the listed precedence order.packages/tempo/public/esm_sh.index.html (2)
262-262: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose result changes to assistive technology.
The script updates
#resultafter asynchronous imports, but the element is not a live region. Screen readers can miss the transition fromInitializing Temporal...to the result or error.Add
role="status",aria-live="polite", andaria-atomic="true"to the result element.Proposed fix
- <div id="result" class="result pulse-loading">Initializing Temporal...</div> + <div id="result" class="result pulse-loading" role="status" aria-live="polite" aria-atomic="true">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` at line 262, Add role="status", aria-live="polite", and aria-atomic="true" to the `#result` element in packages/tempo/public/esm_sh.index.html so screen readers announce asynchronous result and error updates.
254-257: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate the copied snippet to initialize Temporal before Tempo.
packages/tempo/public/esm_sh.index.html:251-252showsimport '@js-temporal/polyfill'followed byimport { Tempo }. That can fail in environments without nativeTemporalbecause the polyfill import does not assignglobalThis.Temporal; the working loader uses async imports, assignsglobalThis.Temporal, then loads Tempo.Proposed fix
- import '`@js-temporal/polyfill`'; - import { Tempo } from '`@magmacomputing/tempo`'; + const { Temporal } = await import('`@js-temporal/polyfill`'); + if (!globalThis.Temporal) globalThis.Temporal = Temporal; + const { Tempo } = await import('`@magmacomputing/tempo`');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/public/esm_sh.index.html` around lines 254 - 257, Update the copied example snippet around the Temporal and Tempo imports to asynchronously load the Temporal polyfill, assign its exported Temporal implementation to globalThis.Temporal, and only then import and initialize Tempo. Keep the existing formatting example unchanged after Tempo has been loaded.
🧹 Nitpick comments (1)
packages/plugins/ai/test/dispatch.test.ts (1)
99-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a direct rejection assertion.
If
executeWithModeresolves unexpectedly,thrownErrorremains undefined and Line 106 raises a secondaryTypeError. Use Vitest’s rejection assertions to report the actual failure and avoidany.Proposed test change
- let thrownError: any; - try { - await executeWithMode('unsupported' as any, mockProviders, task); - } catch (err) { - thrownError = err; - } - expect(thrownError).toBeInstanceOf(TempoAiError); - expect(thrownError.status).toBe(400); - expect(thrownError.code).toBe(400); + const invalidMode = 'unsupported' as unknown as AiMode; + const rejected = executeWithMode(invalidMode, mockProviders, task); + await expect(rejected).rejects.toBeInstanceOf(TempoAiError); + await expect(rejected).rejects.toMatchObject({ status: 400, code: 400 });🤖 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/dispatch.test.ts` around lines 99 - 107, Update the test for executeWithMode to use Vitest’s direct rejection assertion instead of manually catching into thrownError. Assert the rejected error is a TempoAiError and verify its status and code without using any or risking property access on an undefined value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/library/src/common/temporal.library.ts`:
- Around line 132-136: Update the temporal parsing conditional around
Temporal.ZonedDateTime.from so calendar-only bracket annotations receive the
supplied fallback zone before parsing. Preserve existing handling for explicit
[... ]`@tz` or tz[...] annotations, and add a regression test covering a
calendar-qualified local input that produces the fallback-zone result.
In `@packages/library/test/common/proxy.library.test.ts`:
- Around line 49-64: Update the constructor identity test around proxify to
exercise the no-binding configuration by passing false for the frozen/bind
argument in both proxify calls. Preserve the existing CustomTarget and Object
constructor identity assertions, and rename the test if needed so it accurately
reflects the configuration being tested.
In `@packages/plugins/ai/test/manifest.test.ts`:
- Around line 55-68: Add a separate `loadRemoteManifest` assertion in the test
named “should allow http://localhost and http://127.0.0.1 remoteConfigUrl” using
an `http://127.0.0.1` manifest URL, and mock the corresponding successful fetch
response so both documented localhost exceptions are exercised.
In `@packages/plugins/ai/test/parse.test.ts`:
- Around line 118-120: Update the test around the Tempo result assertions to
remove the self-comparison in result.format === result.format. Store the first
result.format access in a local reference, then compare the subsequent format
access against that reference while preserving the existing assertion intent.
In `@packages/plugins/ai/test/schedule.test.ts`:
- Around line 339-341: Update the assertions in the test around the Interval
instance checks: retain the constructor assertion, store each slot method
reference once, then compare subsequent accesses against the stored references
instead of comparing each member to itself. Use the existing slot object and
preserve the test’s intent of verifying stable method identity.
---
Outside diff comments:
In `@packages/plugins/ai/doc/init.md`:
- Line 21: Update the initAI documentation to state that explicit provider
configuration values take precedence over remote manifest defaults, rather than
implying remote values override the caller’s providers. Preserve the distinction
that awaiting initAI fetches and applies remote defaults before proceeding.
- Line 49: Correct the timeout hierarchy description in the “Prevent hanging
requests” documentation by changing the inaccurate “3-tier” wording to “4-level”
or removing the count, while preserving the listed precedence order.
In `@packages/plugins/ai/src/functions/schedule.ts`:
- Around line 284-300: Validate active day values in activeDaysList before
constructing activeDaysSet, retaining only integer numbers from 1 through 7 and
applying the weekday defaults when none remain. Ensure advanceToNextActiveDay
always receives a set containing values that can match
Temporal.ZonedDateTime.dayOfWeek, preventing an infinite loop.
- Around line 304-358: Update the schedule validation flow around the initial
proposedInterval and conflict-adjustment loop so every provider slot is
validated against activeDays, working hours, and busyEvents, including when
busyEvents is empty. Reuse the existing constraint-adjustment logic for
initially valid and conflicting slots, and after MAX_ADJUSTMENT_ITERATIONS
verify the final interval; if it still violates any constraint or overlaps a
busy event, throw a TempoAiError instead of returning it.
In `@packages/tempo/public/esm_sh.index.html`:
- Line 262: Add role="status", aria-live="polite", and aria-atomic="true" to the
`#result` element in packages/tempo/public/esm_sh.index.html so screen readers
announce asynchronous result and error updates.
- Around line 254-257: Update the copied example snippet around the Temporal and
Tempo imports to asynchronously load the Temporal polyfill, assign its exported
Temporal implementation to globalThis.Temporal, and only then import and
initialize Tempo. Keep the existing formatting example unchanged after Tempo has
been loaded.
---
Nitpick comments:
In `@packages/plugins/ai/test/dispatch.test.ts`:
- Around line 99-107: Update the test for executeWithMode to use Vitest’s direct
rejection assertion instead of manually catching into thrownError. Assert the
rejected error is a TempoAiError and verify its status and code without using
any or risking property access on an undefined value.
🪄 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: 5b5d1264-771a-46f9-b434-ead33b6f342f
📒 Files selected for processing (47)
.coderabbit.yaml.gitignorepackages/library/src/common/proxy.library.tspackages/library/src/common/recurrence.library.tspackages/library/src/common/temporal.library.tspackages/library/test/common/proxy.library.test.tspackages/library/test/common/recurrence.library.test.tspackages/library/test/common/temporal.library.test.tspackages/plugins/ai/README.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/init.mdpackages/plugins/ai/package.jsonpackages/plugins/ai/src/core/config.tspackages/plugins/ai/src/core/dispatch.tspackages/plugins/ai/src/core/error.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/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/dispatch.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/plugins/astro/package.jsonpackages/plugins/batch/package.jsonpackages/plugins/finance/package.jsonpackages/plugins/snap/package.jsonpackages/plugins/sync/package.jsonpackages/tempo/doc/1-getting-started/ai-integration.mdpackages/tempo/doc/1-getting-started/installation.mdpackages/tempo/doc/3-extending-tempo/tempo.modularity.mdpackages/tempo/doc/3-extending-tempo/tempo.registry.mdpackages/tempo/public/esm_sh.index.htmlpackages/tempo/src/engine/engine.composer.tspackages/tempo/src/plugin/license/license.manager.tspackages/tempo/src/plugin/license/license.validator.tspackages/tempo/src/support/support.default.tspackages/tempo/src/support/support.enum.tspackages/tempo/src/support/support.init.tspackages/tempo/test/discrete/standalone_parse.test.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/plugins/ai/src/types/recurrence.type.ts
- packages/tempo/src/support/support.enum.ts
- packages/plugins/ai/package.json
- packages/tempo/doc/1-getting-started/ai-integration.md
- packages/tempo/doc/1-getting-started/installation.md
- packages/plugins/ai/src/types/parse.type.ts
- packages/plugins/ai/src/types/schedule.type.ts
- packages/plugins/ai/test/cache.test.ts
- packages/plugins/ai/doc/index.md
- packages/plugins/ai/src/functions/parse.ts
- packages/library/test/common/recurrence.library.test.ts
- packages/plugins/ai/test/recurrence.test.ts
- packages/library/src/common/recurrence.library.ts
- packages/plugins/ai/src/core/manifest.ts
- packages/tempo/doc/3-extending-tempo/tempo.modularity.md
- packages/plugins/ai/src/functions/recurrence.ts
- packages/plugins/ai/README.md
- packages/tempo/src/engine/engine.composer.ts
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 (3)
packages/plugins/ai/src/functions/schedule.ts (1)
308-366: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate working-hours constraints when no conflict exists.
The active-day and working-hours adjustment only runs inside
if (conflictingEvent). If the provider returns a Sunday slot, or an out-of-hours slot, with no overlap inbusyEvents,scheduleAIreturns it unchanged.Apply the constraint validation to the initial proposal and after every conflict bump. If the iteration limit is reached without a valid slot, throw an error instead of returning a conflicting or out-of-hours interval.
🤖 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 308 - 366, Refactor the scheduling flow around the initial proposedInterval and conflict-adjustment loop so active-day and working-hours validation runs even when conflictingEvent is absent, and is re-applied after every bump. Continue checking busyEvents after each adjustment; only return once the slot satisfies both constraints, and throw an error if MAX_ADJUSTMENT_ITERATIONS is exhausted without a valid interval.packages/library/src/common/recurrence.library.ts (1)
406-419: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not fabricate a next occurrence when expansion returns no result.
The one-day fallback can return a timestamp that does not satisfy
FREQ,BY*filters,COUNT, orUNTIL. ForFREQ=DAILY;COUNT=1queried after its only occurrence, the returned next-day timestamp is not an occurrence.Return an explicit no-occurrence result and update callers, or continue searching until a valid occurrence is found.
🤖 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/recurrence.library.ts` around lines 406 - 419, The getNextRRuleEpoch function must not fabricate a timestamp when expandRRuleEpochs returns no occurrences. Remove the one-day fallback and establish an explicit no-occurrence result, then update all callers of getNextRRuleEpoch to handle that result while preserving valid expanded occurrences.packages/library/src/common/coercion.library.ts (1)
111-117: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve BigInt-literal strings before calling
asNumber.
isNumericreturnstruefor values such as9007199254740993nthroughRE_BIGINT_LITERALinpackages/library/src/common/assertion.library.tsLine 50. This branch only recognizes decimal text withRE_INTEGER, so it callsasNumber.parseFloatthen returns a rounded, unsafeNumber.Detect
isIntegerLike(numStr)first, remove the trailingn, and apply the existing safe-integer threshold.🐛 Proposed fix
const numStr = String(str); + if (isIntegerLike(numStr)) { + const big = BigInt(numStr.slice(0, -1)); + if (big > BigInt(Number.MAX_SAFE_INTEGER) || big < BigInt(Number.MIN_SAFE_INTEGER)) return big; + return Number(big); + } if (RE_INTEGER.test(numStr)) {🤖 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/coercion.library.ts` around lines 111 - 117, Update the numeric coercion case around isNumeric and asNumber to detect BigInt-literal strings with isIntegerLike(numStr) before calling asNumber. Remove the trailing n, convert the remaining text with BigInt, and apply the existing Number safe-integer threshold so unsafe values return BigInt while safe values retain current behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/library/src/common/recurrence.library.ts`:
- Around line 16-21: Update RE_RRULE_FREQ to require a semicolon or end-of-input
after the supported frequency value, and update RE_FINITE_RRULE to require the
UNTIL or COUNT token at the start of input or immediately after a semicolon.
Preserve case-insensitive matching while preventing suffixes such as DAILYXYZ
and prefixes such as XCOUNT from matching.
- Around line 108-114: Update the RE_DATE_8DIGIT branch in the UNTIL parsing
logic to reject invalid calendar dates before assigning untilMs. Construct the
UTC date using setUTCFullYear so years below 100 are preserved, then round-trip
and compare its UTC year, month, and day with the parsed values; reject
mismatches instead of allowing Date.UTC normalization.
In `@packages/library/src/common/string.library.ts`:
- Line 10: Update RE_PARAM_MARKER to capture one or more digits so sprintf
substitutes multi-digit markers such as ${10}; add a test covering at least 11
arguments and verifying the higher-index marker is replaced.
In `@packages/library/src/common/temporal.library.ts`:
- Line 119: Update RE_OFFSET_SUFFIX usage in toZonedDateTime so the
Temporal.Instant.from path is selected only when the input includes a normalized
time component (str.includes('T')), preventing date-only values from being
treated as offset timestamps. Preserve fallback-zone handling for date-only
inputs and add a regression test covering a date-only value.
In `@packages/plugins/ai/src/functions/schedule.ts`:
- Around line 284-296: Update the numeric-string handling in the rawDays mapping
for validDays so only complete integer day strings from 1 through 7 are
accepted. Replace the permissive parseInt-based validation with full-string
validation, ensuring values like “1junk” and “1.5” are discarded while valid
numeric strings and DayKey names retain their current behavior.
---
Outside diff comments:
In `@packages/library/src/common/coercion.library.ts`:
- Around line 111-117: Update the numeric coercion case around isNumeric and
asNumber to detect BigInt-literal strings with isIntegerLike(numStr) before
calling asNumber. Remove the trailing n, convert the remaining text with BigInt,
and apply the existing Number safe-integer threshold so unsafe values return
BigInt while safe values retain current behavior.
In `@packages/library/src/common/recurrence.library.ts`:
- Around line 406-419: The getNextRRuleEpoch function must not fabricate a
timestamp when expandRRuleEpochs returns no occurrences. Remove the one-day
fallback and establish an explicit no-occurrence result, then update all callers
of getNextRRuleEpoch to handle that result while preserving valid expanded
occurrences.
In `@packages/plugins/ai/src/functions/schedule.ts`:
- Around line 308-366: Refactor the scheduling flow around the initial
proposedInterval and conflict-adjustment loop so active-day and working-hours
validation runs even when conflictingEvent is absent, and is re-applied after
every bump. Continue checking busyEvents after each adjustment; only return once
the slot satisfies both constraints, and throw an error if
MAX_ADJUSTMENT_ITERATIONS is exhausted without a valid interval.
🪄 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: a352e222-1fba-473b-b752-1c7e9d0dcfaa
📒 Files selected for processing (21)
packages/library/src/common/assertion.library.tspackages/library/src/common/cipher.library.tspackages/library/src/common/coercion.library.tspackages/library/src/common/international.library.tspackages/library/src/common/object.library.tspackages/library/src/common/primitive.library.tspackages/library/src/common/recurrence.library.tspackages/library/src/common/request.library.tspackages/library/src/common/serialize.library.tspackages/library/src/common/string.library.tspackages/library/src/common/temporal.library.tspackages/library/src/common/webtoken.library.tspackages/library/test/common/proxy.library.test.tspackages/library/test/common/temporal.library.test.tspackages/plugins/ai/doc/init.mdpackages/plugins/ai/src/functions/schedule.tspackages/plugins/ai/test/dispatch.test.tspackages/plugins/ai/test/manifest.test.tspackages/plugins/ai/test/parse.test.tspackages/plugins/ai/test/schedule.test.tspackages/tempo/public/esm_sh.index.html
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/library/test/common/proxy.library.test.ts
- packages/plugins/ai/test/dispatch.test.ts
- packages/plugins/ai/doc/init.md
- packages/plugins/ai/test/schedule.test.ts
- packages/plugins/ai/test/manifest.test.ts
- packages/tempo/public/esm_sh.index.html
- packages/plugins/ai/test/parse.test.ts
There was a problem hiding this comment.
Actionable comments posted: 4
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/src/functions/parse.ts (1)
40-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not read context-free cache keys.
The normalized and raw fallback keys omit the anchor and temporal context. A custom adapter entry created for another timezone, locale, calendar, or anchor can return an incorrect temporal result.
Restrict cache reads to
cacheKey. If legacy-key compatibility is required, migrate entries into a context-scoped key before using them.🤖 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` around lines 40 - 64, Restrict cache reads in the adapter lookup and Tempo.cache fallback within the parsing flow to the context-scoped cacheKey only. Remove the normalizedStr and str fallback reads so entries without anchor and temporal context cannot produce incorrect results; do not alter cache writes or introduce legacy-key reads.packages/library/src/common/coercion.library.ts (1)
87-88: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve precision for signed and padded integer strings.
The numeric gate accepts trimmed numeric strings, but
RE_INTEGERandisIntegerLikeinspect the untrimmed value.RE_INTEGERalso excludes an explicit+sign. Values such as+9007199254740993or9007199254740993can therefore fall through toasNumberand lose precision instead of returning abigint. (raw.githubusercontent.com)Normalize the string before the zero-prefix and integer checks. Handle
+before callingBigInt, and add regression tests for both forms.Proposed fix
-const RE_INTEGER = /^-?[0-9]+$/; +const RE_INTEGER = /^[+-]?[0-9]+$/; ... - case isNumeric(str) && (!str?.toString().startsWith('0') || stripZero): { - const numStr = String(str); + case isNumeric(str) && (!String(str).trim().startsWith('0') || stripZero): { + const numStr = String(str).trim(); + const integerStr = numStr.startsWith('+') ? numStr.slice(1) : numStr; ... - const big = BigInt(numStr); + const big = BigInt(integerStr);Also applies to: 113-118
🤖 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/coercion.library.ts` around lines 87 - 88, Update isIntegerLike and the surrounding coercion flow to trim string values before zero-prefix and integer checks, and extend RE_INTEGER to accept an optional leading plus sign. Remove the plus sign before calling BigInt so signed integer strings such as +9007199254740993 and padded equivalents return precise bigint values instead of reaching asNumber. Add regression tests covering both forms.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/library/src/common/type.library.ts`:
- Around line 201-206: Update the Singular<T> type to use MinLength<T, 4> so its
compile-time behavior matches singular: preserve 's' and 'bus' unchanged while
reducing 'cats' to 'cat'. Add compile-time tests covering all three cases, and
keep SingularUnit<T> based on the corrected Singular type.
In `@packages/plugins/ai/doc/modes.md`:
- Around line 48-50: Update the “No” branch from MultiUI to recommend
AiMode.Fallback instead of AiMode.Hedged, reflecting executeHedgedMode’s
fallback delegation when only one provider is configured; leave the
multi-provider Race path unchanged.
In `@packages/plugins/ai/src/core/config.ts`:
- Around line 11-19: Align the RoundRobin value in the exported AiMode object
with the documented literal `roundrobin`, ensuring callers using the documented
string are accepted while preserving the existing mode key.
In `@packages/plugins/ai/src/core/dispatch.ts`:
- Around line 377-381: Update the AiMode.Race branch in the dispatcher and
executeRaceMode to accept and apply options.minConfidence, keeping concurrent
requests active until a qualifying candidate is found. If none qualifies, wait
for all requests to settle and return the best candidate so caller-level
threshold handling remains unchanged; add a regression test covering a fast
low-confidence result versus a slower qualifying result.
---
Outside diff comments:
In `@packages/library/src/common/coercion.library.ts`:
- Around line 87-88: Update isIntegerLike and the surrounding coercion flow to
trim string values before zero-prefix and integer checks, and extend RE_INTEGER
to accept an optional leading plus sign. Remove the plus sign before calling
BigInt so signed integer strings such as +9007199254740993 and padded
equivalents return precise bigint values instead of reaching asNumber. Add
regression tests covering both forms.
In `@packages/plugins/ai/src/functions/parse.ts`:
- Around line 40-64: Restrict cache reads in the adapter lookup and Tempo.cache
fallback within the parsing flow to the context-scoped cacheKey only. Remove the
normalizedStr and str fallback reads so entries without anchor and temporal
context cannot produce incorrect results; do not alter cache writes or introduce
legacy-key reads.
🪄 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: 6082aa9e-d9b6-4124-a84a-74a54abeed4a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
packages/library/src/common/calendar.library.tspackages/library/src/common/coercion.library.tspackages/library/src/common/recurrence.library.tspackages/library/src/common/string.library.tspackages/library/src/common/temporal.library.tspackages/library/src/common/type.library.tspackages/library/test/common/recurrence.library.test.tspackages/library/test/common/string.library.test.tspackages/library/test/common/temporal.library.test.tspackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/modes.mdpackages/plugins/ai/src/core/config.tspackages/plugins/ai/src/core/dispatch.tspackages/plugins/ai/src/core/init.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/parse.type.tspackages/plugins/ai/test/dispatch.test.tspackages/plugins/ai/test/schedule.test.tspackages/tempo/.vitepress/config.tspackages/tempo/package.jsonpackages/tempo/src/plugin/extend/extend.recurrence.tspackages/tempo/test/plugins/extend.recurrence.test.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/plugins/ai/src/types/parse.type.ts
- packages/library/test/common/string.library.test.ts
- packages/tempo/src/plugin/extend/extend.recurrence.ts
- packages/library/test/common/temporal.library.test.ts
- packages/tempo/test/plugins/extend.recurrence.test.ts
- packages/plugins/ai/src/index.ts
- packages/library/src/common/recurrence.library.ts
- packages/library/test/common/recurrence.library.test.ts
- packages/tempo/package.json
- packages/library/src/common/calendar.library.ts
- packages/plugins/ai/src/functions/schedule.ts
- packages/plugins/ai/src/core/support.ts
- packages/library/src/common/string.library.ts
- packages/plugins/ai/src/functions/recurrence.ts
- packages/plugins/ai/src/core/init.ts
- packages/plugins/ai/doc/index.md
- packages/plugins/ai/src/types/common.type.ts
- packages/library/src/common/temporal.library.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/plugins/ai/src/functions/parse.ts (2)
37-47: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound custom cache I/O.
A pending
adapter.get(cacheKey)oradapter.set(...)can blockparseSingleInputindefinitely. Apply a separate cache timeout and fail open when it expires.🤖 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` around lines 37 - 47, Bound both custom cache operations in parseSingleInput: wrap adapter.get(cacheKey) and adapter.set(...) with the separate cache-timeout mechanism, and treat timeout or other cache errors as cache misses/no-ops so parsing continues without blocking indefinitely.Source: MCP tools
161-162: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine a TTL policy for unanimous consensus.
executeConsensusModesetsproviderIdtoAiMode.Consensus, sowinningProvideris undefined and provider-specific TTLs are ignored. Carry the participating provider TTLs or apply a deterministic consensus TTL policy before callingadapter.set.🤖 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` around lines 161 - 162, Update executeConsensusMode and the surrounding TTL resolution in parse.ts so consensus does not rely on the undefined winningProvider lookup. Define and apply a deterministic consensus TTL policy using the participating providers’ TTLs before adapter.set, while preserving explicit ttl and configured fallback precedence.Source: MCP tools
🧹 Nitpick comments (1)
packages/plugins/ai/src/core/patterns.ts (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the shared-pattern claim true.
packages/plugins/ai/src/functions/schedule.tsstill defines duplicate markdown-fence and duration regex literals. Import these constants there, or narrow this module's documentation. This prevents future fixes from being applied to only some AI handlers. (raw.githubusercontent.com)🤖 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/patterns.ts` around lines 3 - 4, Update the schedule function module to import and reuse the shared markdown-fence and duration regex constants from the patterns module instead of defining duplicate literals, keeping the single-source-of-truth claim accurate.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/library/src/common/coercion.library.ts`:
- Around line 114-123: Update the numeric-string guard in the coercion case to
remove any optional leading sign before checking for a leading zero, while
preserving the existing stripZero override. Ensure signed zero-padded inputs
such as +012 and -012 remain strings when stripZero is false, and add regression
tests covering both forms.
---
Outside diff comments:
In `@packages/plugins/ai/src/functions/parse.ts`:
- Around line 37-47: Bound both custom cache operations in parseSingleInput:
wrap adapter.get(cacheKey) and adapter.set(...) with the separate cache-timeout
mechanism, and treat timeout or other cache errors as cache misses/no-ops so
parsing continues without blocking indefinitely.
- Around line 161-162: Update executeConsensusMode and the surrounding TTL
resolution in parse.ts so consensus does not rely on the undefined
winningProvider lookup. Define and apply a deterministic consensus TTL policy
using the participating providers’ TTLs before adapter.set, while preserving
explicit ttl and configured fallback precedence.
---
Nitpick comments:
In `@packages/plugins/ai/src/core/patterns.ts`:
- Around line 3-4: Update the schedule function module to import and reuse the
shared markdown-fence and duration regex constants from the patterns module
instead of defining duplicate literals, keeping the single-source-of-truth claim
accurate.
🪄 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: 133436eb-531e-4806-90aa-428012bb4be3
📒 Files selected for processing (14)
packages/library/src/common/assertion.library.tspackages/library/src/common/coercion.library.tspackages/library/src/common/type.library.tspackages/library/test/common/coercion.library.test.tspackages/library/test/common/type.library.test.tspackages/plugins/ai/doc/modes.mdpackages/plugins/ai/src/core/config.tspackages/plugins/ai/src/core/dispatch.tspackages/plugins/ai/src/core/patterns.tspackages/plugins/ai/src/functions/parse.tspackages/plugins/ai/src/functions/recurrence.tspackages/plugins/ai/src/functions/schedule.tspackages/plugins/ai/test/dispatch.test.tspackages/plugins/ai/test/parse.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/plugins/ai/doc/modes.md
- packages/library/src/common/assertion.library.ts
- packages/plugins/ai/src/core/config.ts
- packages/plugins/ai/src/functions/recurrence.ts
- packages/plugins/ai/test/dispatch.test.ts
- packages/plugins/ai/src/core/dispatch.ts
- packages/plugins/ai/test/parse.test.ts
|
@coderabbitai full review |
|
There was a problem hiding this comment.
Actionable comments posted: 9
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/plan/v0.3.0-roadmap.md (1)
17-32: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove
contextAIto the implemented handler section.This section states that all listed handlers remain scaffolded after v0.3.0.
contextAIis implemented and exported withAiContextOptions, notAiOptions.Move
contextAIinto section 1. Update the signature to useAiContextOptions.🤖 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 17 - 32, The roadmap incorrectly lists contextAI among scaffolded handlers. Move contextAI from section 2 into the implemented handler section 1 and update its documented signature to use AiContextOptions instead of AiOptions, preserving the existing return type and behavior.
🧹 Nitpick comments (1)
packages/tempo/.vitepress/theme/index.ts (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
@ts-ignorewith a Vite CSS type declaration.Add
/// <reference types="vite/client" />to an included.d.tsfile, then remove the directive. The repository has no documentation TypeScript configuration or CSS declaration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/tempo/.vitepress/theme/index.ts` around lines 5 - 6, Replace the `@ts-ignore` above the custom.css import in the theme entry with a Vite CSS type declaration. Add /// <reference types="vite/client" /> to an included .d.ts file, then remove the suppression while keeping the stylesheet import unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/library/src/common/coercion.library.ts`:
- Around line 82-86: Update the integer coercion logic around raw, clean, and
sign so that when clean.length is zero it converts raw instead of an empty
string, causing sign-only inputs such as "+" and "-" to throw SyntaxError.
Preserve the existing sign handling and trailing-n stripping behavior for
non-empty clean values.
- Around line 137-138: Update the numeric branch in the coercion logic around
isInteger, isNumber, and parseBigInt to pass only finite numeric values to
parseBigInt. Add the finite-number guard while preserving integer handling,
allowing NaN and infinities to continue through the existing fallback path.
In `@packages/plugins/ai/doc/contextAI.md`:
- Around line 96-105: Add the missing parseAI import to the pivot example in the
contextAI documentation, alongside the existing contextAI import, so the parseAI
invocation is self-contained and compilable.
- Around line 25-34: Expand the AiContextOptions table in contextAI.md to
document debug, providers, timeout, and hedgeDelay from AiContextOptions, and
document the handler’s softErrors batch-processing option. Include each option’s
type and behavior, or explicitly label the table as a partial list if the
complete contract is not being documented.
- Around line 11-16: Update the contextAI example to initialize the AI plugin
with configured providers via initAI before calling contextAI, or explicitly
state that initialization is required. Ensure the example no longer implies
contextAI works without prior plugin setup.
In `@packages/plugins/ai/plan/v0.3.0-roadmap.md`:
- Around line 59-64: Wrap the initAI example in the roadmap document with an
opening and closing fenced Markdown code block using the TypeScript language
tag, so the configuration renders as code while keeping the example content
unchanged.
In `@packages/plugins/ai/src/functions/context.ts`:
- Around line 41-56: Update the cache read/write flow in the context handler to
persist and restore the actual confidence value instead of hardcoding 1.0.
Before returning a cached context, validate its fields—including the IANA
timezone—and enforce minConfidence using the same checks as the non-cache path;
treat invalid or insufficient cached data as a cache miss and continue fetching.
- Around line 17-22: Update the cacheKey construction in the context generation
flow to include the resolved baseline values tz, loc, cal, and sph alongside
normalizedStr. Ensure requests with identical text but different baseline
context produce distinct cache entries.
- Around line 113-126: Update the confidence handling in the candidate-building
function to require parsedData.confidence to be a finite number between 0.0 and
1.0 inclusive; otherwise reject the candidate rather than defaulting to 0.9.
Ensure only validated confidence values are returned and passed to
executeWithMode.
---
Outside diff comments:
In `@packages/plugins/ai/plan/v0.3.0-roadmap.md`:
- Around line 17-32: The roadmap incorrectly lists contextAI among scaffolded
handlers. Move contextAI from section 2 into the implemented handler section 1
and update its documented signature to use AiContextOptions instead of
AiOptions, preserving the existing return type and behavior.
---
Nitpick comments:
In `@packages/tempo/.vitepress/theme/index.ts`:
- Around line 5-6: Replace the `@ts-ignore` above the custom.css import in the
theme entry with a Vite CSS type declaration. Add /// <reference
types="vite/client" /> to an included .d.ts file, then remove the suppression
while keeping the stylesheet import unchanged.
🪄 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: 4be1d103-5f12-43fa-ab0d-408900a33bc8
📒 Files selected for processing (30)
packages/library/src/common/assertion.library.tspackages/library/src/common/coercion.library.tspackages/library/src/common/type.library.tspackages/library/test/common/coercion.library.test.tspackages/library/test/common/type.library.test.tspackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/README.mdpackages/plugins/ai/doc/contextAI.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/init.mdpackages/plugins/ai/doc/modes.mdpackages/plugins/ai/doc/parseAI.mdpackages/plugins/ai/doc/recurrenceAI.mdpackages/plugins/ai/doc/scheduleAI.mdpackages/plugins/ai/plan/v0.3.0-roadmap.mdpackages/plugins/ai/src/core/config.tspackages/plugins/ai/src/core/dispatch.tspackages/plugins/ai/src/core/patterns.tspackages/plugins/ai/src/functions/context.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/context.type.tspackages/plugins/ai/src/types/index.tspackages/plugins/ai/test/context.test.tspackages/plugins/ai/test/dispatch.test.tspackages/plugins/ai/test/parse.test.tspackages/tempo/.vitepress/theme/custom.csspackages/tempo/.vitepress/theme/index.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/library/test/common/type.library.test.ts
- packages/plugins/ai/src/types/index.ts
- packages/library/test/common/coercion.library.test.ts
- packages/plugins/ai/test/dispatch.test.ts
- packages/plugins/ai/doc/index.md
- packages/plugins/ai/doc/modes.md
- packages/plugins/ai/README.md
- packages/library/src/common/type.library.ts
- packages/plugins/ai/src/core/dispatch.ts
- packages/library/src/common/assertion.library.ts
- packages/plugins/ai/src/functions/parse.ts
- packages/plugins/ai/src/core/patterns.ts
- packages/plugins/ai/src/functions/recurrence.ts
- packages/plugins/ai/src/core/config.ts
- packages/plugins/ai/src/functions/schedule.ts
- packages/plugins/ai/test/parse.test.ts
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/diffAI.md`:
- Around line 11-18: Update the standalone diffAI example to import initAI
alongside diffAI and initialize the AI providers before invoking diffAI. Keep
the existing Tempo setup and diff request unchanged.
In `@packages/plugins/ai/src/functions/diff.ts`:
- Around line 286-290: Update the batch path around Promise.allSettled so each
pair is accessed inside an async callback, ensuring malformed pair property
access becomes a settled rejection. Normalize every rejected result’s unknown
reason to a TempoAiError before returning it, rather than relying on a cast;
preserve fulfilled values unchanged.
- Around line 180-182: Update the result metric assignments near parsedData
handling to always use grounding.calendarDays, grounding.elapsedHours, and
grounding.businessDays. Remove provider-derived numeric fallbacks for these
fields, while preserving provider values only for formatted, reasoning, and
confidence.
- Around line 100-110: Validate parsedCache.formatted in the cache handling path
before constructing and returning the cached result: it must be a non-empty
string. If validation fails, do not return the cache entry and continue through
the existing provider path; preserve the current cached-result mapping for valid
values.
- Around line 79-80: Update the cacheKey construction in the diff function to
include the region value used by the provider, such as options.region, alongside
the existing cache inputs. Ensure calls with different regions produce distinct
cache keys while preserving the current key components.
- Around line 63-64: Update the startTempo and endTempo initialization to always
normalize both values to the requested tz: preserve construction for non-Tempo
inputs, and call set({ timeZone: tz }) on existing Tempo inputs before
calculating metrics or building the cache key.
In `@packages/plugins/ai/src/types/diff.type.ts`:
- Around line 60-61: Update the documentation comment for the mode property in
the relevant type definition to list all six supported execution modes:
AiMode.Fallback, AiMode.Race, AiMode.Consensus, AiMode.Hedged,
AiMode.RoundRobin, and AiMode.Adaptive.
In `@packages/plugins/ai/test/context.test.ts`:
- Around line 261-276: Update the confidence validation tests around contextAI
to separately cover a non-finite confidence value, such as NaN or Infinity, in
addition to the existing out-of-range 1.5 case. Either add a dedicated
fixture/assertion for the non-finite validation path or rename the current test
to describe only out-of-range confidence.
In `@packages/plugins/ai/test/diff.test.ts`:
- Around line 117-146: Update the cache-hit handling in diffAI so cached results
preserve the serialized confidence value from the original response instead of
replacing it with 1.0. Adjust the cache-hit assertion in the “should check cache
and skip network fetch on cache hits” test to expect 0.95 while keeping the
provider as cache and the existing fetch-count checks unchanged.
🪄 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: 861139e7-2be1-40ca-8be8-57095b1c1b5e
📒 Files selected for processing (16)
packages/library/src/common/coercion.library.tspackages/library/test/common/coercion.library.test.tspackages/plugins/ai/doc/contextAI.mdpackages/plugins/ai/doc/diffAI.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/plan/v0.3.0-roadmap.mdpackages/plugins/ai/src/functions/context.tspackages/plugins/ai/src/functions/diff.tspackages/plugins/ai/src/index.tspackages/plugins/ai/src/types/diff.type.tspackages/plugins/ai/src/types/index.tspackages/plugins/ai/test/context.test.tspackages/plugins/ai/test/diff.test.tspackages/tempo/.vitepress/env.d.tspackages/tempo/.vitepress/theme/index.tspackages/tempo/doc/3-extending-tempo/tempo.plugin.md
💤 Files with no reviewable changes (1)
- packages/tempo/.vitepress/theme/index.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/plugins/ai/doc/index.md
- packages/tempo/doc/3-extending-tempo/tempo.plugin.md
- packages/plugins/ai/src/types/index.ts
- packages/library/test/common/coercion.library.test.ts
- packages/plugins/ai/src/index.ts
- packages/plugins/ai/src/functions/context.ts
- packages/library/src/common/coercion.library.ts
There was a problem hiding this comment.
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/CHANGELOG.md (1)
35-36: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect or scope the documented default TTL.
The supplied
packages/plugins/ai/src/functions/context.tscontext shows a fallback of86_400_000milliseconds, which is 24 hours. This conflicts with the documented one-hour default. Scope the statement to handlers that use one hour, or document the context-specific 24-hour default.🤖 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/CHANGELOG.md` around lines 35 - 36, Update the “Cascading Cache TTL Hierarchy” changelog entry to match the implementation in context handling: either scope the one-hour default to handlers that actually use it, or document the context-specific 24-hour (86,400,000 ms) fallback. Ensure the documented TTL precedence remains accurate.
🧹 Nitpick comments (1)
packages/plugins/ai/plan/extractAI.plan.md (1)
62-66: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftSynchronize the planned public APIs with the exported implementations. Both plans define contracts that differ from the current runtime stubs. Align each plan, its public types, and its implementation before exposing these APIs.
packages/plugins/ai/plan/extractAI.plan.md#L62-L66: reconcileTempoEvent[],TempoExtractedEvent[], andTempoAiExtractResultinto one documented return contract.packages/plugins/ai/plan/formatAI.plan.md#L50-L54: update theformatAIstub to the planned overloads, or revise the plan to match the current scalarPromise<string>API.🤖 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/extractAI.plan.md` around lines 62 - 66, Synchronize both AI API plans and implementations: in packages/plugins/ai/plan/extractAI.plan.md (lines 62-66), reconcile TempoEvent[], TempoExtractedEvent[], and TempoAiExtractResult into one contract shared by the public types and extractAI implementation; in packages/plugins/ai/plan/formatAI.plan.md (lines 50-54), either update the formatAI stub to the planned overloads or revise the plan to match its current scalar Promise<string> API.
🤖 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.
Outside diff comments:
In `@packages/plugins/ai/CHANGELOG.md`:
- Around line 35-36: Update the “Cascading Cache TTL Hierarchy” changelog entry
to match the implementation in context handling: either scope the one-hour
default to handlers that actually use it, or document the context-specific
24-hour (86,400,000 ms) fallback. Ensure the documented TTL precedence remains
accurate.
---
Nitpick comments:
In `@packages/plugins/ai/plan/extractAI.plan.md`:
- Around line 62-66: Synchronize both AI API plans and implementations: in
packages/plugins/ai/plan/extractAI.plan.md (lines 62-66), reconcile
TempoEvent[], TempoExtractedEvent[], and TempoAiExtractResult into one contract
shared by the public types and extractAI implementation; in
packages/plugins/ai/plan/formatAI.plan.md (lines 50-54), either update the
formatAI stub to the planned overloads or revise the plan to match its current
scalar Promise<string> API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: af61c7ef-216d-453f-9124-5b981a11fea4
📒 Files selected for processing (10)
.coderabbit.yamlpackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/README.mdpackages/plugins/ai/doc/diffAI.mdpackages/plugins/ai/plan/extractAI.plan.mdpackages/plugins/ai/plan/formatAI.plan.mdpackages/plugins/ai/src/functions/diff.tspackages/plugins/ai/src/types/diff.type.tspackages/plugins/ai/test/context.test.tspackages/plugins/ai/test/diff.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/plugins/ai/README.md
- .coderabbit.yaml
- packages/plugins/ai/doc/diffAI.md
- packages/plugins/ai/test/context.test.ts
- packages/plugins/ai/src/types/diff.type.ts
- packages/plugins/ai/src/functions/diff.ts
- packages/plugins/ai/test/diff.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugins/ai/src/functions/format.ts`:
- Around line 81-86: Update the single-input formatAI overload and its
implementation parameter to use the supported scalar input union Tempo | Date |
string | number instead of any, matching FormatItem.date. Preserve the existing
overload behavior and options handling.
🪄 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: f153430f-393e-43e5-ba2b-61674de8036a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.jsonpackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/plan/extractAI.plan.mdpackages/plugins/ai/plan/v0.3.0-roadmap.mdpackages/plugins/ai/src/functions/extract.tspackages/plugins/ai/src/functions/format.tspackages/plugins/ai/src/index.tspackages/plugins/finance/package.jsonpackages/tempo/package.json
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/plugins/finance/package.json
- packages/plugins/ai/CHANGELOG.md
- packages/plugins/ai/src/index.ts
- packages/tempo/package.json
- packages/plugins/ai/plan/extractAI.plan.md
- packages/plugins/ai/plan/v0.3.0-roadmap.md
1 participant
Summary by CodeRabbit