fix: time, bounds, and the day that must stay dark - #15
Merged
Conversation
Thirteen findings from a full read of 0.2.0. The thread through most of them is that "today" was ambient rather than an input, which is why a test could rot, a report could call a dangerous day free, and docs/ART.md could not reproduce its own example output. The one that matters most: a day inside the text block that is not part of a letter has to stay empty, and a contribution on it is the only loss nothing takes back. Those days were dropped from the plan for having nothing to report, so every surface described them as days outside the text — free to commit on. They are kept now, named `keep-dark`, and surfaced a day early through the Action's `tomorrow-kind`. Closes #2 — `--today DATE`, so a report is a fact about a date Closes #3 — two tests asserted on the wall clock; CI turned red on Aug 20 Closes #4 — the tracker called a must-stay-dark day free to commit on Closes #5 — `--commits -1` wrapped to 4 billion, `--start-week -1` panicked Closes #6 — a saved plan's user was loaded and then overwritten with nothing Closes #7 — `--backfill`, and the back-dating advice now matches its own prose Closes #8 — a `--merge` calendar from another year read as an empty year Closes #9 — `fail-on: behind` fired on background days, blaming a letter day Closes #10 — probe's rustdoc rendered twice, demo's summary belonged to Stats Closes #11 — a Windows job, which the cfg(not(unix)) path had never had Closes #12 — `y`/`Y` removed, `PageUp`/`PageDown` documented Closes #13 — action.yml still described the pre-0.2.0 build-from-checkout Closes #14 — sixel gave up a register; demo re-implemented art::level
Two rough edges found while testing the above. `--today` carries no year of its own, so a date outside the plan's year made the whole "what to do next" half of the report vanish with no explanation — and the help text offers exactly that command as an example. It now says so, and says which `--year` was probably meant. Still allowed: every letter day of an ended year is overdue, which is a retrospective worth asking for. Fourteen test scratch paths named a constant under the temp directory, so two test processes in one checkout raced over the same file and failed in ways that looked like the code. Observed while running a stress loop beside an ordinary `cargo test`. CONTRIBUTING.md §3 asks for hermetic tests, and a fixed global path is not one.
Found by an adversarial review of the previous two commits. Two ways past every bound the command line enforces, both through the plan file, which `--plan PATH` names and which need not have come from your own `--save`. A `top` of `usize::MAX` wrapped `place`'s guard — `usize::MAX + 5` is 4, comfortably "inside" a seven-row week — and drew the letters on scrambled rows; a `commits` of `u32::MAX` quoted four billion commits a day; a `year` of -262143 panicked building the calendar, and 180000 drew a calendar `cli::YEARS` exists to refuse. `Spec::validate` now applies the flags' own ranges, and `Grid::new` keeps the promise its own doc comment makes about returning `None`. Three counts summed from a `--merge` calendar could wrap. The year's peak is the number the whole costing model rests on; a calendar whose busiest day held four billion reported a peak of 8 and quoted a price to match, in release builds. `--backfill` wrote the whole year while every doc surface called it the flag for days already past — including the command `--track` prints under "only back-dated commits reach those". It now stops at `--today`, and its 34 days and 3,464 commits are exactly what `--track` reports as overdue. The `--today` note fired on the first example in the help, the README and docs/ART.md: tracking a year that has not started, where the report already answers properly. It now fires only when the plan's year has ended, which is the case that was silent. Also: `tests/smoke.rs` still asserted the cursor was on Dec 31 2027 "since today is not in it", which goes red for all of calendar 2027; `Shades::ceiling` documented a `None` it can never return; `mossaic --help` was the last place still listing only `[ ]` for the year keys; and the CHANGELOG claimed the markdown report already agreed about `hole`, when in fact it had been calling a day with no contributions at all a permanent hole.
A second review pass confirmed the previous commit closed all ten of its findings, and turned up the sibling of the one it fixed most carefully. `github::parse` bounded how wide a saved calendar was — 400 days — but not where it was, and the grid reaches outside the days it is handed: back to the Sunday before the first, and a week forward from the cursor. A file whose first day sat within three of the earliest date a NaiveDate can express panicked in release as well as debug. In the chart that was the worst of it: the fetch runs on its own thread, so the thread died, the channel never disconnected, and the screen sat on "loading" for ever with no error and no way to retry — worse than a missing file, which does report itself. `Calendar::build` now degrades to an empty grid, for the same reason `art::Grid::new` returns `None`, and `move_cursor` no longer adds to a date before clamping it, which is a crash one arrow key away. Also from that pass: - docs/ART.md claimed the backfill total is "exactly" what --track reports as already past, "by construction". True with no background, false with one — `overdue` counts letter days and a backfill also lays down the past field. - "one pass finishes the plan" survived from before backfill became past-only. It finishes the past; the rest is deliberately left to happen. - The test mirroring that property was named for finishing the plan and its comment claimed to mirror what backfill writes. It mirrors the arithmetic; the date gate is covered end to end in tests/art_cli.rs. - Refusing a plan file now names the way out. 0.2.0 accepted `--commits -1`, which `--save` wrote down as four billion, so a plan in the wild can be one this release will not load. - write_commits streams into git instead of building the whole fast-import body in a String first. Worth about 2 MB at 150,000 commits rather than the 40 the buffer suggested — git fast-import's own mark table is the bulk of it — but the allocation grew with the commit count and now does not.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Thirteen findings from a full read of 0.2.0, filed as #2–#14 and fixed here. The
thread through most of them is that "today" was ambient rather than an input,
which is why a test could rot, a report could call a dangerous day free, and
docs/ART.mdcould not reproduce its own example output.The one that matters most
A day inside the text block that is not part of a letter has to stay empty,
and a contribution on it is the only loss nothing takes back. Those days were
dropped from the plan for having nothing to report (
src/plan.rs, "a dark daythat is dark is not news"), so
Plan::onansweredNoneand every surfacedescribed them as days outside the text:
2026-08-20 is inside VYNCINT's block, week 33 row 4. Committing on it punches a
permanent hole. It now reads:
Standing::kindgainskeep-dark, andholenow names damage rather thanposition — a clean day inside the letters is
keep-dark, and becomes aholeonce it is too bright. The Action gains
today-kindandtomorrow-kind, sodo not commit tomorrow can arrive while it can still be acted on.
Two new flags
--today DATEmakes the tracker's answer a fact about a date rather thanabout when it ran. That is what defuses #3, what lets
docs/ART.mdkeep its"reproduce this exactly" promise, and what answers "what will 1 June owe?".
--backfillcommits each day's shortfall instead of a flat count. A plain--writeputs the same number on every lit day including the ones alreadybright, and adding to the busiest of those raises the year's peak — which raises
what every other letter day needs, so the bar moves as you walk towards it. A
shortfall provably cannot: the brightest shade is three quarters of the peak, so
topping a day up to
neednever exceeds the busiest day. One pass.a_backfill_finishes_the_plan_without_moving_the_targetadds the shortfalls to areal year and checks the price did not move.
Bounds
--yearwas range-checked because a binary once "passed 999999 through to apanic". Every other number took the unguarded path, and
asis not a check:--commits -1→ 4,294,967,295 commits--commits 0→ bright letters, no commits--top -1→ wrapped past the guard meant to catch it--start-week -1→ panic inNaiveDate + Days--start-week 100→ silently drew a plan of no daysOne
Args::number_inprimitive, whichyear()now rides on so the two cannotdrift apart.
Everything else
userwas read out of the file and then overwritten withthe nothing a bare
--trackcarries — so it tracked the wrong person ratherthan saying so.
--mergecalendar from another year filtered down to nothing, and9,527 contributions read as an empty, trivially-reachable year.
fail-on: behindtestedtoday.short, which is also non-zero for ashort background day — so with
backgroundset, ~290 days a year failed thejob and blamed a letter day for it.
--trackprinted did not do what thesentence above it promised: it ignored the saved plan, defaulted to 4 commits
where the plan needed 110, and wrote to every lit day.
term::probehad doc comments on both sides of its#[cfg]andrendered them concatenated on docs.rs;
calendar::demo's summary line belongedto
Stats; the non-unixprobehad none.cfg(not(unix))path was compiled by no gate andhad drifted in two ways
-D warningscatches.y/Yremoved (undocumented, duplicated four other keys, andymeans yank to the audience
h j k lis for);PageUp/PageDowndocumented.before
seen.len() as u8would have wrapped to 0 and aliased a colour;restructured so no such index is constructed.
calendar::demono longerre-implements
art::level.Also, unasked but adjacent: a login resolved from a plan file is now stripped of
control characters where it is resolved rather than at each place that prints it,
which is the rule the calendar already followed.
Testing
keep-dark days,
place's refusal (includingusize::MAX),--todayreproducibility, the flag bounds, the plan's user, the year-mismatch note, and
--backfill --writeverified throughgit log.--today;CONTRIBUTING.md§3 says so.testjob re-runs the CLI suite underTZ=Pacific/Kiritimati, a timezonethat is already tomorrow — a backstop for the next assertion of that shape.
fmt,clippy -D warnings,cargo docwithRUSTDOCFLAGS=-D warnings, MSRV 1.88cargo check --locked, the same clippyand doc gates cross-compiled to
x86_64-pc-windows-msvc, and 25 sequentialrelease runs of the whole suite.
Checklist
cargo fmt --all,cargo clippy --all-targets --all-features -- -D warnings,cargo testCHANGELOG.mdupdated under[Unreleased]README.md,docs/ART.md,docs/DESIGN.md§11,CONTRIBUTING.md§3,action/README.md