Feature/v3.3.1 - #49
Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughVersion 3.3.1 introduces "Slick Object Mutations" for ChangesSlick Object Mutations for Tempo.set()
Sequence Diagram(s)sequenceDiagram
participant Caller
participant MutateSET as module.mutate (set branch)
participant SlickParser as Match.slickValue
participant TermEngine as resolveTermMutation
participant ZDT as currZdt (ZonedDateTime)
Caller->>MutateSET: .set({ mm: '>2', wkd: '>monday' })
MutateSET->>MutateSET: key 'mm' in SLICK_KEYS → validate string payload
MutateSET->>SlickParser: parse '>2'
SlickParser-->>MutateSET: operator='>', sh_nbr='2', unit='mm'
MutateSET->>ZDT: currZdt.add({ months: 2 })
ZDT-->>MutateSET: advanced ZonedDateTime
MutateSET->>MutateSET: key 'wkd' in SLICK_KEYS → validate string payload
MutateSET->>SlickParser: parse '>monday'
SlickParser-->>MutateSET: operator='>', unit='monday'
MutateSET->>TermEngine: resolveTermMutation('set', state, '>', 'monday')
TermEngine-->>MutateSET: weekday-shifted ZonedDateTime
MutateSET-->>Caller: mutated Tempo instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/tempo/plan/slick-syntax-duration-keys.md (1)
65-67: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMinor wording: simplify "exactly the same".
Line 66 uses "exactly the same" which could be concise as "the same" without loss of meaning. The mathematical equivalence is still clear.
✏️ Proposed wording improvement
-- **Analysis:** Mathematically, advancing by a negative number is stepping backwards. `>-2` should evaluate exactly the same as `<2` (and similarly `<-2` = `>2`). +- **Analysis:** Mathematically, advancing by a negative number is stepping backwards. `>-2` should evaluate the same as `<2` (and similarly `<-2` = `>2`).🤖 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/plan/slick-syntax-duration-keys.md` around lines 65 - 67, In the Analysis section of the slick-syntax-duration-keys.md file, simplify the wording in the sentence that describes the mathematical equivalence of advancing by negative numbers. Replace the phrase "exactly the same" with simply "the same" to make the sentence more concise while maintaining clarity about the mathematical equivalence. This change applies to the sentence structure discussing how ">-2" should evaluate in relation to "<2".Source: Linters/SAST 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/tempo/plan/slick-syntax-duration-keys.md`:
- Around line 3-4: The Status section in the slick-syntax-duration-keys.md plan
file currently indicates the feature is "Proposed for a future release (not
scheduled in the current release)" but this feature has been implemented and
released in v3.3.1. Update the Status section to reflect the actual completion
and release status of this feature instead of keeping it marked as proposed.
Change the status line to clearly indicate that this has been implemented and
released in version 3.3.1.
In `@packages/tempo/src/engine/engine.term.ts`:
- Around line 110-113: The weekday shift calculation is incorrectly handling
negative repeat counts (such as `>-2` or `<-3`). The repeat logic around lines
125-130 only applies when `nbr > 1`, which means negative counts are silently
treated as single positive-direction steps. To fix this, extend the repeat logic
condition to check for both `nbr > 1` AND `nbr < -1`, ensuring that the sign of
the repeat count properly controls the direction of the shift application
instead of being ignored.
In `@packages/tempo/src/module/module.mutate.ts`:
- Around line 80-93: The custom modifier words parsed from
state.config.registry.modifiers are not being normalized for backward
operations. When building the words Set from the registry modifiers, you need to
track which custom words correspond to backward operations (those mapped from
backward symbols like '-', '<', '<=', etc.). Then, when the sign logic checks
matchSlickValue at the line checking canonical backward values, extend the logic
to also recognize these custom backward words so they are properly treated as
backward shifts instead of forward shifts.
- Around line 63-70: The slick mutation early-returns in the 'set' type check
block and related slick paths (visible in the diff around lines 63-70, and also
at lines 105-121) occur before mutateDepth is incremented around line 132, but
the corresponding decrement always occurs around line 240-242, causing an
imbalance that allows mutateDepth to underflow and bypass recursion protection.
Fix this by ensuring mutateDepth is incremented before these early-return
checks, or by modifying the decrement logic around line 240-242 to only
decrement when a corresponding increment occurred, so the depth tracking remains
balanced across all code paths in the mutation handler.
---
Nitpick comments:
In `@packages/tempo/plan/slick-syntax-duration-keys.md`:
- Around line 65-67: In the Analysis section of the
slick-syntax-duration-keys.md file, simplify the wording in the sentence that
describes the mathematical equivalence of advancing by negative numbers. Replace
the phrase "exactly the same" with simply "the same" to make the sentence more
concise while maintaining clarity about the mathematical equivalence. This
change applies to the sentence structure discussing how ">-2" should evaluate in
relation to "<2".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b7d0a92-992f-4cba-903d-d2b818608a66
📒 Files selected for processing (16)
package.jsonpackages/library/package.jsonpackages/tempo/CHANGELOG.mdpackages/tempo/doc/releases/v3.x.mdpackages/tempo/doc/tempo.cookbook.mdpackages/tempo/package.jsonpackages/tempo/plan/licensing_strategy.mdpackages/tempo/plan/localized_modifiers.mdpackages/tempo/plan/slick-syntax-duration-keys.mdpackages/tempo/src/engine/engine.term.tspackages/tempo/src/module/module.mutate.tspackages/tempo/src/support/support.default.tspackages/tempo/src/support/support.symbol.tspackages/tempo/src/tempo.type.tspackages/tempo/src/tempo.version.tspackages/tempo/test/instance/instance.set.test.ts
💤 Files with no reviewable changes (2)
- packages/tempo/plan/licensing_strategy.md
- packages/tempo/plan/localized_modifiers.md
Summary by CodeRabbit
New Features
Documentation