Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-timeline): judge gantt dates by TYPE — string, finite number, or Date - #6905
Merged
Conversation
…er, or Date Maintainer ruling 2026-08-30 (option A of three): a gantt date must BE a date type. Everything else — `false`, `true`, arrays, objects with a plausible `toString`, bigints, symbols — is refused through the existing loud diagnostic instead of being coerced by `new Date` into a silent 1970 axis. `0` is KEPT, deliberately and by ruling: it is a legitimate epoch timestamp under a millisecond encoding, so refusing it would take away a real capability to catch a hypothetical input. Its filed `endDate: 0` reading is pinned UNCHANGED, bar geometry included. The type gate runs BEFORE `new Date`, which is load-bearing: `new Date` THROWS on a bigint or a symbol, so on the base the guard crashed while trying to report the author's error. The predicate is now total. `null` and `undefined` are subsumed by the rule rather than named separately; every diagnostic an author sees for them is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sam
commented
Aug 30, 2026
CollaboratorAuthor
The out-of-scope finding the description refers to is #6907 — Session for this implementation: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB Generated by Claude Code Generated by Claude Code |
os-sam
commented
Aug 30, 2026
CollaboratorAuthor
Clause-② review — ACCEPT WITH FOLLOW-UP
CI: not terminal at dispatch (5 runs in progress). Terminal at review time, read from the check-runs table on this head: 30 total · 27 success · 3 skipped · 0 failures. The 3 skips ( Re-measured myself (not inherited)
Taken on report (each cross-covered): the dev's local turbo/type-check/gate exit codes (CI re-ran all of them green on this head); #6907 — is shipping the accept set without the spelling repair net-worse for an author?No — the separation is right, and I would have objected to the opposite. Grounds independent of the dev's stated ones:
So: not a gate. #6907 as filed states the arms and the pinned-spelling constraint accurately (its measurements match my probes), and it should be adjudicated on its own. ChangesetRead as a release-notes reader with no thread context: it says this is a reject-direction change, lists what stops parsing, names the diagnostic and the path it carries, tells the author what to emit instead (date string / ms timestamp / One precision defect, fixable here: the table header reads VerdictACCEPT WITH FOLLOW-UP, on terminal-green CI (30 runs: 27 success, 3 never-ran skips, 0 failures):
The PR remains draft; marking ready and merging are not this reviewer's to do. Generated by Claude Code |
Clause-② review follow-up. The table header read `startDate / endDate / minDate / maxDate`, but its before/after readings are ROW-DATE readings: a falsy `minDate` / `maxDate` pin is discarded by the caller's existing truthy-only `||` before anything judges it, so it never drew the 1970 axis and is not refused now. Promising "refused, named" there would send a `minDate: false` author looking for a diagnostic that never fires. Header now scopes the table to a row item's `startDate` / `endDate` or a TRUTHY pin, and a note states the falsy-pin behaviour explicitly. Release-notes text only — no source, no test, no scope change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sam
commented
Aug 30, 2026
CollaboratorAuthor
Clause-② follow-up discharge — CONFIRMED at |
os-sam
marked this pull request as ready for review
August 30, 2026 14:59
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 30, 2026
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 freeto 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.
Fixes#6781
Maintainer ruling 2026-08-30 (comment 5469030420, batch #6), option A of three:
a gantt date is judged by type, and the rest is refused through the loud
diagnostic that already exists — no second channel, no new i18n key.
The rule, written down as a rule
packages/plugin-timeline/src/renderer.tsxgainsisGanttDateType, a namedmodule-scope predicate carrying the ruling, the accept set stated as an accept
set, the measurements below, and a citation of this card.
findUnusableGanttDateasks it first and parses second. The ruling asked for exactly this — the rule
recorded as a ruled rule rather than as one more unattributed special case, which
is the defect the previous three cards each added one instance of.
The accept/reject move, exactly
Measured on the base
fab4802e3with a throwaway probe, one row item,startDate: '2024-01-01'andendDatevaried. Every row here is a value thatparses today and stops parsing after this PR:
falseleft: 100%; width: -100%;trueleft: 100%; width: -99.99999999994131%;['2024-01-01']["Jan 2024"]; barleft: 0%; width: 100%;[0]{ toString() { return '2024-01-01' } }["Jan 2024"]bigint(0n)TypeError: Cannot convert a BigInt value to a number, uncaught, mid-rendersymbolTypeError: Cannot convert a Symbol value to a number, uncaughtWhat a consumer with such data now sees: the alert #6759 built and #6770
reused —
role="alert", naming the authored path (items[0].items[0].endDate)and the value — and no chart. Before, they saw a chart. The
falsereading isbyte-identical to the
endDate: 0symptom this card was filed on, which is thewhole argument: the renderer could not tell "the author means the epoch" from
"a mapping layer emitted a boolean into a date column".
The last two rows are not silent renders but crashes, and they move in the
good direction:
new Datethrows rather than returning an invalid date for thesetwo types, so on the base the guard died while trying to report the author's
error. Judging the type before parsing makes the predicate total, and it makes
spellGanttDateValue'ssymbolbranch — written by #6759 as "total byconstruction" and documented as unreachable — reachable at last.
Not moved, measured:
[]and{}were already refused on the base(
new Date([])andnew Date({})are invalid dates), and stay refused.NaNand
Infinitylikewise. Thefiniteclause therefore changes no verdict; it isthere so the accept set reads as the rule it is instead of leaving the exclusion
to a
new Datecoercion detail a later reader has to rediscover.0is kept — the control that had to stay greenRuled explicitly, and it is the one behaviour this change must not alter.
0isa finite number, so it is accepted and still draws the epoch. Two pins hold it,
including this card's own filed reading asserted unchanged down to the bar
geometry (axis 649 columns,
left: 100%; width: -100%;), so a future wideningthat catches
0on its way past turns that row red.Pre-verification 1 — census of integer
0fed into a gantt dateZero in-repo sites. Control that must hit, in the same query family: 222
gantt-date assignment sites repo-wide (
startDate/endDate/minDate/maxDate), so the query is live rather than mis-typed. The only numeric ganttdate anywhere in the repo is
endDate: Date.UTC(2024, 2, 1)in #6770's pin file;the only literal is inside a comment in
renderer.tsx.Honest reading: "keep
0" rests on the encoding's terms — under astartDate: 1704067200000millisecond encoding0really is a date, andrefusing it would remove a real capability for a hypothetical input — not on a
measured in-repo consumer. That is why the pin for it is a test rather than a
call site.
Pre-verification 2 — can
false/[]reach the renderer from the SDUI author surface?Row dates: yes, entirely unguarded — the type rule is the only gate there.
TimelineSchemadeclaresitemsasz.array(z.any())(anditems?: any[]inthe TS twin), so measured through
TimelineSchema.safeParse:endDateminDatepinfalse[]{}0'2024-01-01'So the benefit on row dates lands on the full authoring population, JSON
metadata included — not only hand-written TS.
Pins: defence in depth, as the ruling anticipated.
minDate/maxDatearez.string(), so a non-string pin is rejected at the authoring boundary — butonly by the boundary that actually runs it, which is the CLI
(
packages/cli/src/commands/check.tsandvalidate.ts).TimelineRendererdoesnot validate at render, so the type rule is the only gate for a document that
never went through
objectui check. The pin path barely moves regardless: thecaller judges only a truthy pin, so
false/0/''pins stay discardedby the existing
||, exactly as before.Clause ② — this is a reject-direction change
Opened as a draft for the
CONTRACT_REVIEW_TIERreview; not marked ready,not merged. The reject-direction move is the table above; the changeset states
it in the same terms for people who never read this thread, including what an
author should emit instead.
Verification
Reproduce-red came first, as an ablation of the new arm rather than a claim:
!isGanttDateType(value) ||was replaced on disk with the base armvalue === null ||. Mutation proven on disk by anchor counts (new-arm 1 to 0,base-arm 0 to 1) and a changed blob hash; restore proven both ways —
git diff HEADempty andgit hash-objectequal to the HEAD blob5a47a6fef0701cea53fd4b86207492d9020a476e— under atrap ... EXIT INT TERMwith absolute paths. No
distleg exists to skip: the pin file imports../rendererby relative path, so vitest compiled the mutated source directly.Predicted 9 red, observed 9 red, same set, row for row (
Tests 9 failed | 15 passed (24)):falsetrue['2024-01-01'][0]{ toString }ObjectTimelinereaches the same refusalendDate: 0UNCHANGED0as a meaningful epoch startDateobjectDate/ valid-gantt geometrynull/undefined/ one-diagnostic / bad stringThe two
0rows are green in both directions, which is the sharpest form ofthat control: the change provably does not touch
0.Union below ran on the pushed head
7b34c944a(treee80d2a2e3848dda1a7f1b00fc297b5199e9479ea, byte-identical to the tree the runwas made on), from the repo root as AGENTS.md requires:
vitest run packages/plugin-timeline/turbo run type-check --filter @object-ui/plugin-timelineturbo run lint --filter @object-ui/plugin-timelinecheck:control-bytesOK (scanned 5752 tracked text file(s))check:vi-mock-specifiers,check:vi-mock-inheritcheck:i18n-keys,check:i18n-drift,check:i18n-dead-keyscheck-changeset-presence,changeset:checkcheck:side-effects-arraytsc -p tsconfig.test.json --listFilesconfirms both changed files areinside the typecheck program (1 hit each), so "typecheck clean" is a statement
about the new test file too, not around it.
Lint compared against the base version of the same file, not a bare count:
base
renderer.tsx(46711 bytes, extracted fromfab4802e3and byte-checkedbefore use) reports 18 warnings / 0 errors; head reports 17 / 0 — the same rule
set, one fewer
no-explicit-anybecause the predicate dropped itsvalue as anycast. The new pin file: 0 errors, 3no-explicit-anywarnings,against 4 warnings on #6770's sibling pin file it is modelled on.
Declared narrowing. The repo-wide
pnpm lintis CI's run. Locally the wholeaffected package was linted, and the narrowing is measured rather than assumed:
the population is read from eslint's own config (24 files in
packages/plugin-timeline), the count from--format json(0 errors, 96warnings, all pre-existing), and
eslint.config.jssets noparserOptions.projectorprojectService, so type-aware linting is off andthis diff cannot move the verdict on any file it does not touch. Two gates,
check:eager-closureandcheck:sdui-registration-pins, exited 2 asPREREQUISITE NOT MET — both print that they need an
apps/consolebuild andthat an absent build is "a broken gauge, not a passing budget". They are recorded
as NOT MEASURED locally, not as failures; CI builds the console and runs them.
Scope
packages/plugin-timeline/only, plus the changeset. Line numbers werere-derived on this base, not copied from the card (#6782 had already landed in
this file). Nothing under
content/docs/releases/, no test skipped, disabled orquarantined.
spellGanttDateValueis deliberately untouched: how a refused valueis spelled is a separate adjudication from which values are refused, and the
ruling named
findUnusableGanttDate/isUnusableas the landing site. Themeasured consequence is reported separately rather than repaired here — see the
out-of-scope finding filed alongside this PR.
Generated by Claude Code
Generated by Claude Code