support for Magma AI Proxy - #67
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (26)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds internal license scope updates and conditional revocation checks, introduces the AI parsing plugin package, makes Tempo internals lazy, expands localized parsing coverage, and updates release metadata. ChangesLicense scope updates
AI plugin package
Tempo runtime and release updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Tempo
participant LicenseManager
participant LicenseState
Caller->>Tempo: invoke $updateScopeStatus
Tempo->>LicenseManager: updateScopeStatus(scopeKey, status, error)
LicenseManager->>LicenseState: merge scope status and error
LicenseManager->>LicenseState: set overall status to Revoked when all scopes are revoked
sequenceDiagram
participant Application
participant parseAI
participant BoundedCache
participant Provider
Application->>parseAI: submit natural-language query
parseAI->>BoundedCache: read anchor-based cache key
BoundedCache-->>parseAI: cached result or miss
parseAI->>Provider: request unresolved query
Provider-->>parseAI: parsed date response
parseAI->>BoundedCache: store result with TTL and capacity limits
parseAI-->>Application: return Tempo result
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: 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/tempo/src/plugin/license/license.manager.ts`:
- Around line 132-135: Make revocation checking lazy by changing the response
contract used by the license manager from an already-created
res.revocationPromise to a factory, and invoke that factory only after
shouldSkipRevocation is false in the revocation flow. Update the license phase
test mock to provide the same lazy factory and spy on its invocation, verifying
opt-out licenses do not start the revocation request; modify both
packages/tempo/src/plugin/license/license.manager.ts:132-135 and
packages/tempo/test/plugins/license.phase1.test.ts:86-100.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bfaf2e82-e170-4bfe-abc3-f571285e9938
📒 Files selected for processing (8)
package.jsonpackages/library/package.jsonpackages/tempo/package.jsonpackages/tempo/src/plugin/license/license.manager.tspackages/tempo/src/support/support.index.tspackages/tempo/src/support/support.symbol.tspackages/tempo/src/tempo.class.tspackages/tempo/test/plugins/license.phase1.test.ts
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
packages/tempo/test/discrete/parse.locale.test.ts (1)
82-102: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the resolved date, not only parse validity.
These assertions do not verify that
próximo/proximomeans the next Monday or that article removal is applied. Use a fixed anchor and assert the resulting weekday/date for the accented, unaccented, and article-prefixed inputs.🤖 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/parse.locale.test.ts` around lines 82 - 102, Update the “test Spanish el próximo lunes” test to use a fixed anchor date, then assert the resolved date (including Monday’s weekday/date) for “próximo lunes”, “el próximo lunes”, and “el proximo lunes”. Retain validity checks as appropriate, ensuring the assertions verify both modifier resolution and article removal rather than parse success alone.packages/plugins/ai/test/index.spec.ts (1)
57-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winForce the AI path in the natural-language test.
This test is intended to validate mocked LLM parsing, but it neither passes
force: truenor asserts a network call. If native parsing later accepts this phrase, the test can pass without exercising the provider contract.🤖 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/index.spec.ts` around lines 57 - 61, Update the parseAI invocation in the natural-language test to pass force: true alongside the existing anchor and timeZone options, ensuring the test always exercises the mocked AI provider path.
🤖 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/CHANGELOG.md`:
- Line 6: Update the 0.1.0 fallback-routing release note to accurately describe
the shipped real provider requests and fallback routing, removing the “mocked
proxy” wording; only label it test-only scaffolding if that is the actual scope
of the implementation.
In `@packages/plugins/ai/doc/architecture.md`:
- Around line 28-34: The Azure OpenAI example and provider contract currently
assume provider.key is sent as a bearer token. Update the configuration around
the openai provider to support Azure’s api-key header for AZURE_API_KEY, or
explicitly rename/document the value as an Entra bearer token when using
Authorization; keep the authentication behavior consistent with the selected
credential type.
In `@packages/plugins/ai/doc/index.md`:
- Around line 72-75: Update the forced-evaluation example for parseAI to use a
natively handled phrase such as “Next Friday” instead of the AI-oriented query.
Correct the surrounding description so force is documented as skipping cache
lookup, not bypassing the cache entirely, since the result is still written to
cache.
In `@packages/plugins/ai/doc/rate-limits.md`:
- Line 56: Update the “Granular Time Gotcha” documentation to clarify that the
cache salt normally uses the execution date but uses the caller-provided anchor
date when options.anchor is set. Explain that a fixed anchor can keep the cache
key unchanged across midnight, so the automatic midnight miss is not guaranteed.
- Around line 62-63: Update the documentation example and its accompanying
guarantee to clarify that cache: false only disables caching and does not ensure
an LLM request. When demonstrating a required provider call, use force: true
together with cache: false in the parseAI options.
In `@packages/plugins/ai/package.json`:
- Line 21: Remove the main-branch check from the prepublishOnly script in
package.json, leaving the build step intact so publishing works from tag-based
and detached HEAD checkouts. Move branch validation to the release automation if
it is still required.
In `@packages/plugins/ai/README.md`:
- Around line 13-15: Fix MD028 blockquote formatting in
packages/plugins/ai/README.md lines 13-15 by removing or prefixing the blank
line between the security warning and disclaimer; apply the same treatment to
the blank line inside the experimental-plugin warning block in
packages/plugins/ai/doc/index.md lines 9-14.
- Around line 29-33: Validate each required provider environment variable before
constructing the provider list passed to initAI. In
packages/plugins/ai/README.md lines 29-33, guard GROQ_API_KEY before assigning
it to the groq provider key; apply the same conditional filtering to every
provider environment variable in packages/plugins/ai/doc/architecture.md lines
12-18 so AiProvider.key always receives a string.
In `@packages/plugins/ai/src/cache.ts`:
- Around line 83-101: Update the Map-compatible observation APIs in the TTL
cache class: ensure inherited forEach() and size access also invoke
evictExpired() before exposing entries or counts. Preserve the existing behavior
of keys(), values(), entries(), and Symbol.iterator(), and use the class’s
existing eviction mechanism rather than adding separate expiration logic.
- Around line 61-68: Update the eviction loop in the cache class’s set flow to
inspect the iterator result’s done flag rather than treating oldestKey ===
undefined as empty. Delete the yielded key, including an explicitly stored
undefined key, and only break when the iterator is exhausted so the cache
remains within maxSize.
In `@packages/plugins/ai/src/parseAI.type.ts`:
- Around line 44-50: Ensure the rate-limit parsing flow sanitizes provider
header values before assigning _state.limits: reject NaN, sentinel values such
as 999, and invalid Tempo resetAt values. Represent unavailable limits as null
or another established unknown form by updating AiRateLimits and its consumers,
rather than exposing malformed values as valid data.
In `@packages/tempo/src/tempo.class.ts`:
- Line 1642: Update `#resolve`() in packages/tempo/src/tempo.class.ts to check for
an existing resolved `#zdt` before accessing `#now`, and only read `#now` when
fallback or unresolved parsing requires it; retain lazy caching in `#now`. In
packages/tempo/CHANGELOG.md lines 12-14, keep the performance claim only once
the implementation and benchmark verify it; otherwise remove or revise the
claim.
---
Nitpick comments:
In `@packages/plugins/ai/test/index.spec.ts`:
- Around line 57-61: Update the parseAI invocation in the natural-language test
to pass force: true alongside the existing anchor and timeZone options, ensuring
the test always exercises the mocked AI provider path.
In `@packages/tempo/test/discrete/parse.locale.test.ts`:
- Around line 82-102: Update the “test Spanish el próximo lunes” test to use a
fixed anchor date, then assert the resolved date (including Monday’s
weekday/date) for “próximo lunes”, “el próximo lunes”, and “el proximo lunes”.
Retain validity checks as appropriate, ensuring the assertions verify both
modifier resolution and article removal rather than parse success alone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fcbd7c8a-67d6-4c9a-97b0-1fa09fe4a82e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
package.jsonpackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/LICENSEpackages/plugins/ai/README.mdpackages/plugins/ai/doc/architecture.mdpackages/plugins/ai/doc/context.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/rate-limits.mdpackages/plugins/ai/package.jsonpackages/plugins/ai/src/cache.tspackages/plugins/ai/src/error.tspackages/plugins/ai/src/index.tspackages/plugins/ai/src/parseAI.type.tspackages/plugins/ai/test/index.spec.tspackages/plugins/ai/test/tsconfig.jsonpackages/plugins/ai/tsconfig.jsonpackages/plugins/ai/tsup.config.tspackages/plugins/parseAI/README.mdpackages/tempo/.vitepress/theme/data/catalog.jsonpackages/tempo/CHANGELOG.mdpackages/tempo/doc/4-advanced-reference/tempo.locale.mdpackages/tempo/src/plugin/license/license.manager.tspackages/tempo/src/tempo.class.tspackages/tempo/src/tempo.version.tspackages/tempo/test/discrete/parse.locale.test.tspackages/tempo/test/plugins/license.phase1.test.ts
💤 Files with no reviewable changes (1)
- packages/plugins/parseAI/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tempo/test/plugins/license.phase1.test.ts
- packages/tempo/src/plugin/license/license.manager.ts
| If a relative query (like `"The Friday after Thanksgiving"`) is intercepted by the native `Tempo` layout engine, but the anchor context inheritance is returning an undesired timezone, you can forcefully bypass the deterministic engine and the cache by passing `force: true`: | ||
|
|
||
| ```typescript | ||
| const dt = await parseAI("Next Friday at 5pm", { | ||
| const dt = await parseAI("The Friday after Thanksgiving", { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the forced-evaluation example with actual parseAI behavior.
“The Friday after Thanksgiving” is documented elsewhere as an AI-oriented query, so it does not clearly demonstrate bypassing native parsing. Use a natively handled phrase such as “Next Friday”, or reword the example. Also, force skips cache lookup but the current implementation still writes the result to cache.
🤖 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` around lines 72 - 75, Update the
forced-evaluation example for parseAI to use a natively handled phrase such as
“Next Friday” instead of the AI-oriented query. Correct the surrounding
description so force is documented as skipping cache lookup, not bypassing the
cache entirely, since the result is still written to cache.
|
|
||
| > [!WARNING] | ||
| > **Granular Time Gotcha**: The cache key is automatically salted with the **calendar date** (`yyyy-mm-dd`) of execution. This brilliantly protects relative day queries (like `"tomorrow"`) because the cache automatically misses as soon as midnight strikes! However, if you are parsing granular, time-relative phrases (like `"in 5 minutes"` or `"next hour"`), the calendar date salt is not enough to prevent staleness on a long-running server. | ||
| > **Granular Time Gotcha**: The cache key is automatically salted with the **calendar date** (`yyyy-mm-dd`) of execution. This brilliantly protects relative day queries (like `"The Friday after Thanksgiving"`) because the cache automatically misses as soon as midnight strikes! However, if you pass `force: true` for granular time-relative phrases, the calendar date salt is not enough to prevent staleness on a long-running server. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the anchor-date exception.
The cache salt is derived from anchorTempo, not always the execution date. When callers provide options.anchor, the date can remain fixed across midnight, so the cache does not necessarily miss at midnight.
🤖 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` at line 56, Update the “Granular Time
Gotcha” documentation to clarify that the cache salt normally uses the execution
date but uses the caller-provided anchor date when options.anchor is set.
Explain that a fixed anchor can keep the cache key unchanged across midnight, so
the automatic midnight miss is not guaranteed.
| // The LLM will ALWAYS be queried, and the result will NOT be cached | ||
| const dt = await parseAI("in 5 minutes", { cache: false }); | ||
| const dt = await parseAI("The last Friday before Christmas", { cache: false }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not promise an LLM request for cache: false alone.
parseAI attempts native parsing before the cache logic, so { cache: false } bypasses caching but may still resolve without contacting a provider. Use { force: true, cache: false } when a provider call is required.
🤖 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 62 - 63, Update the
documentation example and its accompanying guarantee to clarify that cache:
false only disables caching and does not ensure an LLM request. When
demonstrating a required provider call, use force: true together with cache:
false in the parseAI options.
| > **CRITICAL SECURITY WARNING**: Raw LLM API keys must **never** be exposed in a client-side browser bundle. BYOK (Bring Your Own Key) is only secure on backend servers (Node, edge workers). For public frontend applications, you must use a proxy service. | ||
|
|
||
| > **LLM Output Disclaimer**: Large Language Models are probabilistic text generators, not deterministic calculators. Magma Computing Solutions and Tempo core maintainers provide `@magmacomputing/tempo-plugin-ai` "as-is". Developers and organizations are solely responsible for validating AI-generated date and time outputs before relying on them in financial, legal, medical, or time-critical production systems. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the MD028 blockquote formatting in both documentation files.
The blank lines inside these blockquotes can split the callouts and trigger markdownlint failures.
packages/plugins/ai/README.md#L13-L15: remove or prefix the blank line between the security warning and disclaimer.packages/plugins/ai/doc/index.md#L9-L14: remove or prefix the blank line inside the experimental-plugin warning block.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 14-14: Blank line inside blockquote
(MD028, no-blanks-blockquote)
📍 Affects 2 files
packages/plugins/ai/README.md#L13-L15(this comment)packages/plugins/ai/doc/index.md#L9-L14
🤖 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 13 - 15, Fix MD028 blockquote
formatting in packages/plugins/ai/README.md lines 13-15 by removing or prefixing
the blank line between the security warning and disclaimer; apply the same
treatment to the blank line inside the experimental-plugin warning block in
packages/plugins/ai/doc/index.md lines 9-14.
Source: Linters/SAST tools
| initAI({ | ||
| providers: [ | ||
| { id: 'groq', key: process.env.GROQ_API_KEY }, | ||
| ] | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate environment variables before assigning required provider keys.
Both documentation examples pass possibly undefined environment variables into AiProvider.key, which requires a string and otherwise can produce an undefined bearer token.
packages/plugins/ai/README.md#L29-L33: guardprocess.env.GROQ_API_KEYbefore passing it toinitAI.packages/plugins/ai/doc/architecture.md#L12-L18: guard each provider environment variable before constructing the provider list.
📍 Affects 2 files
packages/plugins/ai/README.md#L29-L33(this comment)packages/plugins/ai/doc/architecture.md#L12-L18
🤖 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 29 - 33, Validate each required
provider environment variable before constructing the provider list passed to
initAI. In packages/plugins/ai/README.md lines 29-33, guard GROQ_API_KEY before
assigning it to the groq provider key; apply the same conditional filtering to
every provider environment variable in packages/plugins/ai/doc/architecture.md
lines 12-18 so AiProvider.key always receives a string.
| * rather than using `new Tempo(..)`. | ||
| */ | ||
| /** @internal */ get #Tempo() { return this.constructor as typeof Tempo; } | ||
| /** @internal */ get #now(): Temporal.Instant { return this.#instant ??= instant(); } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
The lazy-clock change is not effective yet.
#resolve() reads #now before checking whether the instance already has a resolved #zdt, so explicit inputs still acquire the system clock. This also makes the release-note performance claim inaccurate.
packages/tempo/src/tempo.class.ts#L1642-L1642: defer the#nowaccess until a fallback or unresolved parse actually requires it.packages/tempo/CHANGELOG.md#L12-L14: retain the claim only after the implementation and benchmark confirm it.
📍 Affects 2 files
packages/tempo/src/tempo.class.ts#L1642-L1642(this comment)packages/tempo/CHANGELOG.md#L12-L14
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tempo/src/tempo.class.ts` at line 1642, Update `#resolve`() in
packages/tempo/src/tempo.class.ts to check for an existing resolved `#zdt` before
accessing `#now`, and only read `#now` when fallback or unresolved parsing requires
it; retain lazy caching in `#now`. In packages/tempo/CHANGELOG.md lines 12-14,
keep the performance claim only once the implementation and benchmark verify it;
otherwise remove or revise the claim.
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.
Actionable comments posted: 12
🧹 Nitpick comments (2)
packages/tempo/test/discrete/parse.locale.test.ts (1)
82-102: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the resolved date, not only parse validity.
These assertions do not verify that
próximo/proximomeans the next Monday or that article removal is applied. Use a fixed anchor and assert the resulting weekday/date for the accented, unaccented, and article-prefixed inputs.🤖 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/parse.locale.test.ts` around lines 82 - 102, Update the “test Spanish el próximo lunes” test to use a fixed anchor date, then assert the resolved date (including Monday’s weekday/date) for “próximo lunes”, “el próximo lunes”, and “el proximo lunes”. Retain validity checks as appropriate, ensuring the assertions verify both modifier resolution and article removal rather than parse success alone.packages/plugins/ai/test/index.spec.ts (1)
57-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winForce the AI path in the natural-language test.
This test is intended to validate mocked LLM parsing, but it neither passes
force: truenor asserts a network call. If native parsing later accepts this phrase, the test can pass without exercising the provider contract.🤖 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/index.spec.ts` around lines 57 - 61, Update the parseAI invocation in the natural-language test to pass force: true alongside the existing anchor and timeZone options, ensuring the test always exercises the mocked AI provider path.
🤖 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/CHANGELOG.md`:
- Line 6: Update the 0.1.0 fallback-routing release note to accurately describe
the shipped real provider requests and fallback routing, removing the “mocked
proxy” wording; only label it test-only scaffolding if that is the actual scope
of the implementation.
In `@packages/plugins/ai/doc/architecture.md`:
- Around line 28-34: The Azure OpenAI example and provider contract currently
assume provider.key is sent as a bearer token. Update the configuration around
the openai provider to support Azure’s api-key header for AZURE_API_KEY, or
explicitly rename/document the value as an Entra bearer token when using
Authorization; keep the authentication behavior consistent with the selected
credential type.
In `@packages/plugins/ai/doc/index.md`:
- Around line 72-75: Update the forced-evaluation example for parseAI to use a
natively handled phrase such as “Next Friday” instead of the AI-oriented query.
Correct the surrounding description so force is documented as skipping cache
lookup, not bypassing the cache entirely, since the result is still written to
cache.
In `@packages/plugins/ai/doc/rate-limits.md`:
- Line 56: Update the “Granular Time Gotcha” documentation to clarify that the
cache salt normally uses the execution date but uses the caller-provided anchor
date when options.anchor is set. Explain that a fixed anchor can keep the cache
key unchanged across midnight, so the automatic midnight miss is not guaranteed.
- Around line 62-63: Update the documentation example and its accompanying
guarantee to clarify that cache: false only disables caching and does not ensure
an LLM request. When demonstrating a required provider call, use force: true
together with cache: false in the parseAI options.
In `@packages/plugins/ai/package.json`:
- Line 21: Remove the main-branch check from the prepublishOnly script in
package.json, leaving the build step intact so publishing works from tag-based
and detached HEAD checkouts. Move branch validation to the release automation if
it is still required.
In `@packages/plugins/ai/README.md`:
- Around line 13-15: Fix MD028 blockquote formatting in
packages/plugins/ai/README.md lines 13-15 by removing or prefixing the blank
line between the security warning and disclaimer; apply the same treatment to
the blank line inside the experimental-plugin warning block in
packages/plugins/ai/doc/index.md lines 9-14.
- Around line 29-33: Validate each required provider environment variable before
constructing the provider list passed to initAI. In
packages/plugins/ai/README.md lines 29-33, guard GROQ_API_KEY before assigning
it to the groq provider key; apply the same conditional filtering to every
provider environment variable in packages/plugins/ai/doc/architecture.md lines
12-18 so AiProvider.key always receives a string.
In `@packages/plugins/ai/src/cache.ts`:
- Around line 83-101: Update the Map-compatible observation APIs in the TTL
cache class: ensure inherited forEach() and size access also invoke
evictExpired() before exposing entries or counts. Preserve the existing behavior
of keys(), values(), entries(), and Symbol.iterator(), and use the class’s
existing eviction mechanism rather than adding separate expiration logic.
- Around line 61-68: Update the eviction loop in the cache class’s set flow to
inspect the iterator result’s done flag rather than treating oldestKey ===
undefined as empty. Delete the yielded key, including an explicitly stored
undefined key, and only break when the iterator is exhausted so the cache
remains within maxSize.
In `@packages/plugins/ai/src/parseAI.type.ts`:
- Around line 44-50: Ensure the rate-limit parsing flow sanitizes provider
header values before assigning _state.limits: reject NaN, sentinel values such
as 999, and invalid Tempo resetAt values. Represent unavailable limits as null
or another established unknown form by updating AiRateLimits and its consumers,
rather than exposing malformed values as valid data.
In `@packages/tempo/src/tempo.class.ts`:
- Line 1642: Update `#resolve`() in packages/tempo/src/tempo.class.ts to check for
an existing resolved `#zdt` before accessing `#now`, and only read `#now` when
fallback or unresolved parsing requires it; retain lazy caching in `#now`. In
packages/tempo/CHANGELOG.md lines 12-14, keep the performance claim only once
the implementation and benchmark verify it; otherwise remove or revise the
claim.
---
Nitpick comments:
In `@packages/plugins/ai/test/index.spec.ts`:
- Around line 57-61: Update the parseAI invocation in the natural-language test
to pass force: true alongside the existing anchor and timeZone options, ensuring
the test always exercises the mocked AI provider path.
In `@packages/tempo/test/discrete/parse.locale.test.ts`:
- Around line 82-102: Update the “test Spanish el próximo lunes” test to use a
fixed anchor date, then assert the resolved date (including Monday’s
weekday/date) for “próximo lunes”, “el próximo lunes”, and “el proximo lunes”.
Retain validity checks as appropriate, ensuring the assertions verify both
modifier resolution and article removal rather than parse success alone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fcbd7c8a-67d6-4c9a-97b0-1fa09fe4a82e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
package.jsonpackages/plugins/ai/CHANGELOG.mdpackages/plugins/ai/LICENSEpackages/plugins/ai/README.mdpackages/plugins/ai/doc/architecture.mdpackages/plugins/ai/doc/context.mdpackages/plugins/ai/doc/index.mdpackages/plugins/ai/doc/rate-limits.mdpackages/plugins/ai/package.jsonpackages/plugins/ai/src/cache.tspackages/plugins/ai/src/error.tspackages/plugins/ai/src/index.tspackages/plugins/ai/src/parseAI.type.tspackages/plugins/ai/test/index.spec.tspackages/plugins/ai/test/tsconfig.jsonpackages/plugins/ai/tsconfig.jsonpackages/plugins/ai/tsup.config.tspackages/plugins/parseAI/README.mdpackages/tempo/.vitepress/theme/data/catalog.jsonpackages/tempo/CHANGELOG.mdpackages/tempo/doc/4-advanced-reference/tempo.locale.mdpackages/tempo/src/plugin/license/license.manager.tspackages/tempo/src/tempo.class.tspackages/tempo/src/tempo.version.tspackages/tempo/test/discrete/parse.locale.test.tspackages/tempo/test/plugins/license.phase1.test.ts
💤 Files with no reviewable changes (1)
- packages/plugins/parseAI/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/tempo/test/plugins/license.phase1.test.ts
- packages/tempo/src/plugin/license/license.manager.ts
🛑 Comments failed to post (6)
packages/plugins/ai/CHANGELOG.md (1)
6-6: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the fallback-routing release note.
The implementation performs real provider requests and fallback routing, so “mocked proxy” understates the behavior shipped in 0.1.0. Describe the actual provider behavior or clearly mark this as test-only scaffolding.
🤖 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` at line 6, Update the 0.1.0 fallback-routing release note to accurately describe the shipped real provider requests and fallback routing, removing the “mocked proxy” wording; only label it test-only scaffolding if that is the actual scope of the implementation.packages/plugins/ai/doc/architecture.md (1)
28-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Locate architecture and related provider files ==" git ls-files | rg '(^|/)architecture\.md$|ai|provider|openai|azure|auth|authorization' | head -200 echo echo "== Read target file excerpt ==" if [ -f packages/plugins/ai/doc/architecture.md ]; then nl -ba packages/plugins/ai/doc/architecture.md | sed -n '1,120p' fi echo echo "== Search provider key / Authorization usage ==" rg -n "Authorization|provider\.key|AuthorizationHeader|api_key|AZURE_API_KEY|openai\.azure|azure" -S . --glob '!node_modules' --glob '!dist' --glob '!build' | head -300Repository: magmacomputing/magma
Length of output: 1097
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== Read target file excerpt ==" if [ -f packages/plugins/ai/doc/architecture.md ]; then awk '{printf "%6d\t%s\n", NR, $0}' packages/plugins/ai/doc/architecture.md | sed -n '1,140p' fi echo echo "== Search provider key / Authorization usage ==" rg -n "Authorization|provider\.key|AuthorizationHeader|api_key|AZURE_API_KEY|openai\.azure|azure|Authorization:" -S packages packages/plugins/ai --glob '!node_modules' --glob '!dist' --glob '!build' | head -400 echo echo "== Read AI plugin source candidates ==" for f in packages/plugins/ai/src/index.ts packages/plugins/ai/src/parseAI.type.ts packages/plugins/ai/src/error.ts packages/plugins/ai/src/cache.ts; do if [ -f "$f" ]; then echo "--- $f ---" wc -l "$f" awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p' fi doneRepository: magmacomputing/magma
Length of output: 20738
🌐 Web query:
Azure OpenAI chat completions API authentication Authorization header api-key Bearer💡 Result:
Azure OpenAI Service supports multiple authentication methods for the chat completions API, categorized primarily into API Key-based and Token-based (Microsoft Entra ID) authentication [1][2]. API Key Authentication - api-key header: You can provide your API key directly in the api-key HTTP header [1][3][2]. - Authorization header: Alternatively, you can pass the API key via the Authorization header [1][4]. Token-based Authentication (Microsoft Entra ID) - Authorization header: This method requires an OAuth 2.0 access token [5][2]. - Format: The token must be provided in the Authorization header using the Bearer scheme: Authorization: Bearer {Azure_OpenAI_Auth_Token} [5][2]. - Implementation: When using Azure SDKs (such as the Azure OpenAI client libraries), this is often handled automatically using DefaultAzureCredential, which simplifies the process of obtaining and injecting the bearer token [6][7][8]. Microsoft recommends using token-based authentication (Microsoft Entra ID) over API keys as it is more secure and avoids the need to manage and store static keys [5][2][7]. Regardless of the method used, ensure your requests are sent to the correct resource endpoint [1][6].
Citations:
- 1: https://learn.microsoft.com/en-us/rest/api/microsoft-foundry/azureopenai/chat
- 2: https://learn.microsoft.com/en-us/azure/ai-foundry/openai/reference-preview
- 3: https://learn.microsoft.com/en-us/azure/foundry/openai/latest
- 4: https://learn.microsoft.com/en-us/azure/foundry/openai/authoring-reference-preview
- 5: https://learn.microsoft.com/en-us/azure/ai-foundry/openai/reference-preview-latest
- 6: https://learn.microsoft.com/en-us/azure/ai-foundry/openai/how-to/managed-identity
- 7: https://learn.microsoft.com/en-us/azure/developer/ai/keyless-connections
- 8: https://learn.microsoft.com/en-us/dotnet/ai/azure-ai-services-authentication
🌐 Web query:
Azure OpenAI chat completions authentication Authorization header api-key Bearer💡 Result:
Azure OpenAI supports two primary methods for authenticating requests: API Key authentication and Microsoft Entra ID (token-based) authentication [1][2]. Microsoft Entra ID authentication is recommended as the more secure approach [1][3]. API Key Authentication To authenticate using an API key, you must provide the key in the request header using the header name api-key [1][2]. Example (cURL): curl -X POST https://YOUR-RESOURCE-NAME.openai.azure.com/openai/v1/chat/completions \ -H "Content-Type: application/json" \ -H "api-key: $AZURE_OPENAI_API_KEY" \ -d '{...}' Microsoft Entra ID (Bearer Token) Authentication To authenticate using Microsoft Entra ID, you must provide an OAuth 2.0 access token in the Authorization header using the Bearer scheme [1][2]. Example (cURL): curl -X POST https://YOUR-RESOURCE-NAME.openai.azure.com/openai/v1/chat/completions \ -H "Content-Type: application/json" \ -H "Authorization: Bearer $AZURE_OPENAI_AUTH_TOKEN" \ -d '{...}' The required scope for obtaining the access token is typically https://cognitiveservices.azure.com/.default (or https://ai.azure.com/.default in certain AI Foundry contexts) [1][3][4]. When using official Azure SDKs (such as those for.NET, JavaScript/TypeScript, or Python), authentication is typically handled by providing a credential object (e.g., DefaultAzureCredential) or a token provider to the client constructor, which manages the bearer token acquisition automatically [5][6][4][7].
Citations:
- 1: https://learn.microsoft.com/en-us/azure/ai-foundry/openai/reference-preview-latest
- 2: https://learn.microsoft.com/en-us/azure/foundry/openai/api-version-lifecycle
- 3: https://learn.microsoft.com/en-us/azure/foundry/openai/reference-preview-latest
- 4: https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/openai/openai/README.md
- 5: https://learn.microsoft.com/en-us/azure/ai-foundry/openai/how-to/managed-identity
- 6: https://learn.microsoft.com/en-us/dotnet/ai/azure-ai-services-authentication
- 7: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/openai/Azure.AI.OpenAI/README.md
Use Azure’s supported API key header or a bearer token in this example.
The plugin sends
Authorization: Bearer ${provider.key}, but Azure OpenAI API keys are passed in theapi-keyheader. IfAZURE_API_KEYis an API key, change the example/contract to support that header; if it is an Entra token, clarify that the value is an auth token, not an API key.🤖 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/architecture.md` around lines 28 - 34, The Azure OpenAI example and provider contract currently assume provider.key is sent as a bearer token. Update the configuration around the openai provider to support Azure’s api-key header for AZURE_API_KEY, or explicitly rename/document the value as an Entra bearer token when using Authorization; keep the authentication behavior consistent with the selected credential type.packages/plugins/ai/package.json (1)
21-21: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== package file ==" sed -n '1,80p' packages/plugins/ai/package.json 2>/dev/null || true echo echo "== git rev-parse abbrev-ref behavior probe ==" tmp="$(mktemp -d)" cd "$tmp" git init -q echo ok > a.txt git add a.txt git commit -q -m init echo "attached main: $(git rev-parse --abbrev-ref HEAD)" git checkout -q --detach HEAD echo "detached HEAD: $(git rev-parse --abbrev-ref HEAD)" git checkout -q main echo "still attached main: $(git rev-parse --abbrev-ref HEAD)" git tag -a "v1.0.0" -m tag 2>/dev/null || true git checkout -q "v1.0.0" echo "tag checkout: $(git rev-parse --abbrev-ref HEAD)" rm -rf "$tmp"Repository: magmacomputing/magma
Length of output: 1709
🌐 Web query:
git rev-parse --abbrev-ref HEAD detached HEAD returns HEAD documentation💡 Result:
When running git rev-parse --abbrev-ref HEAD in a detached HEAD state, the command returns the literal string HEAD [1][2][3]. This occurs because the --abbrev-ref option attempts to provide a non-ambiguous short name for a reference [4][5]. In a normal state, where HEAD points to a branch (a symbolic reference), Git resolves this to the branch name [3]. However, when in a detached HEAD state, HEAD points directly to a commit object rather than a branch, and Git identifies this state by returning the name of the reference itself, HEAD [1]. This behavior differs from other commands like git symbolic-ref --short HEAD, which will exit with an error in a detached HEAD state because HEAD is no longer a symbolic reference [1][2]. If you need to distinguish between a named branch and a detached state in scripts, it is common practice to check the output of git symbolic-ref or use logic to handle the literal "HEAD" string [2][6].
Citations:
- 1: https://stackoverflow.com/questions/45028134/whats-the-difference-between-the-output-from-rev-parse-and-symbolic-ref
- 2: https://stackoverflow.com/questions/67812278/get-the-current-git-branch-name-or-commit-hash-if-detached
- 3: https://latchkey.dev/learn/command-reference/git-rev-parse
- 4: https://git-scm.com/docs/git-rev-parse.html
- 5: https://git-scm.com/docs/git-rev-parse
- 6: https://stackoverflow.com/questions/6245570/how-do-i-get-the-current-branch-name-in-git
Do not gate npm publishing on being checked out to
main.
prepublishOnlyruns from the checkout used by publish automation, including tag-based releases and detached HEAD checkouts, wheregit rev-parse --abbrev-ref HEADis notmain. This block exits beforenpm publishcan run; move this guard to release automation or explicitly support the release ref.🤖 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/package.json` at line 21, Remove the main-branch check from the prepublishOnly script in package.json, leaving the build step intact so publishing works from tag-based and detached HEAD checkouts. Move branch validation to the release automation if it is still required.packages/plugins/ai/src/cache.ts (2)
61-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the iterator’s
doneflag instead ofundefinedas the empty sentinel.
Kis generic andMappermitsundefinedkeys. If the oldest key isundefined, this condition breaks without deleting it, soset()can permanently exceedmaxSize.Proposed fix
- const oldestKey = super.keys().next().value; - if (oldestKey !== undefined) { - this.delete(oldestKey); + const oldest = super.keys().next(); + if (!oldest.done) { + this.delete(oldest.value);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.while (this.size > this.maxSize) { const oldest = super.keys().next(); if (!oldest.done) { this.delete(oldest.value); } else { break; } }🤖 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/cache.ts` around lines 61 - 68, Update the eviction loop in the cache class’s set flow to inspect the iterator result’s done flag rather than treating oldestKey === undefined as empty. Delete the yielded key, including an explicitly stored undefined key, and only break when the iterator is exhausted so the cache remains within maxSize.
83-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Enforce TTL through all Map observation APIs.
Inherited
forEach()andsizedo not callevictExpired(), so expired entries can still be returned or counted when consumers use the Map-compatible interface. Override those APIs or centralize expiration before every observation.🤖 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/cache.ts` around lines 83 - 101, Update the Map-compatible observation APIs in the TTL cache class: ensure inherited forEach() and size access also invoke evictExpired() before exposing entries or counts. Preserve the existing behavior of keys(), values(), entries(), and Symbol.iterator(), and use the class’s existing eviction mechanism rather than adding separate expiration logic.packages/plugins/ai/src/parseAI.type.ts (1)
44-50: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not expose malformed rate-limit headers as valid values.
The implementation can publish
NaN, sentinel values such as999, or an invalidTempowhen provider headers are missing or malformed. Validate/sanitize these values before assigning_state.limits, or represent unknown values as nullable inAiRateLimits.🤖 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/parseAI.type.ts` around lines 44 - 50, Ensure the rate-limit parsing flow sanitizes provider header values before assigning _state.limits: reject NaN, sentinel values such as 999, and invalid Tempo resetAt values. Represent unavailable limits as null or another established unknown form by updating AiRateLimits and its consumers, rather than exposing malformed values as valid data.
1 participant
Summary by CodeRabbit
parseAIbehavior.