fix: offer a picture the column that draws it cleanly - #98
Merged
Conversation
`--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>
`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>
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.
Closes #97.
--trackon a picture passedNonewhere 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.
Measured on the plan that reported it —
vyncint/contribution-art'seleven-column heart, live data,
--today 2026-09-06:Cannot be drawn cleanly — 5 days are brighter than the picture wants, and nothing takes a contribution away. + --start-week 37 draws it cleanly.and the issue's own reproduction:
suggested=nullsuggested=41, 0 holessuggested=nullsuggested=41, 0 holessuggested=nullsuggested=41, 0 holesThe call site
The comment there said 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 —
dragonis 53columns,
wave/pulse/invaderare 51 — and wrong about every picturenarrower than the year, which is the shape
docs/ART.mdand #57 askcontributors to draw.
The overhang it was guarding against is now measured by the sweep rather
than assumed at the call site: a placement that pushes lit cells out of the
year is not a candidate, so a picture with ink to its edges still gets nothing
— for the reason, instead of by refusing to look. (Blank margins don't count,
so
dragonstill places; it just never has anywhere better to go.)Why a canvas needs its own sweep
Text has one shade — a day is inside a letter or it is 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. A
[[bool; 7]]cannot express that.Rather than reimplement the test,
best_start_week_ofbuilds each candidateinto a real
Planand asks for its ownholes()— the same callReportmakes for the number it publishes. A suggestion that disagreed with the verdict
printed beside it would be worse than no suggestion. It costs one plan per
column, at most 53, in a run that talks to the GitHub API first.
Two things that came out of testing it
A suggestion has to be a column you can still start in. Ranked by holes then
by column, the first answer for the heart was week 11 — clean, correct, and
in March. That year has nine placements costing zero holes, and the leftmost is
always the one furthest in the past; the only way to draw there is
--backfillinto days five months gone. Among columns that tie, one that has not begun now
wins, which makes the answer week 37.
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.
Text needed this too: the example in #97 suggests
--start-week 1in August.A clean column deserves to be described as one.
The
holedheadline carries it as well, which is where it was most missing —that line is the Action's
headlineoutput and the subject of the issue theshipped consumer opens, so it is where "this year is lost" was actually being
published:
Action
New
suggested-start-weekandsuggested-holesoutputs, so a workflow can acton the answer rather than only print it.
Tests
The CLI test is the one that matters. The defect was a single argument at a
call site — every unit test of the sweep passed while the tool said nothing —
so the only layer that could have caught it is the one a user sees.
a_holed_picture_is_told_where_it_can_be_drawn— text, markdown and json allcarry it, the headline carries it, and taking the advice actually works:
re-running at the suggested column reports
holes: 0.a_picture_on_track_is_not_told_to_move— nothing to offer, nothing said.a_picture_is_offered_the_column_that_draws_it_cleanlya_full_width_picture_is_still_offered_nothinga_placement_that_has_already_begun_loses_a_tiea_cleaner_placement_beats_a_later_oneBreaking
plan::best_start_weektakes atoday. Library-only; the binaries and theAction are unaffected. Listed under Changed in the changelog, per the 0.x
rule at the top of the file.