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..8437612 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -504,6 +504,11 @@ 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: 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, @@ -511,12 +516,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 +538,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 +1026,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 +1068,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 +1217,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); +}