[WIP] 'until' template and improved 'starting' template#6927
[WIP] 'until' template and improved 'starting' template#6927Gylfkxjyjdll wants to merge 22 commits intoactualbudget:masterfrom
Conversation
…the 'starting' templates. This allows more controll over the templates. - Introduced 'until' field in template syntax to specify when a template should stop being applied. - Updated parsing and un-parsing logic to handle 'until' in various template types. - Implemented logic to skip expired templates based on the current month and 'until' date.
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hello contributor! We would love to review your PR! Before we can do that, please make sure:
We do this to reduce the TOIL the core contributor team has to go through for each PR and to allow for speedy reviews and merges. For more information, please see our Contributing Guide. |
✅ Deploy Preview for actualbudget-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…the 'starting' templates. This allows more controll over the templates. Introduced 'until' field in template syntax to specify when a template should stop being applied. Updated parsing and un-parsing logic to handle 'until' in various template types. Implemented logic to skip expired templates based on the current month and 'until' date.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional Changes
Sequence Diagram(s)sequenceDiagram
participant Parser
participant Context
participant Store
participant Unparser
Parser->>Context: parse templates (include starting/until)
Context->>Context: isTemplateExpired(template, currentMonth)?
alt not expired
Context->>Store: add template to templates/remainder/goals
else expired
Context-->>Store: skip template
end
Store->>Unparser: request unparse of stored templates
Unparser-->>Store: return textual templates including "starting" / "until"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/loot-core/src/server/budget/category-template-context.ts (1)
331-355:⚠️ Potential issue | 🟠 MajorExpired templates are still passed to
checkLimitand other validators.The constructor filters expired templates out of
this.templates,this.remainder, andthis.goals(lines 333–336), but then passes the original unfilteredtemplatesarray tocheckLimit(templates)at line 353. This means an expiredlimitorremaindertemplate could still setthis.limitCheck = trueand enforce a spending cap even though the template is logically expired. The same applies tocheckByAndScheduleAndSpend(line 75 ininit), which validates the unfiltered list.Consider filtering expired templates before passing them to
checkLimit, or passingthis.templates(post-filter) instead:Proposed fix
+ const activeTemplates = templates + ? templates.filter(t => !CategoryTemplateContext.isTemplateExpired(t, month)) + : []; // sort the template lines into regular template, goals, and remainder templates - if (templates) { - templates.forEach(t => { - // Skip expired templates (where month > until) - if (CategoryTemplateContext.isTemplateExpired(t, month)) { - return; - } + if (activeTemplates.length) { + activeTemplates.forEach(t => { if ( t.directive === 'template' && t.type !== 'remainder' && @@ .. }); } - this.checkLimit(templates); + this.checkLimit(activeTemplates); this.checkSpend(); this.checkGoal();And similarly in
init, passactiveTemplatestocheckByAndScheduleAndSpend.
🤖 Fix all issues with AI agents
In `@packages/loot-core/src/server/budget/goal-template.pegjs`:
- Around line 4-5: The PEG grammar's rawScheduleName and name rules are
currently too greedy and can absorb the "starting" or "until" keywords into the
name/category (e.g., in the percentage branch returned by the template
production), so update both rules to add negative lookaheads that prevent
matching when the next token is the reserved keywords "starting" or "until";
specifically modify the rawScheduleName rule and the name rule used in the
percentage branch so they assert (via negative lookahead) that the upcoming text
is not "starting" or "until" before consuming characters, ensuring starting: and
until: productions are matched correctly in functions/productions like template
and the percentage branch.
🧹 Nitpick comments (1)
packages/loot-core/src/server/budget/template-notes.ts (1)
170-175: Consider extracting the repeatedstarting/untilsuffix pattern.The same 4-line block (
if (template.starting) ... if (template.until) ...) is duplicated 8 times across all template branches. A small helper would reduce repetition and keep the branches focused on template-specific logic.♻️ Example helper extraction
+function appendDateBounds( + result: string, + template: { starting?: string; until?: string }, +): string { + if (template.starting) { + result += ` starting ${template.starting}`; + } + if (template.until) { + result += ` until ${template.until}`; + } + return result; +}Then each branch simplifies to:
- if (template.starting) { - result += ` starting ${template.starting}`; - } - if (template.until) { - result += ` until ${template.until}`; - } - return result.trim(); + return appendDateBounds(result, template).trim();Also applies to: 192-197, 204-210, 219-221, 238-243, 255-260, 275-280, 286-291
|
🤖 Auto-generated Release Notes Hey @Gylfkxjyjdll! I've automatically created a release notes file based on CodeRabbit's analysis: Category: Features If you're happy with this release note, you can add it to your pull request. If not, you'll need to add your own before a maintainer can review your change. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@upcoming-release-notes/6927.md`:
- Line 6: Replace the verbose, technical release note with a single, user-facing
sentence that summarizes the benefit: mention that templates now support an
"until" date and that template start behavior is more flexible (refer to the
'until' and 'starting' features) and remove implementation details like "parsing
and un-parsing logic"; ensure the sentence is non-technical, concise, and fits
the upcoming-release-notes/*.md one-line rule for Enhancements.
…ules were too eager and would absorb 'starting' or 'until'. Added negative lookaheads to prevent this, ensuring correct parsing.
✅ Deploy Preview for actualbudget-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/loot-core/src/server/budget/goal-template.pegjs`:
- Around line 101-116: The rawScheduleName rule's negative lookaheads use `_`
(optional whitespace) so they wrongly trigger mid-word (e.g., "restarting");
update the lookaheads in rawScheduleName to require mandatory whitespace before
keywords (use `__` instead of `_`) and ensure the keyword is followed by either
whitespace or end-of-input (e.g., check `'starting'i`/`'until'i` are followed by
`__` or end via `(__ / !.)` or an equivalent lookahead), and apply the same
change for both the initial and continuation checks (the parts referencing `_
'starting'i`/`_ 'until'i` and the continuation that uses `[^\r\n]`) so schedule
names containing those substrings are not cut mid-word.
- Around line 118-125: The per-character negative lookaheads in the PEG rule
cause early stops inside words (e.g., "restarting"); update the lookaheads in
the name rule (and the rawScheduleName rules) to only trigger when the keyword
is preceded by whitespace by checking for a space/tab before the keyword in the
lookahead (i.e., replace !('starting'i _) and !('until'i _) with a negative
lookahead that looks for [ \t] (or explicit space) immediately before the
keyword plus the following separator, so the check only fires for standalone "
starting" / " until" and not mid-word occurrences.
This makes it very easy. thx bot =)). |
Removed visualizer configuration for web stats file.
Gylfkxjyjdll
changed the title
youngcw
left a comment
youngcw
left a comment
There was a problem hiding this comment.
I haven't done a thorough test yet, but it looks quite good so far. Thanks for not leaving a pile of AI spaghetti.
| @@ -442,6 +445,26 @@ export class CategoryTemplateContext { | |||
| }); | |||
| } | |||
|
|
|||
| private static isTemplateExpired(template: Template, month: string): boolean { | |||
There was a problem hiding this comment.
The name here probably should be something more general as expired sounds like you are only doing the "until" check.
There was a problem hiding this comment.
Done, name should be more specific now.
| @@ -331,6 +330,10 @@ export class CategoryTemplateContext { | |||
| // sort the template lines into regular template, goals, and remainder templates | |||
| if (templates) { | |||
| templates.forEach(t => { | |||
| // Skip expired templates (where month > until) | |||
| if (CategoryTemplateContext.isTemplateExpired(t, month)) { | |||
There was a problem hiding this comment.
This is simpler then I would have thought. I was worried this would have to touch a lot of things to work.
| = template: template _ percentOf:percentOf category: name starting: startingDate? until:until? | ||
| { return { type: 'percentage', percent: +percentOf.percent, previous: percentOf.prev, category, starting, until, priority: template.priority, directive: template.directive }} | ||
| / template: template _ amount: amount _ repeatEvery _ period: periodCount _ starting: startingDate limit: limit? until: until? | ||
| { return { type: 'periodic', amount, period, starting, limit, until, priority: template.priority, directive: template.directive }} |
There was a problem hiding this comment.
I was wondering if you would catch that there was already a start date here 🙂
|
Oh, this needs documentation before merging. |
|
@jfdoming Any thoughts? |
|
My apologies for the potentially confusing naming convention. The concept was initially 'until' but was later expanded to 'starting'. I had considered using 'ending' as an alternative. My coding skills are a bit rusty x) The utility of this feature lies in providing greater control over budgeting by allowing it to be "scripted", thereby enabling planning for the entire year. A future enhancement could involve a "budget carryover" template. |
| / template: template _ amount: amount _ by _ month: month from: spendFrom? repeat: (_ repeatEvery _ repeat)? | ||
| = template: template _ percentOf:percentOf category: name starting: startingDate? until:until? | ||
| { return { type: 'percentage', percent: +percentOf.percent, previous: percentOf.prev, category, starting, until, priority: template.priority, directive: template.directive }} | ||
| / template: template _ amount: amount _ repeatEvery _ period: periodCount _ starting: startingDate limit: limit? until: until? |
There was a problem hiding this comment.
This breaks existing functionality. Weekly and daily periodic templates need a specific day to start on. If there is a clean way to force the start date to have a day on weekly and daily versions then we are fine. If not, allowing YYYY-MM dates needs to be re-evaluated.
There was a problem hiding this comment.
Oh right... I didn't think about that. I need to think about this and look at it more closely.
The YYYY-MM format should be a convenience function that makes things intuitive when you have to write a lot. And the syntax "starting YYYY-MM until YYYY-MM" remains consistent.
There was a problem hiding this comment.
Maybe just the periodic template could require a day for the starting value. I would think the timeframe check would still work basically the same if the starting value has a day for those.
There was a problem hiding this comment.
in the repeat template:
// Wenn "repeat every week/day" und "starting" ist YYYY-MM = 7 Zeichen, ergänze -01
starting: (period.period === 'week' || period.period === 'day') && starting?.length === 7 ? starting + '-01' : starting,
But i have to think about, if this will work in all cases..
And if this is needed in the until template as well..
There was a problem hiding this comment.
I have to overthink this and will take some time. Because repeat every day starting 2026-01-01 until 2026-01-05 still calculates the whole month. didn't think about this.
There was a problem hiding this comment.
I think only allowing the until and starting values to be months makes sense for everything but the periodic template. If just the periodic template can be fixed then you should be good
|
VRT tests ❌ failed. View the test report. To update the VRT screenshots, comment |
Gylfkxjyjdll
changed the title
…ndition. So until works with runPeriodic without starting. need testing
|
tbh i had some trouble with the periodic starting and until - i had a very hard time. It needed a very, very specific syntax to work and was unintuitive. And if something is implemented, it should "always" work or giving feedback to the user what's wrong. but i have to think about the logic of the templates.. if this make sense =/ |
…sing in goal-template.pegjs
Usable on the Template 'repeat every day / week'
|
I used ChatGPT to optimize runPeriodic(). Had some trouble making the template usage consistent, but it should be working now. Feedback welcome! |
2 participants
Add 'until' feature to templates, skip expired templates & optimized the 'starting' template. This allows more control over the templates.
Bundle Stats
View detailed bundle stats
desktop-client
Total
Changeset
locale/ca.jsonlocale/en.jsonView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
loot-core
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/server/budget/goal-template.pegjshome/runner/work/actual/actual/packages/loot-core/src/server/budget/template-notes.tshome/runner/work/actual/actual/packages/loot-core/src/server/budget/category-template-context.tsView detailed bundle breakdown
Added
Removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged
api
Total
Changeset
src/server/budget/goal-template.pegjssrc/server/budget/template-notes.tssrc/server/budget/category-template-context.tsView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged