fix(awareness): correct waking cycle schedule#280
Merged
Conversation
📝 WalkthroughWalkthroughThe default awareness cron expression is changed from ChangesAwareness Schedule Default Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Contributor
✅ Recall Benchmark ResultsFull outputCommit: a76c873 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@specs/waking-cycle-spec.md`:
- Line 137: The comment on line 137 describing the default schedule as "every 4
hours during waking hours" does not match the actual cron expression default
shown on line 138, which specifies fixed times at 8, 12, 16, and 20. Update the
comment text to accurately reflect that the default uses fixed-hour scheduling
at those four specific times rather than describing a repeating 4-hour interval.
In `@src/awareness/config/awareness.config.spec.ts`:
- Line 15: The test name 'defaults to every 4 hours, not every 4 minutes' is
misleading because the test validates fixed run times at specific hours
[8,12,16,20], not a repeating every-4-hours pattern. Rename the test to
accurately reflect that it defaults to fixed-run times at those specific hours
rather than using the misleading "every 4 hours" language that suggests a
recurring interval.
- Line 18: The destructured require statement for DEFAULT_AWARENESS_SCHEDULE and
AwarenessConfig violates Prettier formatting rules by being on a single line.
Reformat the destructuring to be multiline by placing the opening brace and each
destructured variable on separate lines, with the closing brace on its own line
before the require function call. This will comply with Prettier's line length
and formatting standards.
- Around line 35-40: The test helper nextRunHours uses getHours() which returns
the hour in the local timezone of the test runner, causing timezone-dependent
test failures in CI environments. Change the return statement from returning
next.getHours() to returning next.getUTCHours() to ensure the function
consistently returns hours in UTC, making the test timezone-independent and
stable across all environments.
In `@src/awareness/config/awareness.config.ts`:
- Line 15: Update the doc string for the default schedule configuration in the
awareness.config.ts file. The current comment states "every 4 hours during
waking hours" but this does not match the actual default cron expression which
is fixed at specific times (08:00, 12:00, 16:00, 20:00). Replace the comment
describing the schedule frequency to accurately reflect that the default
schedule runs at these four specific fixed times throughout the day rather than
at 4-hour intervals.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 214214bc-d869-4ac2-bf34-d8df1e9ed7ef
📒 Files selected for processing (4)
specs/waking-cycle-spec.mdsrc/awareness/config/awareness.config.spec.tssrc/awareness/config/awareness.config.tssrc/awareness/waking-cycle.service.ts
|
|
||
| # Schedule (cron expression, default every 4 hours during waking hours) | ||
| AWARENESS_SCHEDULE=0 */4 8-23 * * * | ||
| # Schedule (six-field cron expression, default every 4 hours during waking hours) |
There was a problem hiding this comment.
Update the comment text to match the fixed-hour default shown on the next line.
Line 137 still describes an every-4-hours schedule, but Line 138 is fixed times (8,12,16,20).
🤖 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 `@specs/waking-cycle-spec.md` at line 137, The comment on line 137 describing
the default schedule as "every 4 hours during waking hours" does not match the
actual cron expression default shown on line 138, which specifies fixed times at
8, 12, 16, and 20. Update the comment text to accurately reflect that the
default uses fixed-hour scheduling at those four specific times rather than
describing a repeating 4-hour interval.
| } | ||
| }); | ||
|
|
||
| it('defaults to every 4 hours, not every 4 minutes', () => { |
There was a problem hiding this comment.
Rename the test to reflect fixed-run times instead of “every 4 hours.”
The assertion validates fixed hours [8,12,16,20], so the title should match that behavior.
🤖 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 `@src/awareness/config/awareness.config.spec.ts` at line 15, The test name
'defaults to every 4 hours, not every 4 minutes' is misleading because the test
validates fixed run times at specific hours [8,12,16,20], not a repeating
every-4-hours pattern. Rename the test to accurately reflect that it defaults to
fixed-run times at those specific hours rather than using the misleading "every
4 hours" language that suggests a recurring interval.
| it('defaults to every 4 hours, not every 4 minutes', () => { | ||
| delete process.env.AWARENESS_SCHEDULE; | ||
|
|
||
| const { DEFAULT_AWARENESS_SCHEDULE, AwarenessConfig } = require('./awareness.config'); |
There was a problem hiding this comment.
Fix the Prettier violation in the destructured require.
This line is flagged by ESLint/Prettier and should be multiline-formatted.
Suggested patch
- const { DEFAULT_AWARENESS_SCHEDULE, AwarenessConfig } = require('./awareness.config');
+ const {
+ DEFAULT_AWARENESS_SCHEDULE,
+ AwarenessConfig,
+ } = require('./awareness.config');📝 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.
Suggested change
| const { DEFAULT_AWARENESS_SCHEDULE, AwarenessConfig } = require('./awareness.config'); | |
| const { | |
| DEFAULT_AWARENESS_SCHEDULE, | |
| AwarenessConfig, | |
| } = require('./awareness.config'); |
🧰 Tools
🪛 ESLint
[error] 18-18: Replace ·DEFAULT_AWARENESS_SCHEDULE,·AwarenessConfig with ⏎······DEFAULT_AWARENESS_SCHEDULE,⏎······AwarenessConfig,⏎···
(prettier/prettier)
🤖 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 `@src/awareness/config/awareness.config.spec.ts` at line 18, The destructured
require statement for DEFAULT_AWARENESS_SCHEDULE and AwarenessConfig violates
Prettier formatting rules by being on a single line. Reformat the destructuring
to be multiline by placing the opening brace and each destructured variable on
separate lines, with the closing brace on its own line before the require
function call. This will comply with Prettier's line length and formatting
standards.
Source: Linters/SAST tools
Comment on lines
+35
to
+40
| let cursor = new Date('2026-06-20T07:59:59-07:00'); | ||
|
|
||
| return Array.from({ length: 4 }, () => { | ||
| const next = cron.getNextDateFrom(cursor).toJSDate(); | ||
| cursor = new Date(next.getTime() + 1000); | ||
| return next.getHours(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For cron version 4.4.0, what is the exact CronTime constructor signature, and what is the recommended way to force UTC when using getNextDateFrom?
💡 Result:
In cron version 4.4.0, the CronTime constructor has the following signature [1][2][3]: constructor(time: string | Date, zone?: string, utcOffset?: number) The parameters are: - time: [REQUIRED] The cron expression string or a JavaScript Date object [1][3]. - zone: [OPTIONAL] A timezone string (e.g., 'America/Los_Angeles'). This cannot be used simultaneously with utcOffset [1][4]. - utcOffset: [OPTIONAL] A numeric UTC offset in minutes [1][4]. To force UTC when using getNextDateFrom, the recommended approach is to pass the 'UTC' zone string as the second argument to the method [5][4]. The method signature for getNextDateFrom is defined as [4]: getNextDateFrom(start: Date | DateTime, timeZone?: string): DateTime When you provide 'UTC' as the timeZone argument, the library ensures the calculation is performed within the UTC context [5][4]. Example usage: const cronTime = new CronTime('0 0 * * *'); const nextDate = cronTime.getNextDateFrom(new Date, 'UTC');
Citations:
- 1: https://www.npmjs.com/package/cron
- 2: https://cdn.jsdelivr.net/npm/cron@4.4.0/README.md
- 3: https://github.com/kelektiv/node-cron/
- 4: https://tessl.io/registry/tessl/npm-cron/4.3.0/files/docs/time-calculations.md
- 5: https://github.com/kelektiv/node-cron/blob/main/src/time.ts
🏁 Script executed:
find . -name "awareness.config.spec.ts" -type fRepository: heybeaux/engram
Length of output: 107
🏁 Script executed:
head -50 ./src/awareness/config/awareness.config.spec.ts | tail -25Repository: heybeaux/engram
Length of output: 898
Fix timezone instability in nextRunHours to prevent flaky CI tests.
The test helper uses new Date('2026-06-20T07:59:59-07:00') and getHours(), which depend on the runner's local timezone. Expected hours [8, 12, 16, 20] will fail in CI environments with different timezone settings.
Apply this fix
function nextRunHours(schedule: string): number[] {
- const cron = new CronTime(schedule);
- let cursor = new Date('2026-06-20T07:59:59-07:00');
+ const cron = new CronTime(schedule, 'UTC');
+ let cursor = new Date('2026-06-20T07:59:59Z');
return Array.from({ length: 4 }, () => {
const next = cron.getNextDateFrom(cursor).toJSDate();
cursor = new Date(next.getTime() + 1000);
- return next.getHours();
+ return next.getUTCHours();
});
}📝 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.
Suggested change
| let cursor = new Date('2026-06-20T07:59:59-07:00'); | |
| return Array.from({ length: 4 }, () => { | |
| const next = cron.getNextDateFrom(cursor).toJSDate(); | |
| cursor = new Date(next.getTime() + 1000); | |
| return next.getHours(); | |
| function nextRunHours(schedule: string): number[] { | |
| const cron = new CronTime(schedule, 'UTC'); | |
| let cursor = new Date('2026-06-20T07:59:59Z'); | |
| return Array.from({ length: 4 }, () => { | |
| const next = cron.getNextDateFrom(cursor).toJSDate(); | |
| cursor = new Date(next.getTime() + 1000); | |
| return next.getUTCHours(); | |
| }); | |
| } |
🤖 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 `@src/awareness/config/awareness.config.spec.ts` around lines 35 - 40, The test
helper nextRunHours uses getHours() which returns the hour in the local timezone
of the test runner, causing timezone-dependent test failures in CI environments.
Change the return statement from returning next.getHours() to returning
next.getUTCHours() to ensure the function consistently returns hours in UTC,
making the test timezone-independent and stable across all environments.
| /** | ||
| * Cron expression for the Waking Cycle scheduler. | ||
| * Default: every 4 hours during waking hours (08:00–23:00). | ||
| * Default: every 4 hours during waking hours. |
There was a problem hiding this comment.
Update the default-schedule doc string to match the new fixed-hours cron.
Line 15 says the default is “every 4 hours,” but the exported default is fixed at 08:00,12:00,16:00,20:00. Please align the comment with the actual value.
Suggested patch
- * Default: every 4 hours during waking hours.
+ * Default: runs at 08:00, 12:00, 16:00, and 20:00.📝 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.
Suggested change
| * Default: every 4 hours during waking hours. | |
| * Default: runs at 08:00, 12:00, 16:00, and 20:00. |
🤖 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 `@src/awareness/config/awareness.config.ts` at line 15, Update the doc string
for the default schedule configuration in the awareness.config.ts file. The
current comment states "every 4 hours during waking hours" but this does not
match the actual default cron expression which is fixed at specific times
(08:00, 12:00, 16:00, 20:00). Replace the comment describing the schedule
frequency to accurately reflect that the default schedule runs at these four
specific fixed times throughout the day rather than at 4-hour intervals.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Production mitigation already applied
Summary by CodeRabbit
New Features
Tests
Chores