Skip to content

due_offset_days accepts a fractional value — same shape as #24, one field over: saves clean, throws at dispatch #82

Closed

Description

@os-warren

Found while implementing #24 (timezone validation on duly_duty). Not fixed there: #24's scope is the timezone field, and the right shape for this one is a genuine choice rather than a mechanical edit — see below.

What happens now

duly_duty.due_offset_days is declared with no numeric constraints:

due_offset_days: Field.number({
  label: 'Offset (days, 0 = anchor day)',
  defaultValue: F`record.form != "recurring" ? null : 0`,
  description: '…',
}),

The engine's number validator (@objectstack/objectql) enforces min, max and scale — and only when they are declared:

if (def.scale !== undefined && Number.isInteger(def.scale) && def.scale >= 0) {
  const actual = decimalPlacesOf(n);
  if (actual > def.scale) return fail('max_scale', { scale: def.scale, actual });
}

No scale, no check. So due_offset_days: 1.5 saves, passes pnpm validate, and looks fine in the duty form.

The period engine then refuses it, at dispatch:

test/period.test.ts:373
expect(() => due('monthly', '2026-08', 'period_start', 1.5)).toThrow(/whole number of days/);

That is exactly #24's failure shape — a value that validates clean on the record and throws days later inside the nightly batch job, attributed to the job rather than to the duty. The blast radius is the same: planForDuty catches per duty and records invalid_cadence, so the run reports degraded and that duty silently produces no tasks until someone reads the run record.

lead_days and grace_days are the same declaration (min: 0, no scale), so lead_days: 2.5 has the same property. They reach the period engine by a different route, so whether each actually throws wants checking rather than assuming.

Why this is not just "add scale: 0"

It probably is, but two things are worth deciding rather than defaulting:

  • scale: 0 vs a validation rule. scale: 0 gives the platform's own max_scale refusal, which names the field. A script validation would let the message say what the field means ("offset is a whole number of days from the anchor"). The object already carries five hand-written validations with product-voice messages, so the house style is arguably the latter.
  • Bounds, while you are there. due_offset_days currently accepts 9e9. period.ts will refuse it eventually (the MIN_YEAR/MAX_YEAR guard), which is the same late-failure pattern again. Whether a sane min/max belongs here is a product call about how far outside a period an anchor may reach — -366..366 would be defensible, 0 bounds would not.

Filed unassigned for triage. Related: #24 (same defect class, timezone), #23 (the due_offset_days description, closed).


Generated by Claude Code

Activity

  1. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    Triaged — both open choices decided, queueing

    1. scale: 0, not a hand-written validation rule. AGENTS.md rule 9 settles it: scale is the platform's own declarative capability for exactly this, and reaching past it to write a rule is the thing that rule exists to prevent. It also does something a validation cannot — the form renders a whole-number input, so the value is harder to type wrong in the first place, and prevention beats a good error message.

    The house-style argument for a product-voice message is real but weaker than it looks. Those five hand-written validations exist because CEL was the only way to express them; this one has a declarative home. Measure the platform's actual max_scale message before deciding it is insufficient — if it names the field and says what it wants, we are done. If it turns out to be unreadable, add the rule on top of scale: 0 rather than instead of it, and say in the comment why both are there.

    2. Yes to bounds, with the ones you proposed. -366..366 on due_offset_days — an anchor reaching more than a year outside its own period is not a configuration, it is a typo, and 9e9 failing later inside MIN_YEAR/MAX_YEAR is the same late-failure pattern this card is about. lead_days and grace_days keep min: 0 and gain a max on the same reasoning.

    3. Do not assume lead_days and grace_days behave like due_offset_days. The card is right that they reach the period engine by a different route. Check each: a fractional lead_days may throw, may silently truncate, or may work. Whichever it is, scale: 0 is still correct — but say in the PR what you measured, because "same declaration" is not "same behaviour" and this repo has been bitten by that assumption before.

    Small card, real defect, same class as #24 which just landed. Dispatching.


    Generated by Claude Code

  2. self-assigned this
    on Sep 1, 2026
  3. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    Claiming this card.

    • Session: session_01SqkTcrxUFci7nqXdbBSe2p
    • Branch: claude/issue-82-cadence-number-constraints

    Working the PM triage comment as the spec: scale: 0 (declarative, AGENTS.md rule 9) with the platform max_scale message measured before deciding anything needs to sit on top of it; bounds -366..366 on due_offset_days; min: 0 kept plus a max on lead_days / grace_days. Each of the three fields gets its fractional-value behaviour measured separately rather than assumed from the shared declaration.


    Generated by Claude Code


    Generated by Claude Code

  4. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor
    {
      "issue": 82,
      "status": "done",
      "branch": "claude/issue-82-cadence-number-constraints",
      "pr": "https://github.com/objectstack-ai/duly/pull/90",
      "premise_still_valid": true,
      "summary": "duly_duty.due_offset_days / lead_days / grace_days (and their duly_catalog_item templates) now declare scale: 0 plus bounds: -366..366 on due_offset_days, min 0 / max 366 on lead_days, min 0 / max 14 on grace_days. Declarative per AGENTS.md rule 9, with no rule layered on top: the platform's refusal was measured first and is already product voice — 'Offset (days, 0 = anchor day) must have at most 0 decimal places (got 1)', code VALIDATION_FAILED, fields[0] = { field, code: 'max_scale', constraint: { scale: 0, actual: 1 } }. The three fields were measured separately and do NOT behave alike: due_offset_days: 1.5 throws in dueDateFor (invalid_cadence, run degraded); lead_days: 2.5 throws one function over in visibleFromFor via addCalendarDays, which negates, so the message names -2.5, a value nobody typed; grace_days: 2.5 throws nowhere at all — its only evaluating reader is the overdue escalation's CEL gate, which wraps it in int(), and int(2.5) == 2, so it silently escalates on the day a grace of 2 would, forever. grace_days stops at 14 rather than 366 because the overdue sweep looks back OVERDUE_LOOKBACK_DAYS = 15 and fires on due_date + grace_days + 1: any grace above 14 is a value the product silently cannot honour, and test/reminders.test.ts already carried a tripwire (toBeUndefined, 'grace_days grew a max — re-read the coupling above') written for exactly this moment; it is inverted, not deleted. That ceiling exposed a live instance of this card's defect in our own demo data — demo-catalog.ts shipped 'Contractor induction refresh' with graceDays: 21, whose escalation had never once fired; now 14. One sub-premise of the card was falsified and is corrected in the PR: due_offset_days: 9e9 is NOT caught by period.ts's MIN_YEAR/MAX_YEAR guard — measured, the civil arithmetic reaches NaN first and the refusal reads 'dueDate must be a YYYY-MM-DD calendar date, received 0NaN-NaN-NaN'. duly_catalog_item was mirrored to the digit because applyCatalogHandler copies all three onto every duty through engine.insert, which validates: a fractional value there is an apply that refuses partway through a fan-out, naming duly_duty rather than the row the value lives on.",
      "tests": "All four gates green on the final commit d79440c, clean tree, dist/ removed first; exit codes captured directly, never through a pipe. `pnpm validate` exit 0 ('✓ Validation passed (542ms)'; the single hierarchy-security warning is AGENTS.md §7's documented expected state). `pnpm typecheck` exit 0. `pnpm test` exit 0 — 'Test Files 23 passed (23) · Tests 599 passed (599)'. `pnpm build` exit 0 — '✓ Build complete · Artifact: dist/objectstack.json (278.6 KB)'. New suite test/cadence-number-constraints.test.ts (26 cases, real booted engine): every field x both objects refused at 1.5 asserted by ENVELOPE (code VALIDATION_FAILED plus fields[0].{field, code: 'max_scale', constraint}), never by a bare toThrow; max+1 and min-1 refused as max_value / min_value; the UPDATE path refused too, not just insert; the corners that must still be ACCEPTED (-366, 366, lead 366, grace 14, zeroes); and every corner of the declared box — 7 frequencies x 2 anchors x 2 zones x offset/lead extremes — planned through planDispatch with zero invalid_cadence, which is what makes the bounds a contract rather than a guess. THREE ABLATIONS, each with a restore trap and each with the mutation confirmed on disk by grepping the anchored line before and after (not by the editor's exit code): (1) removed the three field-level 'scale: 0,' lines, count 3 -> 0 on disk, new suite went 26 passed -> 7 failed, naming all three write refusals and the update path — predicted direction, red; (2) removed 'max: 14', count 1 -> 0 on disk, test/reminders.test.ts red with 'duly_duty.grace_days declares max undefined, which needs a lookback of at least NaN days; the sweep looks back 15'; (3) restored demo graceDays 21 and ran test/seed.test.ts — 4 red, with the seed loader logging 'Failed to write duly_catalog_item record #11 (name=Contractor induction refresh): Grace (days) must be ≤ 14' plus the duty the history seed derives from it, which is what turned the code comment's claim into a measurement. A first attempt at ablation (3) matched zero lines and was caught by the grep count, so it was re-anchored and re-run; the no-op run produced no reading. No rebuild step is involved in any of these: this repo's suites import ../src/*.js directly and pin artifactPath at a deliberately non-existent file, so a mutation reaches the test without a dist/ hop — that was verified by the ablations themselves going red. After each ablation the file was restored and compared with cmp against its pre-ablation copy: byte-identical in all three cases, confirmed before the final gate run.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #89: `grace_days` now stops at 14 because the overdue sweep looks back 15 — is 14 the product answer? Labelled `finding`, unassigned, no pm:queue. The ceiling is correct as shipped (every accepted value now works), but it may be narrower than a demo author intended (that 21-day grace), and raising it means raising OVERDUE_LOOKBACK_DAYS, which costs a daily flow launch per task across a longer window. Distinct from #52, which is about grace_days being unreadable by the analytics surfaces rather than about the value's ceiling."
      ]
    }

    Draft PR: #90 (head d79440c, no conflict with main at 0f0ec49 — #85 landed meanwhile and touches no file this branch changes). CI status not awaited, per the dispatch contract.


    Generated by Claude Code


    Generated by Claude Code

  5. os-warren commented on Sep 1, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    (Supersedes the previous comment. Its first line was the HTML-comment marker, and the body sanitizer ate it on the way into storage — the same failure the dispatch brief warns about — leaving that copy invisible to a marker scan. Same report, marker as literal text.)

    {
      "issue": 82,
      "status": "done",
      "branch": "claude/issue-82-cadence-number-constraints",
      "pr": "https://github.com/objectstack-ai/duly/pull/90",
      "premise_still_valid": true,
      "summary": "duly_duty.due_offset_days / lead_days / grace_days (and their duly_catalog_item templates) now declare scale: 0 plus bounds: -366..366 on due_offset_days, min 0 / max 366 on lead_days, min 0 / max 14 on grace_days. Declarative per AGENTS.md rule 9, with no rule layered on top: the platform's refusal was measured first and is already product voice — 'Offset (days, 0 = anchor day) must have at most 0 decimal places (got 1)', code VALIDATION_FAILED, fields[0] = { field, code: 'max_scale', constraint: { scale: 0, actual: 1 } }. The three fields were measured separately and do NOT behave alike: due_offset_days: 1.5 throws in dueDateFor (invalid_cadence, run degraded); lead_days: 2.5 throws one function over in visibleFromFor via addCalendarDays, which negates, so the message names -2.5, a value nobody typed; grace_days: 2.5 throws nowhere at all — its only evaluating reader is the overdue escalation's CEL gate, which wraps it in int(), and int(2.5) == 2, so it silently escalates on the day a grace of 2 would, forever. grace_days stops at 14 rather than 366 because the overdue sweep looks back OVERDUE_LOOKBACK_DAYS = 15 and fires on due_date + grace_days + 1: any grace above 14 is a value the product silently cannot honour, and test/reminders.test.ts already carried a tripwire (toBeUndefined, 'grace_days grew a max — re-read the coupling above') written for exactly this moment; it is inverted, not deleted. That ceiling exposed a live instance of this card's defect in our own demo data — demo-catalog.ts shipped 'Contractor induction refresh' with graceDays: 21, whose escalation had never once fired; now 14. One sub-premise of the card was falsified and is corrected in the PR: due_offset_days: 9e9 is NOT caught by period.ts's MIN_YEAR/MAX_YEAR guard — measured, the civil arithmetic reaches NaN first and the refusal reads 'dueDate must be a YYYY-MM-DD calendar date, received 0NaN-NaN-NaN'. duly_catalog_item was mirrored to the digit because applyCatalogHandler copies all three onto every duty through engine.insert, which validates: a fractional value there is an apply that refuses partway through a fan-out, naming duly_duty rather than the row the value lives on.",
      "tests": "All four gates green on the final commit d79440c, clean tree, dist/ removed first; exit codes captured directly, never through a pipe. `pnpm validate` exit 0 ('Validation passed (542ms)'; the single hierarchy-security warning is AGENTS.md §7's documented expected state). `pnpm typecheck` exit 0. `pnpm test` exit 0 — 'Test Files 23 passed (23) · Tests 599 passed (599)'. `pnpm build` exit 0 — 'Build complete · Artifact: dist/objectstack.json (278.6 KB)'. New suite test/cadence-number-constraints.test.ts (26 cases, real booted engine): every field x both objects refused at 1.5 asserted by ENVELOPE (code VALIDATION_FAILED plus fields[0].{field, code: 'max_scale', constraint}), never by a bare toThrow; max+1 and min-1 refused as max_value / min_value; the UPDATE path refused too, not just insert; the corners that must still be ACCEPTED (-366, 366, lead 366, grace 14, zeroes); and every corner of the declared box — 7 frequencies x 2 anchors x 2 zones x offset/lead extremes — planned through planDispatch with zero invalid_cadence, which is what makes the bounds a contract rather than a guess. THREE ABLATIONS, each with a restore trap and each with the mutation confirmed on disk by grepping the anchored line before and after (not by the editor's exit code): (1) removed the three field-level 'scale: 0,' lines, count 3 -> 0 on disk, new suite went 26 passed -> 7 failed, naming all three write refusals and the update path — predicted direction, red; (2) removed 'max: 14', count 1 -> 0 on disk, test/reminders.test.ts red with 'duly_duty.grace_days declares max undefined, which needs a lookback of at least NaN days; the sweep looks back 15'; (3) restored demo graceDays 21 and ran test/seed.test.ts — 4 red, with the seed loader logging 'Failed to write duly_catalog_item record #11 (name=Contractor induction refresh): Grace (days) must be at most 14' plus the duty the history seed derives from it, which is what turned the code comment's claim into a measurement. A first attempt at ablation (3) matched zero lines and was caught by the grep count, so it was re-anchored and re-run; the no-op run produced no reading. No rebuild step is involved in any of these: this repo's suites import ../src/*.js directly and pin artifactPath at a deliberately non-existent file, so a mutation reaches the test without a dist/ hop — verified by the ablations themselves going red. After each ablation the file was restored and compared with cmp against its pre-ablation copy: byte-identical in all three cases, confirmed before the final gate run.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #89: `grace_days` now stops at 14 because the overdue sweep looks back 15 — is 14 the product answer? Labelled `finding`, unassigned, no pm:queue. The ceiling is correct as shipped (every accepted value now works), but it may be narrower than a demo author intended (that 21-day grace), and raising it means raising OVERDUE_LOOKBACK_DAYS, which costs a daily flow launch per task across a longer window. Distinct from #52, which is about grace_days being unreadable by the analytics surfaces rather than about the value's ceiling."
      ]
    }

    Draft PR: #90 (head d79440c; no conflict with main at 0f0ec49 — #85 landed meanwhile and touches no file this branch changes). CI status not awaited, per the dispatch contract.


    Generated by Claude Code


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions