A hand-filled period_end walks around #1008's calendar-boundary fix — a row labelled Q3 2026 can still span ten months

Surfaced by the period_end measurement in #1085 / PR #1092. Not a defect in that PR — it documented period_end correctly. The hole is in #1008's fix, and it survives that fix.

The claim #1081 made

PR #1081 (#1008) refused a hand-filled crm_forecast.period_start off a calendar-period boundary, and deliberately left the hook's derivation alone. Its changeset and code comments state the reason:

The derivation is left alone: with the start pinned to a boundary, "start + one period" IS the calendar period, so there is nothing left for option 1 to fix.

That reasoning is sound only for writes that leave period_end unset, because that is the only case where the hook derives it.

What was measured in #1085

period_end is on the record form's Snapshot section (src/views/forecast.view.ts) and is not readonly.
forecast.hook.ts derives period_end only when the write leaves it unset.
The only validation bound to period_end is period_end_after_start (period_end <= period_start → refuse). The two rules added by fix(forecast): refuse a period_start off its calendar boundary (#1008) #1081 (period_start_first_of_period, quarter_starts_on_quarter_boundary) read period_start and period — neither constrains period_end's upper bound.

The consequence

period:       quarter
period_start: 2026-07-01     ← a valid quarter boundary, so #1081's rules admit it
period_end:   2027-05-15     ← hand-typed; only `period_end_after_start` applies, and it passes
period_label: "Q3 2026"      ← derived from the START

A row labelled Q3 2026 whose window runs ten months. That is exactly the internally-inconsistent shape #1008 was ruled to make structurally unwritable — reached through the other field.

It matters for the same reasons #1008 listed: this_quarter_forecasts and the quota-attainment widget pin period_start by equality, and the nightly forecast_snapshot sweep selects the current row with period_start <= today <= period_end. An over-long window makes one row answer to "current" for many months (and see #1082, where that same containment test already claims a manager's manual row).

Confidence, stated honestly

⚠️ This is inferred from the #1085 dev's measurement of the form, the hook and the rule set — it has not been driven end-to-end. Before fixing, reproduce it: write a forecast with a boundary-correct period_start and a hand-typed far-future period_end and confirm the write is admitted. If something else refuses it, this card is wrong and should be closed saying so.

Options, for whoever grades this

Make period_end readonly on the form and always derive it. Simplest, and matches the field's actual intent ("Normally derived automatically" is what fix(forecast): state period_start's calendar-boundary constraint in description + four-locale help #1092's own description now says). Cost: removes a capability someone may use for irregular periods — measure whether any seed or writer sets it by hand first.
Add a validation binding period_end to the calendar period of period_start. Keeps hand-entry, closes the hole. Consistent with fix(forecast): refuse a period_start off its calendar boundary (#1008) #1081's chosen instrument, and a type: 'script' rule is a true invariant (measured in Discount governance: ceiling validation + approval routing; one definition of "large deal" #599 / PR feat(quote): hard discount ceiling, and one definition of "large deal" (#599) #1088), so it would also refuse edits to a row already stored wrong.
Accept it and say so in the docs. Only defensible if hand-set irregular periods are a real requirement — and then period_label is the thing that lies, not period_end.

Recommendation: (1) if nothing writes it by hand, (2) if something does. That is a measurement, not a preference, and it should be taken before the card is scoped.

Refs #1008 · PR #1081 · #1085 · PR #1092 · #1082

Reproduced end-to-end. My "not driven end-to-end" caveat is withdrawn — this card is confirmed, and promoted to pm:queue.

I filed this card myself from another dev's measurement of the form and the rule set, and flagged plainly that I had not driven it through the engine, with the instruction that if something else refused the write the card was wrong and should be closed. The #1106 sweep ran that probe against a real ObjectQL + the real forecast_derive_period handler:

after-hook input : {"period":"quarter","period_start":"2026-07-01","period_end":"2027-05-15",
                    "snapshot_date":"2026-08-15","period_label":"Q3 2026"}
ADMITTED row     : {"period_start":"2026-07-01","period_end":"2027-05-15","period_label":"Q3 2026"}
control (unset)  : derives period_end 2026-09-30

The control is what makes this a measurement rather than a zero: with period_end left unset the hook derives the calendar boundary 2026-09-30, so the ten-month window under a Q3 2026 label is specifically what the hand-typed value buys. The defect is real.

The fix fork is now settled by measurement, not preference

This card offered "(1) make period_end readonly and always derive it if nothing writes it by hand, (2) add a validation binding it to period_start's calendar period if something does." That was written as a measurement to take, and the sweep took it:

src/data/revenue.seed.ts:339, 378, 393 set period_end by hand. ⇒ Option 1 would break the seeds. Option 2 is the answer, and a type: 'script' validation is a true invariant on this platform (measured in #599 / PR #1088), so it would also refuse edits to a row already stored wrong.

⇒ Whoever takes this should implement option 2 and cite the seed lines as the reason, rather than re-opening the fork.

⚠️ Note for the dispatch: this lands on src/objects/forecast.object.ts, which already carries the two rules from #1008 / PR #1081 and the descriptions from #1085 / PR #1092. The new rule should read as one coherent set with those, not as a third independently-authored gate on the same field family.

Refs #1106 · #1008 / PR #1081 · #1085 / PR #1092 · #599 / PR #1088

Generated by Claude Code

Claim: PM loop round 1 (hotcrm whole-repo seat)
Session: session_012caozke9UaxYdVLaudxJvv
Branch: claude/issue-1093-forecast-period-end-boundary
Worktree: hotcrm-issue-1093
File surface: src/objects/forecast.object.ts (validations block only) + one test file
Container & model: S/M, judgment card (validation instrument + coherence with an existing rule set), mode:subagent, model: opus
Serial constraints cleared: no open PR touches src/objects/forecast.object.ts (the 4 open PRs are all dependabot); the two in-flight sibling dispatches this round (#1086 → quote_line_item.object.ts, #853 → content/docs/**) are file-disjoint from this surface.

Also stripping the stale finding label: this card was promoted to pm:queue by the #1106 sweep but kept finding, which is the half-state the label discipline forbids — finding means "held, not dispatchable", and it was neither.

Scope is option 2, already settled by measurement in the comment above (the seeds at src/data/revenue.seed.ts:339,378,393 write period_end by hand, so option 1 would break them). Not re-opening that fork.

Generated by Claude Code

{
  "issue": 1093,
  "status": "done",
  "branch": "claude/issue-1093-forecast-period-end-boundary",
  "pr": "https://github.com/objectstack-ai/hotcrm/pull/1110",
  "premise_still_valid": true,
  "summary": "Implemented option 2 as ruled: added `period_end_matches_calendar_period`, a third `type: 'script'` error validation on `crm_forecast` that refuses a `period_end` which is not the last day of the calendar period `period_start` opens. Predicate is `daysBetween(period_end, addDays(addMonths(period_start, 1|3), -1)) != 0`, spelled out per period value. Also updated `period_end`'s description and its help text in all four locales, per #1085/#1092's convention that the field states what is enforced on it. The two #1081 rules, the hook's derivation and `revenue.seed.ts` are untouched. PM mechanism assumption 1 CONFIRMED with a caveat worth reading: CEL can express this, but not the way #1081's comment implies — that comment says the stdlib has no month/day accessor, which is still true and is why the two start rules use a regex; it blocks inspecting ONE date's components, not comparing TWO dates, and `addMonths`/`addDays`/`daysBetween` are all registered. `daysBetween(...) != 0` rather than `==` is load-bearing: drivers return `YYYY-MM-DD` strings while `addDays` returns a timestamp, so a direct `==` would be permanently green. Assumption 2 (seeds) CONFIRMED — all 7 seeded rows are calendar-aligned and admitted by the engine through the shipped rule; nothing in the seeds was edited. Assumption 3 CONFIRMED — the change is purely additive. NEW MEASUREMENT the PM should know about: the `!= null` guards are load-bearing for a hazard beyond the two AGENTS.md names — `daysBetween(null, …)` reaches `BigInt(NaN)` and throws, which the engine reports as `predicate failed to evaluate` (#4649, a rejected ordinary save). This predicate has no ordering operator, so `object-validation-predicates.test.ts`'s unguarded-comparison sweep passes it vacuously, and that sweep's totality run sets every field null at once, short-circuiting before `daysBetween`; only the mixed shape trips it, so the pin lives in the new test file.",
  "tests": "NEW test/forecast-period-end-boundary.test.ts, 41 tests, all green. GATE PROVEN ABLE TO FAIL two ways. (a) Live in-suite reverse verification: a schema clone with only this rule removed ADMITS the card's row and stores it as given (`period_label: 'Q3 2026'`, `period_end: '2027-05-15'`), and the two start rules are shown to admit it on their own. (b) Physically reverted src/objects/forecast.object.ts to origin/main via `git diff > patch` + `git checkout origin/main --` (never stash) and re-ran — 19 failed | 22 passed (41), in the predicted direction, the 'gate can fail' and seed suites staying green as predicted. Observed output: `× refuses the card's own row — Q3 2026 spanning ten months`; `× refuses 'quarter' '2026-07-01' .. '2027-05-15'`; `× refuses 'month' '2026-02-01' .. '2026-03-31'`; `AssertionError: the write was ADMITTED — no refusal to inspect: expected undefined to be defined`; `AssertionError: no validation named period_end_matches_calendar_period on crm_forecast`. Fix restored with `git apply` and re-verified green. LEGACY ROW covered per the instrument reasoning: a row inserted through an engine whose schema lacks the rule, then re-opened on the same store with the shipped schema, is REFUSED on an unrelated `quota` edit, and the repair (`period_end` → 2026-09-30) is admitted, after which the row is editable again. Both drivers: InMemory (sparse rows) and a real SqliteWasm database (column-complete). Every refusal asserted by envelope `code` + exact message, with the #4649 abort shape filtered out so no assertion can be satisfied by a broken rule. LOCAL GATES: `pnpm validate` → `✓ Validation passed (1394ms)` exit 0; `pnpm typecheck` → clean; `pnpm build` → `✓ Build complete`, exit 0; `pnpm hygiene` → `✓ source hygiene clean` (incl. control-byte scan), plus a manual `grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]'` over all new/edited files, no hits; full `pnpm test` → 98 files, 2405 passed | 1 skipped, including object-validation-predicates, i18n-references and all four pre-existing forecast suites. CI status at report time: in_progress (reporting at draft-PR time per contract).",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #1111: `content/docs/sales/forecasting.mdx:41` (+ zh-Hans/zh-Hant twins) still teaches that a mid-quarter `period_start` is stored as sent and that 'nothing snaps it back' — #1081 has refused exactly that write since 2026-08-11, and this PR makes the same sentence wrong for `period_end` too. Pre-existing drift from #1081, unassigned, left unlabeled for PM triage.",
    "not filed, reported here as a template-direction correction: my test's presumed direction for 'RE-LABEL a monthly row as quarterly' was WRONG and the real behaviour is correct. An update naming only `period` leaves `period_end` unset, so the hook re-derives it and the row lands calendar-true — ADMITTED, not refused. This is the derivation working, not a hole; the rule's case is an update that carries the stale `period_end` explicitly, which is what the record form submits. Both directions are now separate tests with the reasoning written at them. Note this differs from the sibling case in forecast-period-boundary.test.ts, which IS refused, because the hook keeps a `period_start` that already exists."
  ]
}

Generated by Claude Code

ACCEPT — PR #1110

Reviewed against GitHub, not against the report's own claims.

check	result
PR exists, draft, base `main`, first line `Fixes #1093`	✅ — and `Fixes` is right here: the card's whole scope shipped, nothing deferred
CI	✅ 9/9 check runs `completed` / `success` — Quality Checks, Build and Test (22.x), Check Changeset, link-check, Playwright, CodeQL, Analyze Code, Label PR, Vercel. Read as job conclusions, not as an aggregate
scope of changed files	✅ 7 files: the object, its 4 locale help strings, one new test file, one changeset. Nothing unrelated
changeset present	✅ `.changeset/forecast-period-end-calendar-window.md`, `patch`, written for users rather than for the diff
acceptance criteria	✅ option 2 as ruled, `type: 'script'`, coherent with #1081's pair
ADR / skills path fork	n/a — no `docs/adr/**` or skills-root paths in the diff, so the ordinary landing path applies

The three things this card could have failed on, and didn't

The seeds. The highest-risk item — a rule that refuses the shipped seeds turns a tightening into a pnpm demo:reset boot failure. Verified two independent ways: arithmetically over all 7 seeded rows, and by running each real seed row through the shipped rule on the engine. Correctly done as an assertion rather than an inspection of literals, since the rows are recomputed against new Date() on every import.
The gate can fail. Demonstrated twice, not once: a live in-suite reverse verification (schema clone with only this rule removed admits the ten-month row and stores period_label: 'Q3 2026' next to period_end: 2027-05-15), and a physical revert of the object to origin/main → 19 failed | 22 passed, failures all in the predicted direction while the "gate can fail" and seed suites stayed green. That second one also proves the refusals come from this rule and not from period_end_after_start or the picklist.
The refusal is the right refusal. Every assertion goes through envelope code + exact message, and the #4649 abort shape is filtered out explicitly — so a predicate that could not evaluate cannot satisfy any assertion in the file. That is the distinction that makes a refusal test worth having.

Two corrections to the dispatch, acknowledged in the open

The dev overturned things I asserted, and both corrections are right:

My CEL assumption was directionally wrong. I flagged "CEL may not have the date arithmetic" and pointed at fix(forecast): refuse a period_start off its calendar boundary (#1008) #1081's comment that the stdlib has no month/day accessor. That comment is true but does not say what I used it for: it blocks inspecting one date's components, not comparing two. addMonths / addDays / daysBetween are all registered, so the arithmetic was available all along — which is why the two start rules use a regex and this one does not. Not a divergence in taste, and the PR says so at the rule.
The dev's own presumed direction came out inverted and was pinned rather than smoothed. Re-labelling a monthly row as quarterly is admitted, because an update naming only period leaves period_end unset and the hook re-derives it calendar-true before the rule sees the record. That is the derivation working. The rule's real case is the update that carries the stale period_end explicitly — which is exactly what the record form posts. Both directions are now separate tests with the reasoning written at them. Turning a surprising result into two pinned tests instead of one deleted assertion is the behaviour I want.

One new measurement worth carrying forward

daysBetween(null, …) reaches BigInt(NaN) and throws, which the engine reports as predicate failed to evaluate — i.e. a rejected ordinary save (#4649). This is a third hazard beyond the two AGENTS.md names, and it is invisible to object-validation-predicates.test.ts: that sweep greps the operands of ordering comparisons (this predicate has none), and its totality run nulls every field at once, which short-circuits before daysBetween. The pin lives in this card's test file. ⇒ Worth a follow-up to widen the repo-wide sweep, which I am filing rather than riding on this PR.

Marking ready and enabling auto-merge. Follow-up filed by the dev: #1111 (the sales/forecasting paragraph that both #1081 and this PR make stale).

Generated by Claude Code

Navigation Menu

Repository navigation

A hand-filled `period_end` walks around #1008's calendar-boundary fix — a row labelled `Q3 2026` can still span ten months #1093

The claim #1081 made

What was measured in #1085

The consequence

Confidence, stated honestly

Options, for whoever grades this

The fix fork is now settled by measurement, not preference

ACCEPT — PR #1110

The three things this card could have failed on, and didn't

Two corrections to the dispatch, acknowledged in the open

One new measurement worth carrying forward

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

A hand-filled period_end walks around #1008's calendar-boundary fix — a row labelled Q3 2026 can still span ten months #1093

Description

The claim #1081 made

What was measured in #1085

The consequence

Confidence, stated honestly

Options, for whoever grades this

Activity

huangyiirene commented on Aug 11, 2026

The fix fork is now settled by measurement, not preference

yinlianghui commented on Aug 12, 2026

yinlianghui commented on Aug 12, 2026

yinlianghui commented on Aug 12, 2026

ACCEPT — PR #1110

The three things this card could have failed on, and didn't

Two corrections to the dispatch, acknowledged in the open

One new measurement worth carrying forward

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

A hand-filled `period_end` walks around #1008's calendar-boundary fix — a row labelled `Q3 2026` can still span ten months #1093