Repository navigation
Period engine — period keys, boundaries and due dates, timezone-correct #1
Description
Activity
Claim: PM loop round 1
Session:session_01SqkTcrxUFci7nqXdbBSe2p
Branch:claude/issue-1-period-engine
Worktree:duly-issue-1
Domain: n/a (single-package app repo; nodomain:*lanes in this repo)
File surface:src/functions/,test/period.test.ts(stop on breach; explain in the report)
Container & model: M,mode:subagent,model: opus
Clause-②: no — this repo authors application metadata; it does not touchpackages/specor any protocol contract surface.
Serial constraints cleared: batch 1 is #1 / #3 / #5 / #6, file surfaces disjoint by construction (src/functions/,src/hooks/,src/actions/,src/flows/). Every metadata barrel and theonEnablehandler-registration point are pre-wired onmain, so no task in this batch touchesobjectstack.config.ts.
Generated by Claude Code
{ "issue": 1, "status": "done", "branch": "claude/issue-1-period-engine", "pr": "https://github.com/objectstack-ai/duly/pull/25", "premise_still_valid": true, "files_changed": ["src/functions/period.ts (new)", "test/period.test.ts (new)"], "files_deliberately_untouched": [ "src/functions/index.ts — dulyFunctions is the map a `script` flow node resolves BY NAME; no flow exists in this repo yet and #2 imports the functions directly, so an entry would be dead metadata and a needless collision point for the three sibling branches in flight. The issue scoped this as 'only if a flow node needs to call one'.", "objectstack.config.ts — never touched; no reason to." ], "summary": "Implemented the seven-frequency period engine as pure functions in src/functions/period.ts: periodKeyFor, periodBounds, dueDateFor, visibleFromFor, periodsBetween, plus the exported Frequency type and FREQUENCIES vocabulary. The key spelling table is implemented exactly as adjudicated (2026-W04 padding included) and every key is length-checked against period_key's maxLength 16. All arithmetic runs on calendar parts in the supplied IANA zone and converts to an instant exactly once. Two things the implementation had to get right that the issue's suggested route does not by itself give you, both now measured: (1) the textbook two-pass wall-clock inversion does not merely round on America/Santiago 2026-09-06, it lands in the PREVIOUS local day (converges on 2026-09-06T03:00Z = 23:00 on the 5th), so the inversion verifies its answer against the zone and resolves forward on a gap, earliest-occurrence on a fold; (2) a civil-date walk in periodsBetween emitted 2011-12-30 for Pacific/Apia, a local day the zone skipped at the date line — a backfill would have filed a task for a day nobody lived through, so the walk now skips periods holding no instants and returns only keys periodKeyFor could itself return. All four gates green at 8946861. One thing wants a person before #2 and #7 consume it: duly_duty.due_offset_days's own description is self-inconsistent (see open_questions and #23); I shipped the reading the schema default forces and pinned it in tests, but both readings pass every acceptance example in this issue.", "pm_assumptions": [ { "assumption": "Intl.DateTimeFormat with a timeZone is sufficient, invertible without a date library", "verdict": "CONFIRMED — no dependency added, none needed", "detail": "Local parts come from formatToParts with hourCycle 'h23' (plain hour12:false still renders midnight as hour 24 under some ICU builds); the zone's offset at an instant is those parts read back as if UTC, minus the instant. Inversion is NOT a fixed-point iteration, though: candidates are drawn from the offsets one day either side of the target and then VERIFIED against the zone. The naive two-pass is wrong, not imprecise — ablation leg 1 below." }, { "assumption": "ISO week-year handling has to be written by hand", "verdict": "CONFIRMED — the platform ships no helper", "detail": "Searched all seven installed @objectstack packages (spec, runtime, cli, plugin-hono-server, connector-rest/openapi/mcp) for isoWeek/weekYear/startOfWeek/periodKey shapes: zero hits. The nearest thing is spec's date-range-presets / date-macros vocabulary (today, this_week, last_30_days …), which is a dashboard filter enum that lowers to macro tokens — no calendar arithmetic and no timezone inversion. Written by hand, ~30 lines, Thursday rule." }, { "assumption": "The fortnight anchoring rule is unambiguous", "verdict": "CONFIRMED unambiguous — but it has a forced consequence you should know about", "detail": "Only one reading survives 'ISO week 1 begins one': fortnights start on ODD ISO weeks, re-anchored each ISO week-year. The alternative (a 14-day cycle running continuously across the year boundary) would make W01 NOT begin a fortnight in the year after a 53-week year, contradicting the rule itself — so it is excluded, not chosen against. The consequence: in a 53-week ISO week-year the final fortnight is ONE week long (2020-W53 and 2026-W53, both in your acceptance list; next is 2032). Periods stay contiguous, keys still round-trip, periodsBetween has no gaps or duplicates. This is a determinate consequence rather than an ambiguity, so I implemented it — but it is visible in seed data and in #2's backfill, so it is flagged rather than buried, and both years have a dedicated test." } ], "tests": "All four gates run at HEAD 8946861 with a clean tree (git status --porcelain empty), each captured as `pnpm <gate> > log 2>&1; echo EXIT=$?` so the exit code is the gate's and not a pipe's. pnpm validate -> EXIT=0, '✓ Validation passed (232ms)' + 'Data: 5 Objects 57 Fields'. pnpm typecheck -> EXIT=0, tsc --noEmit, no diagnostics. pnpm test -> EXIT=0, 'Test Files 2 passed (2) / Tests 97 passed (97)' (89 new in test/period.test.ts, 8 pre-existing invariants still green, untouched). pnpm build -> EXIT=0, '✓ Build complete (227ms)' + 'Artifact: dist/objectstack.json (51.7 KB)'. CI (.github/workflows/ci.yml) runs these same four on Node 22; local Node was v22.22.2 with full ICU (418 zones). REVERSE VERIFICATION: three ablations, each driven by a script carrying `trap '<git checkout>' EXIT INT TERM`. No rebuild leg applies — vitest transforms src/ directly, there is no dist/ for a stale artifact to hide in, so the mutation reaches the code under test at import; the on-disk confirmation was still done every leg and is what makes each reading real: for each, the injected marker was grepped present (count 1), the removed anchor grepped absent (count 0), and git diff --stat shown non-empty BEFORE the run — an anchor miss would have been a silent no-op reading as a passing ablation. Results, all in the predicted direction (red): leg 1, inversion replaced with the naive unverified two-pass -> 2 failed (Santiago midnight-shift, skipped-day); leg 2, clamp removed from dueDateFor -> 5 failed (February leap pair + all four clamping tests); leg 3, fortnights always 14 days -> 3 failed (both 53-week tests + the fortnightly periodsBetween walk). Tree verified clean and marker-free afterwards (git status --porcelain empty, grep -c ABLATED = 0). Note for the maintainer: the DST tests assert real host tzdata facts (Berlin 2026-03-29/10-25, Santiago 2026-09-06, Apia 2011-12-30). All are current, stable rules; a future tzdata change to Chile's rules would correctly turn a test red rather than silently pass.", "open_questions": [ { "question": "What does due_offset_days mean, exactly? duly_duty's own field description contradicts itself: 'Days from the anchor. \"5\" with a monthly period anchored to period start = due on the 5th.' A monthly period starts on the 1st, so five days from the anchor is the 6th. #2 (dispatcher) and #7 (seed) both bake in whichever answer is live, and they differ by one day on every duty with a non-zero offset.", "options": [ "A — days from the anchor, 0 = the anchor day. period_start + 5 is the 6th; to land on the 5th you write 4. SHIPPED in this PR.", "B — the Nth day of the period, 1-based. period_start + 5 is the 5th, as the field's example says; needs a defined meaning for offset 0, which is the schema default." ], "recommendation": "A. Decisive evidence: defaultValue is 0 on both duly_duty and duly_catalog_item, and 0 has to mean something on BOTH anchors — under A it is the anchor day itself (period_start + 0 = first day, period_end + 0 = last day), while under B it is neither 1 nor -1 and every duty left on the default is undefined behaviour. period_end + 0 = the last day is also exactly the worked example repeated in duty.object.ts's header and docs/product/data-model.md ('a quarterly duty due in Q3 is due on 30 September'), and A is the reading under which 'Negative offsets count back from period end' is symmetric. Contrary evidence, stated so you can weigh it: the field's own '5 -> the 5th' parenthetical, and — weakly — this issue's phrase 'not 2 March', which is the unclamped result of 1 Feb + 29 days in a NON-leap February, i.e. B's arithmetic (under A the unclamped non-leap result is 3 March and 2 March is the leap-year result). Both readings satisfy every clamped example in this issue's acceptance criteria, so no gate can separate them. Filed as #23; if B wins it is a one-day shift in dueDateFor plus two tests, both named in that issue." } ], "out_of_scope_findings": [ "filed as #23: duly_duty.due_offset_days's description is self-inconsistent ('Days from the anchor' vs '\"5\" = due on the 5th'); the fix lives on src/objects/duty.object.ts, outside this card's file surface, and the choice changes dueDateFor — see open_questions", "filed as #24: duly_duty.timezone (and duly_catalog_item.timezone) accept any string, so a typo'd IANA zone validates clean, passes all four gates, and first throws inside the dispatcher at run time; the engine refuses an unknown zone deliberately rather than falling back to UTC, which is why an author-time validation is the missing half" ], "deviations": [ "periodsBetween treats BOTH endpoints as inclusive of the period containing them (a backfill must produce the period currently in flight, or it finishes one task short every run), and returns [] rather than throwing when to < from. The issue specified 'ascending, for backfill' without pinning the bounds; documented in the function's doc comment and pinned by tests.", "periodsBetween skips a period that holds no instants (a local day the zone erased at the date line). Not in the issue; adopted because the alternative files a task against a day that never happened. Costs one zone inversion per period, not two, because neighbouring periods share a boundary.", "periodBounds on such an erased day returns start === end (an empty window) rather than fabricating 24 hours, and the round-trip invariant is therefore stated as holding for REACHABLE keys — a key no instant can produce has no correct round-trip. Pinned by the Pacific/Apia 2011-12-30 test.", "Malformed period keys throw rather than being repaired, including a near-miss like 2026-W4 or an even-numbered fortnight key (whose message names the odd key you probably meant). duly_task is unique on (duty, owner, period_key), so two spellings of one period would be two tasks for one obligation with nothing downstream able to tell.", "Years are restricted to 1583-9999 so every key's year is exactly four digits, which is what the key spellings and maxLength 16 assume." ] }
Generated by Claude Code
os-dev-report
This supersedes the previous comment. That one opened with the HTML-comment form of the marker and GitHub's sanitizer stripped it on save, which makes it invisible to a scan keyed on the marker. Same report, marker as literal text, and two shell fragments below rewritten without angle brackets (the sanitizer ate those too —
pnpm <gate>and thetrapargument came out empty).{ "issue": 1, "status": "done", "branch": "claude/issue-1-period-engine", "pr": "https://github.com/objectstack-ai/duly/pull/25", "premise_still_valid": true, "files_changed": ["src/functions/period.ts (new)", "test/period.test.ts (new)"], "files_deliberately_untouched": [ "src/functions/index.ts — dulyFunctions is the map a `script` flow node resolves BY NAME; no flow exists in this repo yet and #2 imports the functions directly, so an entry would be dead metadata and a needless collision point for the three sibling branches in flight. The issue scoped this as 'only if a flow node needs to call one'.", "objectstack.config.ts — never touched; no reason to." ], "summary": "Implemented the seven-frequency period engine as pure functions in src/functions/period.ts: periodKeyFor, periodBounds, dueDateFor, visibleFromFor, periodsBetween, plus the exported Frequency type and FREQUENCIES vocabulary. The key spelling table is implemented exactly as adjudicated (2026-W04 padding included) and every key is length-checked against period_key's maxLength 16. All arithmetic runs on calendar parts in the supplied IANA zone and converts to an instant exactly once. Two things the implementation had to get right that the issue's suggested route does not by itself give you, both now measured: (1) the textbook two-pass wall-clock inversion does not merely round on America/Santiago 2026-09-06, it lands in the PREVIOUS local day (converges on 2026-09-06T03:00Z = 23:00 on the 5th), so the inversion verifies its answer against the zone and resolves forward on a gap, earliest-occurrence on a fold; (2) a civil-date walk in periodsBetween emitted 2011-12-30 for Pacific/Apia, a local day the zone skipped at the date line — a backfill would have filed a task for a day nobody lived through, so the walk now skips periods holding no instants and returns only keys periodKeyFor could itself return. All four gates green at 8946861. One thing wants a person before #2 and #7 consume it: duly_duty.due_offset_days's own description is self-inconsistent (see open_questions and #23); I shipped the reading the schema default forces and pinned it in tests, but both readings pass every acceptance example in this issue.", "pm_assumptions": [ { "assumption": "Intl.DateTimeFormat with a timeZone is sufficient, invertible without a date library", "verdict": "CONFIRMED — no dependency added, none needed", "detail": "Local parts come from formatToParts with hourCycle 'h23' (plain hour12:false still renders midnight as hour 24 under some ICU builds); the zone's offset at an instant is those parts read back as if UTC, minus the instant. Inversion is NOT a fixed-point iteration, though: candidates are drawn from the offsets one day either side of the target and then VERIFIED against the zone. The naive two-pass is wrong, not imprecise — ablation leg 1 below." }, { "assumption": "ISO week-year handling has to be written by hand", "verdict": "CONFIRMED — the platform ships no helper", "detail": "Searched all seven installed @objectstack packages (spec, runtime, cli, plugin-hono-server, connector-rest/openapi/mcp) for isoWeek/weekYear/startOfWeek/periodKey shapes: zero hits. The nearest thing is spec's date-range-presets / date-macros vocabulary (today, this_week, last_30_days …), which is a dashboard filter enum that lowers to macro tokens — no calendar arithmetic and no timezone inversion. Written by hand, ~30 lines, Thursday rule." }, { "assumption": "The fortnight anchoring rule is unambiguous", "verdict": "CONFIRMED unambiguous — but it has a forced consequence you should know about", "detail": "Only one reading survives 'ISO week 1 begins one': fortnights start on ODD ISO weeks, re-anchored each ISO week-year. The alternative (a 14-day cycle running continuously across the year boundary) would make W01 NOT begin a fortnight in the year after a 53-week year, contradicting the rule itself — so it is excluded, not chosen against. The consequence: in a 53-week ISO week-year the final fortnight is ONE week long (2020-W53 and 2026-W53, both in your acceptance list; next is 2032). Periods stay contiguous, keys still round-trip, periodsBetween has no gaps or duplicates. This is a determinate consequence rather than an ambiguity, so I implemented it — but it is visible in seed data and in #2's backfill, so it is flagged rather than buried, and both years have a dedicated test." } ], "tests": "All four gates run at HEAD 8946861 with a clean tree (git status --porcelain empty). Each was captured by redirecting to a log file first and reading the exit code straight afterwards, never through a pipe, so the code is the gate's and not tail's. pnpm validate -> EXIT=0, '✓ Validation passed (232ms)' + 'Data: 5 Objects 57 Fields'. pnpm typecheck -> EXIT=0, tsc --noEmit, no diagnostics. pnpm test -> EXIT=0, 'Test Files 2 passed (2) / Tests 97 passed (97)' (89 new in test/period.test.ts, 8 pre-existing invariants still green, untouched). pnpm build -> EXIT=0, '✓ Build complete (227ms)' + 'Artifact: dist/objectstack.json (51.7 KB)'. CI (.github/workflows/ci.yml) runs these same four on Node 22; local Node was v22.22.2 with full ICU (418 zones). REVERSE VERIFICATION: three ablations, each driven by a script whose trap on EXIT INT TERM restores the file via git checkout. No rebuild leg applies — vitest transforms src/ directly, there is no dist/ for a stale artifact to hide in, so the mutation reaches the code under test at import; the on-disk confirmation was still done every leg and is what makes each reading real: for each, the injected marker was grepped present (count 1), the removed anchor grepped absent (count 0), and git diff --stat shown non-empty BEFORE the run — an anchor miss would have been a silent no-op reading as a passing ablation. Results, all in the predicted direction (red): leg 1, inversion replaced with the naive unverified two-pass -> 2 failed (Santiago midnight-shift, skipped-day); leg 2, clamp removed from dueDateFor -> 5 failed (February leap pair + all four clamping tests); leg 3, fortnights always 14 days -> 3 failed (both 53-week tests + the fortnightly periodsBetween walk). Tree verified clean and marker-free afterwards (git status --porcelain empty, grep -c ABLATED = 0). Note for the maintainer: the DST tests assert real host tzdata facts (Berlin 2026-03-29 and 2026-10-25, Santiago 2026-09-06, Apia 2011-12-30). All are current, stable rules; a future tzdata change to Chile's rules would correctly turn a test red rather than silently pass.", "open_questions": [ { "question": "What does due_offset_days mean, exactly? duly_duty's own field description contradicts itself: 'Days from the anchor. \"5\" with a monthly period anchored to period start = due on the 5th.' A monthly period starts on the 1st, so five days from the anchor is the 6th. #2 (dispatcher) and #7 (seed) both bake in whichever answer is live, and they differ by one day on every duty with a non-zero offset.", "options": [ "A — days from the anchor, 0 = the anchor day. period_start + 5 is the 6th; to land on the 5th you write 4. SHIPPED in this PR.", "B — the Nth day of the period, 1-based. period_start + 5 is the 5th, as the field's example says; needs a defined meaning for offset 0, which is the schema default." ], "recommendation": "A. Decisive evidence: defaultValue is 0 on both duly_duty and duly_catalog_item, and 0 has to mean something on BOTH anchors — under A it is the anchor day itself (period_start + 0 = first day, period_end + 0 = last day), while under B it is neither 1 nor -1 and every duty left on the default is undefined behaviour. period_end + 0 = the last day is also exactly the worked example repeated in duty.object.ts's header and docs/product/data-model.md ('a quarterly duty due in Q3 is due on 30 September'), and A is the reading under which 'Negative offsets count back from period end' is symmetric. Contrary evidence, stated so you can weigh it: the field's own '5 gives the 5th' parenthetical, and — weakly — this issue's phrase 'not 2 March', which is the unclamped result of 1 Feb + 29 days in a NON-leap February, i.e. B's arithmetic (under A the unclamped non-leap result is 3 March and 2 March is the leap-year result). Both readings satisfy every clamped example in this issue's acceptance criteria, so no gate can separate them. Filed as #23; if B wins it is a one-day shift in dueDateFor plus two tests, both named in that issue." } ], "out_of_scope_findings": [ "filed as #23: duly_duty.due_offset_days's description is self-inconsistent ('Days from the anchor' vs '\"5\" = due on the 5th'); the fix lives on src/objects/duty.object.ts, outside this card's file surface, and the choice changes dueDateFor — see open_questions", "filed as #24: duly_duty.timezone (and duly_catalog_item.timezone) accept any string, so a typo'd IANA zone validates clean, passes all four gates, and first throws inside the dispatcher at run time; the engine refuses an unknown zone deliberately rather than falling back to UTC, which is why an author-time validation is the missing half" ], "deviations": [ "periodsBetween treats BOTH endpoints as inclusive of the period containing them (a backfill must produce the period currently in flight, or it finishes one task short every run), and returns [] rather than throwing when 'to' is earlier than 'from'. The issue specified 'ascending, for backfill' without pinning the bounds; documented in the function's doc comment and pinned by tests.", "periodsBetween skips a period that holds no instants (a local day the zone erased at the date line). Not in the issue; adopted because the alternative files a task against a day that never happened. Costs one zone inversion per period, not two, because neighbouring periods share a boundary.", "periodBounds on such an erased day returns start === end (an empty window) rather than fabricating 24 hours, and the round-trip invariant is therefore stated as holding for REACHABLE keys — a key no instant can produce has no correct round-trip. Pinned by the Pacific/Apia 2011-12-30 test.", "Malformed period keys throw rather than being repaired, including a near-miss like 2026-W4 or an even-numbered fortnight key (whose message names the odd key you probably meant). duly_task is unique on (duty, owner, period_key), so two spellings of one period would be two tasks for one obligation with nothing downstream able to tell.", "Years are restricted to 1583-9999 so every key's year is exactly four digits, which is what the key spellings and maxLength 16 assume." ] }
Generated by Claude Code
ACCEPT — PM review of #25, round 1.
Checklist: draft PR, base
main,Fixes #1on the first line, 2 files both inside the declared surface,objectstack.config.tsandsrc/objects/untouched. No changeset (correct for this repo). CIverifycompletedsuccesson8946861.Verified independently, not from the report. Branch fetched to a review worktree; all four gates re-run locally (validate / typecheck / test / build,
EXIT=0each, 97 tests). Then a PM probe against the module built from ground truths derived separately from the test file — the Thursday rule for ISO week-years, Chile's first-Sunday-of-September midnight shift, Berlin's two transitions — plus 1,960 randomised round-trip and containment checks across zones the PR's own tests do not cover:Asia/Kathmandu(+05:45),Australia/Lord_Howe(a 30-minute DST shift),Pacific/Chatham(+12:45/+13:45),Asia/Tehran. Zero failures.periodsBetweenverified unique and contiguous across the 2026→2027 boundary for all seven frequencies, and the malformed-key refusals confirmed.The two design calls both hold up:
- The verified inversion is the right shape. The probe independently reproduces the failure it exists for —
America/Santiago2026-09-06 resolves to2026-09-06T04:00Z, local 01:00 on the 6th, where the naive two-pass lands on the 5th. Getting the period wrong, not the hour. - Skipping periods that hold no instants is the correct reading of "for backfill". Filing a task against a day the zone erased is worse than the gap.
The 53-week fortnight consequence is called out rather than buried, which is the right handling — it will show up in #7's seed and #2's backfill and now nobody will be surprised by it.
On #23 — adjudicated, reading A stands. The measured behaviour is
period_start +0→ the first day,period_end +0→ the last day,quarterly Q3 period_end +0→ 30 September, which is exactly the worked example induty.object.tsanddocs/product/data-model.md. Decisive:defaultValue: 0has to mean something on both anchors, and only this reading gives it one. The self-contradictory field description is the defect — not the arithmetic — so #23 is re-scoped to fixing the description and label, and is queued ahead of the cards that consume the semantics.#24 is a real gap and goes to the queue: a typo'd IANA zone passing every author-time gate and first throwing inside the scheduled dispatcher is precisely the failure this product cannot afford — a duty that silently resolves in the wrong zone is a wrong due date nobody can see.
Merging. This unblocks #2 and #7.
Generated by Claude Code
- The verified inversion is the right shape. The probe independently reproduces the failure it exists for —
Everything downstream agrees on periods only if one module owns them. This is that module: pure functions, no I/O, no platform imports beyond types.
Files you own
src/functions/period.ts(new)src/functions/index.ts— add to thedulyFunctionsmap only if a flow node needs to call one; the dispatcher imports directlytest/period.test.ts(new)Do not touch
objectstack.config.ts— every barrel is already wired.API
Period key spelling — this is a contract, not a preference
YYYY-MM-DD2026-08-21YYYY-Www(ISO-8601 week)2026-W34YYYY-Wwwof the starting ISO week; fortnights anchored so ISO week 1 begins one2026-W33YYYY-MM2026-08YYYY-Qn2026-Q3YYYY-Hn2026-H2YYYY2026duly_task.period_keyismaxLength: 16. Keys must round-trip:periodKeyFor(f, periodBounds(f, k, tz).start, tz) === k.Rules that will bite you
Intl.DateTimeFormatwithtimeZoneto get local calendar parts; do not do naive UTC arithmetic and add hours.America/Santiago) shift at midnight, so "local midnight" may not exist — resolve forward to the first valid instant.2026-01-01is in ISO week2026-W01, but2027-01-01falls in2026-W53. The year in aYYYY-Wwwkey is the ISO week-year.due_anchor: 'period_start', due_offset_days: 30on February resolves to the 28th (29th in a leap year), not 2 March. Negative offsets fromperiod_endclamp atperiod_start.dueDateForreturns a calendar day string, becauseduly_duty/duly_taskstoredate, notdatetime.Acceptance
test/period.test.ts, table-driven, covering at minimum:UTC,Europe/Berlin,Asia/Shanghai)period_endoffsetsEurope/Berlin2026-03-29,America/Santiago2026-09-06 (midnight shift)periodsBetweenover a year boundary for each frequency, ascending, no gaps, no duplicatesGates
pnpm validate && pnpm typecheck && pnpm test && pnpm buildall green before the draft PR.