From fbeab536522be1b567d6dd1df6f9d6580c61794b Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:07:00 +0700 Subject: [PATCH 1/2] fix: offer a picture the column that draws it cleanly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--track` on a picture passed `None` where the text path passed a real sweep, so for the one verdict a reader cannot answer by contributing more the tool printed the diagnosis and stopped. The reasoning at the call site was that a picture "is usually the full width of the year, so there is no column to move it to, and offering one that does not fit is worse than offering none". True of the four shipped templates — dragon is 53 columns, wave, pulse and invader are 51 — and wrong about every picture narrower than the year, which is the shape docs/ART.md and #57 ask contributors to draw. The overhang it guarded against is now measured by the sweep rather than assumed at the call site, so a picture with ink to its edges still gets nothing, for the reason instead of by refusing to look. `best_start_week_of` is the canvas twin of `best_start_week`, and the two differ in what a hole *is*. Text has one shade: inside a letter or not. A picture has five, so a day can be a hole by being too bright for the shade it is drawn at — level 2 with the year's peak on it is a hole exactly as level 0 with a single commit is. Rather than reimplement that test, each candidate is built into a real Plan and asked for its own holes(), the same call Report makes for the number it publishes; a suggestion that disagreed with the verdict beside it would be worse than no suggestion. Two things came out of testing it against the plan that reported it. A suggestion has to be a column you can still start in. Ranked by holes and then by column, the answer for vyncint/contribution-art's eleven-column heart was week 11 — clean, correct, and in March. That year had nine placements costing zero holes and the leftmost is always the one furthest in the past; the only way to draw there is --backfill into days five months gone. Among columns that tie, one that has not begun now wins, which makes the answer week 37: an instruction that can be followed this afternoon. Holes still win outright, because back-dating is a thing this tool does and unlighting a day is not. Text gets the same treatment, and needs it — the example in the issue suggested week 1 in August. And a clean column deserves to be described as one. "would leave 0 holes instead of 5" is arithmetic the reader has to finish before they know it is the answer to their problem; "--start-week 37 draws it cleanly" is the answer. The holed headline carries it too — that line is the Action's headline output and the subject of the issue the shipped consumer opens, so it is where "this year is lost" was actually being published. The Action gains suggested-start-week and suggested-holes so a workflow can act on the answer rather than only print it. The CLI test is the one that matters here. The defect was a single argument at a call site, so every unit test of the sweep passed while the tool said nothing; only the layer a user sees could have caught it. Closes #97 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- CHANGELOG.md | 45 ++++++++- README.md | 5 + action/README.md | 18 +++- action/action.yml | 19 ++++ docs/ART.md | 47 +++++++++- src/bin/mossaic-art.rs | 55 ++++++++--- src/plan.rs | 158 ++++++++++++++++++++++++++++--- src/render_tests.rs | 205 +++++++++++++++++++++++++++++++++++++++-- tests/art_cli.rs | 132 ++++++++++++++++++++++++++ 9 files changed, 651 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68dde3f..dedf5e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,49 @@ listed under a **Changed** or **Removed** heading. ## [Unreleased] +### Fixed + +- **A picture was never offered a better placement, even when one drew it + cleanly** (#97). `--track` on a picture passed `None` where the text path + passed a real sweep, so the tool said *"cannot be drawn cleanly"* and stopped + — for the one verdict a reader cannot answer by contributing more. The + reasoning at the call site was that a picture "is usually the full width of + the year, so there is no column to move it to", which is true of the four + shipped templates and wrong about every picture narrower than the year — the + shape `docs/ART.md` and #57 ask contributors to draw. The overhang it was + guarding against is now *measured*, so a full-width template still gets no + suggestion, for the reason rather than by refusing to look. + + Found on a live plan: `vyncint/contribution-art` draws an eleven-column heart + and posts the report to a public issue. Five days inside the picture picked up + ordinary contributions after the plan was made, so every day it published + "this year is lost" when what it meant was "move it two columns right". + +### Added + +- **`suggested-start-week` and `suggested-holes` Action outputs**, so a + workflow can act on the answer rather than read it. `suggested-holes` of `0` + means that column draws the plan cleanly. + +### Changed + +- **A suggestion prefers a column that has not begun**, when several cost the + same number of holes. A clean column in March is arithmetic, not advice: the + only way to draw there is `--backfill` into days months gone. This is not a + corner case — an eleven-column picture in a fifty-three column year can have + nine placements costing zero holes, and ranked by column alone the answer is + always the one in January. Holes still win outright; the preference only + breaks ties, because back-dating is a thing this tool does and unlighting a + day is not. Applies to text as well as to pictures. +- **A clean column is described as one.** `--start-week 37 draws it cleanly.` + rather than "would leave 0 holes instead of 5" — arithmetic the reader had to + finish before knowing it was the answer to their problem. The `holed` headline + carries it too, which is where it was most missing: that line is the Action's + `headline` output and the subject of the issue the shipped consumer opens. +- **`plan::best_start_week` takes a `today`**, and `plan::best_start_week_of` + is its canvas twin. Breaking for anything calling the library directly; the + binaries and the Action are unaffected. + ## [0.7.0] - 2026-09-06 Twenty findings and one security advisory, all reported against 0.6.3 with a @@ -1066,7 +1109,7 @@ there was none. [termlens]: https://github.com/vyncint/termlens -[Unreleased]: https://github.com/vyncint/mossaic/compare/v0.6.3...HEAD +[Unreleased]: https://github.com/vyncint/mossaic/compare/v0.7.0...HEAD [0.7.0]: https://github.com/vyncint/mossaic/compare/v0.6.3...v0.7.0 [0.6.3]: https://github.com/vyncint/mossaic/compare/v0.6.2...v0.6.3 [0.6.2]: https://github.com/vyncint/mossaic/compare/v0.6.1...v0.6.2 diff --git a/README.md b/README.md index c625f98..f40250b 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,11 @@ placement that would salvage the most: --start-week 1 would leave 23 instead of 61. ``` +When a column draws it *cleanly* it says so instead — `--start-week 37 draws it +cleanly.` — and it says it for pictures as well as for text, preferring a +column that has not begun when several cost the same. A clean column in March +is arithmetic; one starting next Sunday is advice. + **A field beats an empty graph.** Letters on nothing means not contributing on the other 290 days of the year. `--background 1` draws the background as a shade instead, so the art is one green against another and the year stays alive — the diff --git a/action/README.md b/action/README.md index 10f561c..c15a9aa 100644 --- a/action/README.md +++ b/action/README.md @@ -123,13 +123,29 @@ least two levels between the two — the `legibility` output says `clear`, notification title), `markdown` (the whole report, fit for a message body), `json` (everything), and the scalars: `bright`, `letters`, `owing-days`, `owing-commits`, `holes`, `today-short`, `tomorrow-need`, `today-kind`, -`tomorrow-kind`. +`tomorrow-kind`, `start-week`, `columns`, `suggested-start-week`, +`suggested-holes`. With a `background` set, also: `field-level`, `field-days`, `field-bright`, `field-owing-days`, `field-owing-commits`, plus `legibility` (`clear` / `readable` / `faint`) and `separation` (the CIE76 ΔE between the two shades in the worst palette a reader might have). +`suggested-start-week` and `suggested-holes` say where the plan would be +better placed. They matter for one verdict: `holed` is the only one you cannot +answer by contributing more, so the column to move to is the only thing left to +do about it — and until 0.8.0 a picture was never offered one. Act on them when +`suggested-holes` is below `holes`; `0` there means that column draws the plan +cleanly. + +```yaml + - name: Say where it can still be drawn + if: >- + steps.art.outputs.verdict == 'holed' && + steps.art.outputs.suggested-holes == '0' + run: echo "move it to week ${{ steps.art.outputs.suggested-start-week }}" +``` + `today-short` counts a background day too, so a daily "what do I owe today" notification keeps working unchanged when you add one. `today-kind` is what tells the two apart — `letter`, `background`, `keep-dark`, `hole` or `outside`. diff --git a/action/action.yml b/action/action.yml index 4f8c845..0f01a04 100644 --- a/action/action.yml +++ b/action/action.yml @@ -190,6 +190,23 @@ outputs: columns: description: How many calendar columns the plan spans. value: ${{ steps.track.outputs.columns }} + suggested-start-week: + description: >- + The best column the year has for this plan — often the one it already + sits in, so it is worth acting on only when `suggested-holes` is below + `holes`. A `holed` plan is the case this exists for: it is the one + verdict that cannot be answered by contributing more, so the column to + move to is the only thing left to do about it. Empty when no column + fits: a placement that pushes part of the drawing out of the year is + never suggested, and the first and last calendar columns are partial + weeks, so a picture with ink right to its edges has nowhere to go. + value: ${{ steps.track.outputs.suggested-start-week }} + suggested-holes: + description: >- + The holes `suggested-start-week` would leave. `0` means that column + draws the plan cleanly; any other number below `holes` means less bad + rather than fixed. + value: ${{ steps.track.outputs.suggested-holes }} legibility: description: >- `clear`, `readable` or `faint` — how well the letters stand out from the @@ -369,6 +386,8 @@ runs: "field-level=" + (.field_level | tostring), "start-week=" + (.start_week | tostring), "columns=" + (.columns | tostring), + "suggested-start-week=" + ((.suggested_start_week // "") | tostring), + "suggested-holes=" + ((.suggested_holes // "") | tostring), "legibility=" + .legibility, "separation=" + (.separation | floor | tostring), "field-days=" + (.field_days | tostring), diff --git a/docs/ART.md b/docs/ART.md index 1f8379e..d66f560 100644 --- a/docs/ART.md +++ b/docs/ART.md @@ -345,6 +345,48 @@ plan wants dark, and contributing on it punches a hole in the drawing exactly as contributing inside a letter does. That is why the table above reports 219 of them: they are part of the picture. +### When a picture cannot be drawn where it sits + +`holed` is the one verdict you cannot answer by contributing more: days inside +the picture are already brighter than the shade they are drawn at, and nothing +takes a contribution away. So it is the verdict that most owes you a next move, +and `--track` sweeps every column of the year to find one: + +``` + Cannot be drawn cleanly — 5 days are brighter than the picture wants, + and nothing takes a contribution away. + --start-week 37 draws it cleanly. +``` + +It is offered in every format — `suggested_start_week` and `suggested_holes` in +json, a line of its own in markdown, and the `suggested-start-week` output on +the Action. + +Two things decide which column it picks. + +**Fewest holes wins.** If nothing draws the picture cleanly you are told the +least bad column instead — `--start-week 12 would leave 3 holes instead of 25` +— and if every column is equally bad you are told that, because an emptier year +is then the only way out. + +**Among columns that tie, one that has not begun yet.** A clean column in March +is arithmetic, not advice: the only way to draw there is `--backfill` into days +five months gone. This matters more than it sounds. An eleven-column picture in +a fifty-three column year can easily have nine placements costing zero holes, +and ranked by column alone the answer is always the one in January. + +The preference only breaks ties. A past column that draws the picture cleanly +still beats a future one that does not, because back-dating is a thing this tool +does and unlighting a day is not. + +A column that would push part of the picture off the end of the year is never +suggested: a truncated picture is not a cleaner drawing of the same picture, it +is a smaller one. The first and last calendar columns are partial weeks, so a +picture carrying ink right to its edges overhangs them wherever it is put, and +is offered nothing at all. Blank margins are not counted — losing an empty cell +costs the picture nothing — so `dragon`, fifty-three columns wide with quiet +edges, still places. + ## Tracking it, day by day Drawing the art is one command. Getting there while also living a normal year is @@ -411,7 +453,10 @@ Four kinds of answer, and only two of them are work: Nothing takes contributions away, so it is a hole in the text for good. This is the honest answer to "why can't I write VYNCINT in 2026": not that it is expensive, but that the year has already been written on. `--track` counts the - holes, and sweeps `--start-week` to find the placement that runs into fewest. + holes, and sweeps `--start-week` to find the placement that runs into fewest — + for a picture as well as for text, preferring a column that has not begun when + several cost the same. See + [When a picture cannot be drawn where it sits](#when-a-picture-cannot-be-drawn-where-it-sits). - **A day outside the text with contributions.** Noise around the letters rather than damage to them; reported, not warned about. diff --git a/src/bin/mossaic-art.rs b/src/bin/mossaic-art.rs index 64660ad..00a4fbe 100644 --- a/src/bin/mossaic-art.rs +++ b/src/bin/mossaic-art.rs @@ -829,10 +829,17 @@ fn track_canvas( let year_total = actual .values() .fold(0u32, |sum, count| sum.saturating_add(*count)); - // No suggestion: a picture is usually the full width of the year, so - // there is no column to move it to, and offering one that does not fit - // is worse than offering none. - let report = plan::Report::of(&plan, &who, year_total, today, None); + // The sweep, on the path that publishes. A picture used to pass + // `None` here on the reasoning that it "is usually the full width of + // the year, so there is no column to move it to" — true of the four + // shipped templates and wrong about every picture narrower than the + // year, which is the shape docs/ART.md asks contributors to draw. + // The overhang it was guarding against is now measured by + // `best_start_week_of` rather than assumed here, so a full-width + // template still gets exactly `None` — for the reason, not by + // refusing to look. + let suggestion = plan::best_start_week_of(canvas, grid, &actual, today); + let report = plan::Report::of(&plan, &who, year_total, today, suggestion); match options.format { Format::Json => println!( "{}", @@ -869,11 +876,29 @@ fn track_canvas( {owing_days} to go.", plural(wanted, "day is", "days are") ), - plan::Verdict::Holed { holes } => println!( - " Cannot be drawn cleanly — {holes} {} brighter than the \ - picture wants,\n and nothing takes a contribution away.", - plural(holes, "day is", "days are") - ), + plan::Verdict::Holed { holes } => { + println!( + " Cannot be drawn cleanly — {holes} {} brighter than the \ + picture wants,\n and nothing takes a contribution away.", + plural(holes, "day is", "days are") + ); + // `holed` is the one verdict a reader cannot act on by + // contributing more, so it is the one that most owes them a next + // move. The letters path has said this since it had a sweep; the + // picture path printed the diagnosis and stopped. + match plan::best_start_week_of(canvas, grid, &actual, today) { + Some((week, 0)) => println!(" --start-week {week} draws it cleanly."), + Some((week, left)) if left < holes => println!( + " --start-week {week} would leave {left} {} instead of {holes}.", + plural(left, "hole", "holes") + ), + _ => println!( + " Every placement in {} runs into the same problem; an emptier\n \ + year is the way out.", + grid.year + ), + } + } } println!("\n level days done owing each"); @@ -1277,8 +1302,15 @@ fn track_progress( // number the text report prints, so a notification never has to be parsed // out of a screen. if options.format != Format::Text { - let suggestion = - plan::best_start_week(grid, columns.len(), options.top, columns, &actual, hideable); + let suggestion = plan::best_start_week( + grid, + columns.len(), + options.top, + columns, + &actual, + hideable, + today, + ); let year_total = actual .values() .fold(0u32, |sum, count| sum.saturating_add(*count)); @@ -1454,6 +1486,7 @@ fn track_progress( columns, &actual, hideable, + today, ) { Some((week, left)) if left < holes => { println!(" --start-week {week} would leave {left} instead of {holes}.") diff --git a/src/plan.rs b/src/plan.rs index 2116e9d..dcaf952 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -504,6 +504,9 @@ fn tally<'a>(days: impl Iterator) -> (usize, u32) { /// spoiling the letters — zero when the background is empty and any /// contribution in there is a hole, higher when the plan draws a background /// those days can hide in. +/// +/// `today` decides ties, through [`chosen`]: among columns that cost the same +/// it prefers one that has not begun. pub fn best_start_week( grid: &Grid, columns: usize, @@ -511,12 +514,13 @@ pub fn best_start_week( lit_shape: &[[bool; GLYPH_ROWS]], actual: &BTreeMap, ceiling: u32, + today: NaiveDate, ) -> Option<(usize, usize)> { if columns > grid.weeks { return None; } - (0..=grid.weeks - columns) - .map(|start| { + chosen( + (0..=grid.weeks - columns).map(|start| { let holes = lit_shape .iter() .enumerate() @@ -532,8 +536,96 @@ pub fn best_start_week( .filter(|date| actual.get(date).is_some_and(|count| *count > ceiling)) .count(); (start, holes) - }) - .min_by_key(|(start, holes)| (*holes, *start)) + }), + grid, + today, + ) +} + +/// Pick one placement out of a sweep: fewest holes first, and among columns +/// that tie on holes, one that has not begun yet. +/// +/// The tie-break is the whole of this function, and it earns its place from a +/// live plan. `vyncint/contribution-art` draws an eleven-column heart in a +/// fifty-three column year, and on 2026-09-06 that year had *nine* placements +/// costing zero holes. Ranked by column alone the answer was **week 11** — +/// clean, correct, and in March. The only way to draw there is `--backfill` +/// into days five months gone, which is not what a reader who has just been +/// told their year cannot be drawn is asking for. The earliest column that has +/// not started is week 37, and that is an instruction they can follow this +/// afternoon. +/// +/// Holes still win outright. A past column that draws the picture cleanly beats +/// a future one that does not, because back-dating is a thing this tool does +/// and unlighting a day is not; the preference only breaks ties. +fn chosen( + candidates: impl Iterator, + grid: &Grid, + today: NaiveDate, +) -> Option<(usize, usize)> { + let swept: Vec<(usize, usize)> = candidates.collect(); + let fewest = swept.iter().map(|(_, holes)| *holes).min()?; + // Ascending by column, because the sweep is, so `find` and `next` both + // mean "the earliest one". + let mut tied = swept.iter().filter(|(_, holes)| *holes == fewest); + let ahead = tied + .clone() + .find(|(start, _)| grid.date_at(*start, 0) >= today); + ahead.or_else(|| tied.next()).copied() +} + +/// The placement that draws a **picture** with the fewest holes. +/// +/// The canvas twin of [`best_start_week`], and it exists because the two +/// differ in what a hole *is*. Text has one shade: a day is inside a letter +/// or it is background, and a hole is any contribution on a day that must +/// stay dark. A picture has five, so a day can be a hole by being too +/// bright for the shade it is drawn at — level 2 with the year's peak on it +/// is a hole exactly as level 0 with a single commit is. A boolean shape +/// cannot express that, which is why this sweeps levels instead. +/// +/// Rather than reimplement the test, each candidate is built into a real +/// [`Plan`] and asked for its own [`Plan::holes`] — the same call +/// [`Report`] makes for the `holes` field it publishes. A suggestion that +/// disagreed with the verdict beside it would be worse than no suggestion, +/// and the only way to be sure they agree is to ask the same question. It +/// costs one plan per column, at most fifty-three of them, over a year of +/// dates; the sweep is not the expensive part of a run that talks to the +/// GitHub API first. +/// +/// A placement that pushes lit cells out of the year is not a candidate. +/// [`Canvas::place`](art::Canvas::place) counts those as `skipped`, and a +/// truncated picture is not a cleaner drawing of the same picture — it is a +/// different, smaller one. This is what keeps a full-width template +/// answering `None` here, which was the whole of the old reasoning for +/// never asking: it is now enforced by measuring the overhang rather than +/// by declining to look. +/// +/// Returns the column and the holes it leaves; `None` when no placement +/// fits the year at all. +#[must_use] +pub fn best_start_week_of( + canvas: &art::Canvas, + grid: &Grid, + actual: &BTreeMap, + today: NaiveDate, +) -> Option<(usize, usize)> { + let width = canvas.width(); + if width == 0 || width > grid.weeks { + return None; + } + chosen( + (0..=grid.weeks - width).filter_map(|start| { + let (levels, skipped) = canvas.place(grid, start); + if skipped > 0 || levels.is_empty() { + return None; + } + let plan = Plan::from_levels("", grid, &levels, start, width, actual); + Some((start, plan.holes().len())) + }), + grid, + today, + ) } /// Contributions a year holds, keyed by date — the shape every function here @@ -932,14 +1024,29 @@ impl Report { fn summarise(&self) -> String { match self.verdict { "drawn" => format!("{} · {} — drawn", self.text, self.year), + // The way out belongs here when there is one. This line is the + // Action's `headline` output and the subject of the issue the + // shipped consumer opens, so for a plan that is holed it is the + // whole of what most readers see — and "cannot be unlit" with no + // second half reads as "this year is lost" when it means "move it + // two columns right". + // + // Only a *clean* column earns the room. A subject line has one + // sentence in it, and "week 12 would leave 17 instead of 25" is a + // trade to weigh rather than a thing to do; that one stays in the + // body, where there is space to weigh it. "holed" => format!( - "{} · {} — {} of {} bright, {} {} that cannot be unlit", + "{} · {} — {} of {} bright, {} {} that cannot be unlit{}", self.text, self.year, self.bright, self.letters, self.holes, - plural(self.holes, "hole", "holes") + plural(self.holes, "hole", "holes"), + match self.better_placement() { + Some((week, 0)) => format!(" — week {week} draws it"), + _ => String::new(), + } ), // The background is work too, and saying "0 to go" while three // hundred field days are bare is the kind of confidently wrong a @@ -959,6 +1066,22 @@ impl Report { } } + /// The placement worth moving to, if there is one: where it goes and the + /// holes it leaves. + /// + /// `None` when nothing was swept, when nothing fits, or when the best + /// column on offer is no better than the one the plan already sits in. + /// That last case is the one worth being strict about: a plan that is + /// holed everywhere is holed, and "try week 12, it is just as bad" is + /// advice that costs a reader a re-run to discover it was not advice. + #[must_use] + pub fn better_placement(&self) -> Option<(usize, usize)> { + match (self.suggested_start_week, self.suggested_holes) { + (Some(week), Some(holes)) if holes < self.holes => Some((week, holes)), + _ => None, + } + } + /// A summary that reads the same in a GitHub step summary, a Slack message, /// a Discord embed and an email — the four places this ends up. pub fn markdown(&self) -> String { @@ -1092,14 +1215,23 @@ impl Report { )); } - if let (Some(week), Some(holes)) = (self.suggested_start_week, self.suggested_holes) { - if holes < self.holes { - out.push_str(&format!( - "\n`--start-week {week}` would leave {holes} {} instead of {}.\n", - plural(holes, "hole", "holes"), + if let Some((week, holes)) = self.better_placement() { + out.push_str(&match holes { + // Worth its own sentence. "would leave 0 holes instead of 5" + // is arithmetic the reader has to finish before they know it + // is the answer to their problem; "draws it cleanly" is the + // answer. The distinction only became reachable when + // pictures started being swept — a text plan wide enough to + // fill the year rarely has a spotless column to move to, + // and an eleven-column picture in a fifty-three column year + // usually has several. + 0 => format!("\n`--start-week {week}` draws it cleanly.\n"), + left => format!( + "\n`--start-week {week}` would leave {left} {} instead of {}.\n", + plural(left, "hole", "holes"), self.holes - )); - } + ), + }); } out } diff --git a/src/render_tests.rs b/src/render_tests.rs index 085d762..93392bb 100644 --- a/src/render_tests.rs +++ b/src/render_tests.rs @@ -2385,7 +2385,15 @@ fn the_emptiest_placement_is_the_one_suggested() { } } } - let (best, holes) = best_start_week(&grid, columns.len(), 1, &columns, &busy, 0).unwrap(); + // The first column's own Sunday as today, so every column is still ahead + // and the tie-break in `chosen` reduces to "the earliest" — which is what + // these assertions are about. Note it is *not* `grid.first`: column 0 + // starts in the December before the year, and using Jan 1 here made the + // sweep skip past a perfectly good week 0. The preference for a column + // that has not begun has a test of its own below. + let today = grid.date_at(0, 0); + let (best, holes) = + best_start_week(&grid, columns.len(), 1, &columns, &busy, 0, today).unwrap(); assert!( best >= 6, "it should clear the busy stretch, got week {best}" @@ -2394,13 +2402,21 @@ fn the_emptiest_placement_is_the_one_suggested() { // With nothing in the way, the earliest placement wins — a stable answer // rather than an arbitrary one. - let (best, holes) = - best_start_week(&grid, columns.len(), 1, &columns, &Default::default(), 0).unwrap(); + let (best, holes) = best_start_week( + &grid, + columns.len(), + 1, + &columns, + &Default::default(), + 0, + today, + ) + .unwrap(); assert_eq!((best, holes), (0, 0)); // Text that cannot fit has no placement at all. let wide = art::bitmap("ABCDEFGHIJ").unwrap(); - assert!(best_start_week(&grid, wide.len(), 1, &wide, &Default::default(), 0).is_none()); + assert!(best_start_week(&grid, wide.len(), 1, &wide, &Default::default(), 0, today).is_none()); } // ---------------------------------------------------------------- untrusted input @@ -3153,13 +3169,31 @@ fn a_background_lets_the_letters_land_where_a_bare_graph_could_not() { date = date.succ_opt().unwrap(); } - let (_, bare_holes) = best_start_week(&grid, columns.len(), 1, &columns, &busy, 0).unwrap(); + let (_, bare_holes) = best_start_week( + &grid, + columns.len(), + 1, + &columns, + &busy, + 0, + grid.date_at(0, 0), + ) + .unwrap(); assert!( bare_holes > 0, "on a bare graph every quiet day inside the block is a hole" ); - let (_, hidden) = best_start_week(&grid, columns.len(), 1, &columns, &busy, 1).unwrap(); + let (_, hidden) = best_start_week( + &grid, + columns.len(), + 1, + &columns, + &busy, + 1, + grid.date_at(0, 0), + ) + .unwrap(); assert_eq!( hidden, 0, "a level-1 background is where those days belong, so nothing is a hole" @@ -4423,3 +4457,162 @@ mod editor { assert_eq!(editor.canvas.at(0, 0), 3, "and the paint landed"); } } + +// ------------------------------------------------------- suggesting a placement +// +// `holed` is the only verdict a reader cannot act on by contributing more, so +// it is the one that most owes them a next move. Until 0.8.0 the picture path +// passed `None` for the suggestion and printed the diagnosis alone — which is +// the shape of plan most able to move, because a picture narrower than the +// year has columns to move *to*. Issue #97, found on a live plan. + +/// The canvas equivalent of `the_emptiest_placement_is_the_one_suggested`. +#[test] +fn a_picture_is_offered_the_column_that_draws_it_cleanly() { + use crate::art::{Canvas, Grid}; + use crate::plan::{best_start_week_of, Plan}; + use std::collections::BTreeMap; + + let grid = Grid::new(2027).unwrap(); + // Five columns, both shades used, so a hole can be either "bright where it + // must be dark" or "brighter than the shade it is drawn at". + let canvas = Canvas::parse("# name: Blip\n04040\n40404\n04040\n40404\n04040\n40404\n04040\n") + .expect("a canvas"); + assert_eq!(canvas.width(), 5); + + // A busy stretch at the start of the year, and nothing after it. + let mut busy = BTreeMap::new(); + for week in 0..8 { + for row in 0..7 { + let date = grid.date_at(week, row); + if grid.holds(date) { + busy.insert(date, 200); + } + } + } + + let today = grid.date_at(0, 0); + let (week, holes) = best_start_week_of(&canvas, &grid, &busy, today).expect("a placement"); + assert!( + week >= 8, + "it should clear the busy stretch, got week {week}" + ); + assert_eq!(holes, 0, "and land somewhere with no holes at all"); + + // The suggestion has to agree with the verdict beside it, which is the + // whole reason the sweep builds real plans rather than reimplementing the + // test: taking the advice must actually produce a plan with no holes. + let (levels, skipped) = canvas.place(&grid, week); + assert_eq!(skipped, 0, "and lose no part of the picture off the end"); + let moved = Plan::from_levels("Blip", &grid, &levels, week, canvas.width(), &busy); + assert_eq!(moved.holes().len(), 0, "the advice has to be true"); + + // Where it sits now, it is not. + let (here, _) = canvas.place(&grid, 0); + let stuck = Plan::from_levels("Blip", &grid, &here, 0, canvas.width(), &busy); + assert!( + !stuck.holes().is_empty(), + "the busy stretch has to hole the plan, or this proves nothing" + ); +} + +/// The old reasoning, kept — but enforced by measuring the overhang rather +/// than by declining to look. +#[test] +fn a_full_width_picture_is_still_offered_nothing() { + use crate::art::{Canvas, Grid, CANVAS_COLS}; + use crate::plan::best_start_week_of; + + let grid = Grid::new(2027).unwrap(); + let mut body = String::from("# name: Wide\n"); + for _ in 0..7 { + body.push_str(&"4".repeat(CANVAS_COLS)); + body.push('\n'); + } + let canvas = Canvas::parse(&body).expect("a canvas"); + + // Every column of a full-width picture is ink, so the partial weeks at the + // ends of the year drop some of it wherever it is put. Nothing to suggest, + // which is exactly what the call site used to assume without checking. + assert!( + best_start_week_of(&canvas, &grid, &Default::default(), grid.date_at(0, 0)).is_none(), + "a picture as wide as the year has nowhere to go" + ); +} + +/// A clean column in March is arithmetic, not advice. +#[test] +fn a_placement_that_has_already_begun_loses_a_tie() { + use crate::art::{Canvas, Grid}; + use crate::plan::best_start_week_of; + + let grid = Grid::new(2027).unwrap(); + let canvas = Canvas::parse("# name: Bar\n444\n444\n444\n444\n444\n444\n444\n").expect("canvas"); + + // An empty year: every placement costs zero holes, so every placement ties + // and only the tie-break decides. + let empty = Default::default(); + + // Asked in December of the year before, the earliest column wins. Week 1 + // rather than week 0: the first calendar column is a partial week, so an + // all-ink picture placed there loses cells off the top of the year and is + // not a candidate at all. + let (early, _) = best_start_week_of(&canvas, &grid, &empty, grid.date_at(0, 0)).unwrap(); + assert_eq!( + early, 1, + "with the whole year ahead, start as early as it fits" + ); + + // Asked in the middle of the year, a column in March is still clean and + // still useless: the only way to draw there is to back-date into days that + // have gone. The answer has to be a column that has not begun. + let midyear = grid.date_at(30, 0); + let (later, holes) = best_start_week_of(&canvas, &grid, &empty, midyear).unwrap(); + assert_eq!(holes, 0, "the year is empty, so nothing is ever a hole"); + assert_eq!( + later, 30, + "the earliest column that has not started, not the earliest column" + ); + assert!( + grid.date_at(later, 0) >= midyear, + "and it genuinely has not started" + ); +} + +/// Holes still win outright — the preference for a future column only breaks +/// ties, because back-dating is a thing this tool does and unlighting a day is +/// not. +#[test] +fn a_cleaner_placement_beats_a_later_one() { + use crate::art::{Canvas, Grid}; + use crate::plan::best_start_week_of; + use std::collections::BTreeMap; + + let grid = Grid::new(2027).unwrap(); + // A dark middle column, so a contribution there is a hole. An all-ink + // picture would prove nothing: `4` wants the year's peak, and a day + // holding less than that is *short*, which is a thing you fix by + // contributing — not a hole. + let canvas = Canvas::parse("# name: Bar\n404\n404\n404\n404\n404\n404\n404\n").expect("canvas"); + + // Everything from week 20 on is lightly lit, so no column from there on is + // clean. Weeks 0..20 are empty, and by week 30 they are all in the past. + let mut busy = BTreeMap::new(); + for week in 20..grid.weeks { + for row in 0..7 { + let date = grid.date_at(week, row); + if grid.holds(date) { + busy.insert(date, 1); + } + } + } + + let midyear = grid.date_at(30, 0); + let (week, holes) = best_start_week_of(&canvas, &grid, &busy, midyear).unwrap(); + assert_eq!(holes, 0, "the empty first half draws it with no holes"); + assert!( + grid.date_at(week, 0) < midyear, + "even though that means a column that has already begun — \ + fewer holes beats sooner, got week {week}" + ); +} diff --git a/tests/art_cli.rs b/tests/art_cli.rs index 86d17d1..a351518 100644 --- a/tests/art_cli.rs +++ b/tests/art_cli.rs @@ -2639,3 +2639,135 @@ fn the_documented_reports_still_read_the_way_the_docs_print_them() { "a parenthesized plural is back in the docs" ); } + +/// Issue #97: a picture that is holed has to be told where to go. +/// +/// This is a CLI test rather than a unit one on purpose. The defect was a +/// single argument at a call site — `Report::of(.., None)` on the picture path +/// while the text path passed a real sweep — so every unit test of the sweep +/// itself passed while the tool said nothing. The only layer that could have +/// caught it is the one a user sees. +#[test] +fn a_holed_picture_is_told_where_it_can_be_drawn() { + // Five columns in a fifty-three column year, so there is somewhere to go. + // The old comment at the call site said a picture "is usually the full + // width of the year"; docs/ART.md asks contributors for exactly this shape. + let path = scratch("blip.art"); + std::fs::write( + &path, + "# name: Blip\n04040\n40404\n04040\n40404\n04040\n40404\n04040\n", + ) + .expect("the scratch file is writable"); + let art_path = path.to_string_lossy().into_owned(); + + let run = |format: &str| { + let out = art(&[ + "--matrix", + &art_path, + "--year", + "2026", + "--start-week", + "34", + "--track", + "--merge", + "art/vyncint-2026.json", + "--no-colour", + "--today", + "2026-08-19", + "--format", + format, + ]); + assert!( + out.status.success(), + "{format}: {}", + String::from_utf8_lossy(&out.stderr) + ); + stdout(&out) + }; + + let text = run("text"); + assert!(text.contains("Cannot be drawn cleanly"), "{text}"); + assert!( + text.contains("--start-week 41 draws it cleanly."), + "the diagnosis without the way out is the bug:\n{text}" + ); + + // Every format carries it, because the shipped consumer reads markdown and + // json and never sees the screen. + let markdown = run("markdown"); + assert!( + markdown.contains("`--start-week 41` draws it cleanly."), + "{markdown}" + ); + + let json = run("json"); + assert!(json.contains("\"suggested_start_week\": 41"), "{json}"); + assert!(json.contains("\"suggested_holes\": 0"), "{json}"); + // The headline is the Action's own output and the subject of the issue it + // opens, so it is where "this year is lost" was actually being published. + assert!( + json.contains("week 41 draws it"), + "the headline has to carry it too:\n{json}" + ); + + // Taking the advice has to work: the suggested column draws it with no + // holes at all, or the advice is worse than silence. + let moved = art(&[ + "--matrix", + &art_path, + "--year", + "2026", + "--start-week", + "41", + "--track", + "--merge", + "art/vyncint-2026.json", + "--no-colour", + "--today", + "2026-08-19", + "--format", + "json", + ]); + let moved = stdout(&moved); + assert!(moved.contains("\"holes\": 0"), "{moved}"); + assert!(moved.contains("\"verdict\": \"reachable\""), "{moved}"); + + let _ = std::fs::remove_file(&path); +} + +/// A picture that already draws cleanly is offered nothing — there is nothing +/// to offer, and a suggestion beside `reachable` reads as a correction. +#[test] +fn a_picture_on_track_is_not_told_to_move() { + let path = scratch("blip-clean.art"); + std::fs::write( + &path, + "# name: Blip\n04040\n40404\n04040\n40404\n04040\n40404\n04040\n", + ) + .expect("the scratch file is writable"); + + let out = art(&[ + "--matrix", + &path.to_string_lossy(), + "--year", + "2026", + "--start-week", + "41", + "--track", + "--merge", + "art/vyncint-2026.json", + "--no-colour", + "--today", + "2026-08-19", + "--format", + "markdown", + ]); + let text = stdout(&out); + assert!(text.contains("**On track**"), "{text}"); + assert!( + !text.contains("--start-week"), + "nothing to suggest, so nothing said:\n{text}" + ); + + let _ = std::fs::remove_file(&path); +} From d96029fb5223c78276d63a82677121cb8f57658b Mon Sep 17 00:00:00 2001 From: Vyncint Ng <115854244+vyncint@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:09:47 +0700 Subject: [PATCH 2/2] docs: describe the tie-break instead of linking a private fn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo doc` runs with `-D warnings`, and a public item's documentation cannot link to a private one — `best_start_week` is public, `chosen` is not. Saying what the tie-break does reads better than a link to a name a reader of the public docs cannot follow anyway. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com> --- src/plan.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plan.rs b/src/plan.rs index dcaf952..8437612 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -505,8 +505,10 @@ fn tally<'a>(days: impl Iterator) -> (usize, u32) { /// contribution in there is a hole, higher when the plan draws a background /// those days can hide in. /// -/// `today` decides ties, through [`chosen`]: among columns that cost the same -/// it prefers one that has not begun. +/// `today` decides ties: among columns that cost the same number of holes, one +/// that has not begun yet wins. A clean column in March is arithmetic, not +/// advice — the only way to draw there is by back-dating into days that have +/// gone. pub fn best_start_week( grid: &Grid, columns: usize,