Skip to content

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

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

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