Repository navigation
Lint the Python scripts and test teams-copy in CI - #180
Conversation
197f2fd to
d88344e
Compare
Document Review: 3 files on lint-and-test-python (
|
| Scope | The changes on lint-and-test-python since a41d887 (origin/main), with the branch's commit message and the body of pull request #180 |
| Branch | lint-and-test-python |
| Reviewed | 2026-10-24 |
| Reviewed by | Opus 5.5 (claude-opus-5-5) — code-best-practices-reviewer, test-suite-architect and documentation-expert |
| Collated by | Opus 5.5 (claude-opus-5-5) |
Documents
| Document | Commit |
|---|---|
.github/workflows/ci.yml |
ef29aa6 |
.ruff.toml |
ef29aa6 |
test/test-teams-copy.sh |
ef29aa6 |
| commit message | ef29aa6 |
| PR body | #180 at ef29aa6 |
Review History
2026-10-24 — Initial review by Opus 5.5 (claude-opus-5-5)
The branch changes no document files, so /doc-review's scope filter would have found nothing to review. This file follows its shape, with three expert reviewers in place of one documentation-expert per document: code-best-practices-reviewer on ci.yml and .ruff.toml, test-suite-architect on test/test-teams-copy.sh and documentation-expert on the prose (comments, case descriptions, commit message and PR body). The code findings use two categories /doc-review lacks, Correctness and Test Coverage. The orchestrator collated the reports itself rather than through a collator agent, and re-ran the checks marked under Verification.
2026-10-24 — F6 ruled and settled by Opus 5.5 (claude-opus-5-5) (F6 fixed; F25 added)
F6 was ruled (b). The debug re-run of CI run 40123162318 showed super-linter classifies teams-copy as HTML and lints only unfilled-blocks.py, which F25 records; G7 opens for it ahead of G3.
2026-10-24 — Fixes applied by Opus 5.5 (claude-opus-5-5) (F1–F5, F7–F14, F17–F21 and F25 fixed)
F25 ruled (a). Every Implement finding was fixed in the amended commit e53efd9 (was ef29aa6) and the edited PR body; each test finding was checked against the mutant it names in a scratch copy, and CI run 40125228908 passed on e53efd9. F15, recommended Defer, stays open. The Documents table keeps ef29aa6, the revision the reviewers read.
2026-10-24 — Follow-up by Opus 5.5 (claude-opus-5-5) (F25's fix revised)
F25's step now runs Ruff from super-linter's image rather than a separately pinned pipx install, and the expect calls in test/test-teams-copy.sh were refilled to 100 columns at the user's request; both landed in 197f2fd (was e53efd9), and CI run 40125954265 passed on it.
2026-10-24 — F15 ruled by Opus 5.5 (claude-opus-5-5) (F15 deferred)
During /ship-it, the user ruled F15 Defer. The branch's commits were squashed into d88344e (was 197f2fd, plus a comment refill), so line references may have drifted.
Overall Assessment
The CI change itself is sound: super-linter v8.7.0 resolves the root .ruff.toml, pins Ruff 0.15.17 and passed on ef29aa6, and no README, CLAUDE.md or skill describes CI's steps, so nothing is stale. The findings fall into three clusters. First, the new teams-copy test proves less than it appears to: one case passes with the protection it names removed, one depends on case order, and the file-path entry point, several list and heading forms and some error paths have no case (G3, G4, G5). Second, the commit message and PR body overstate what ruff format would change and mislabel the mutation that was run; the commit message becomes permanent at merge (G2). Third, and most important, F6's debug re-run showed that super-linter never lints teams-copy — Ubuntu's file classifies it as HTML — so the pull request's central "no extra step" claim is false; F25 was ruled (a) and fixed with an explicit Ruff step. Every Implement finding is now fixed in 197f2fd; F15 (--help) was deferred by the user.
Formatting
F1 🟢 Low Priority - The commit body is not refilled to 72 columns ✅ Fixed
- Document — commit message
- Location — paragraph 1, line 2; paragraph 2, line 5; paragraph 3, line 1
- Issue — The body wraps short of the 72 columns
/commitsets: "detector" fits on paragraph 1's second line (62 columns today), "Both" on paragraph 2's fifth and "the" on paragraph 3's first. The user-wide rule is to refill so every line takes as many words as fit. - Suggestion — Refill the three paragraphs greedily at 72, keeping
`ruff format`on one line, and check withLC_ALL=en_US.UTF-8 grep -n '.\{73,\}'. - Recommendation — Implement — line breaks only, and cheap before the message becomes permanent
- Verification —
git log -1 --format=%B | awk '{ print length }': line 2 is 62 columns, confirmed - Status — ✅ Fixed: commit message refilled greedily at 72 columns with Python's
textwrap;LC_ALL=en_US.UTF-8 grep -n '.\{73,\}'prints nothing one53efd9
F2 🟢 Low Priority - The mention case's line is 226 columns and writes the MRI twice ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — lines 86–88
- Issue — Line 88 is 226 columns, the longest in any of the repository's test scripts, and the string can be split, so the exemption for unsplittable strings does not apply. The same MRI literal appears on lines 86 and 88.
- Suggestion — Set
mri=8:orgid:00000000-0000-0000-0000-000000000000once, use it in theprintfthat writes the people file and build the expected markup in a variable across lines. - Recommendation — Implement — brings the line under 100 columns and removes the duplication
- Verification —
awk 'length > 100' test/test-teams-copy.sh: line 88, 226 columns, confirmed - Status — ✅ Fixed: a
mention()helper builds the markup frommriandreadonly_tag, set once;awk 'length > 100'finds no line
Consistency
F3 🟢 Low Priority - The .ruff.toml comment says "Matches" where the sibling lint configs use the imperative ✅ Fixed
- Document —
.ruff.toml - Location — line 1
- Issue —
.yaml-lint.ymlsays "Match the 100 columns this repo wraps at everywhere else" and.markdown-lint.ymlsays "Wrap prose at 100 columns"; the new file opens "Matches the repository's 100-column limit." - Suggestion — Open with "Match the repository's 100-column limit".
- Recommendation — Implement — one word aligns the three configs
- Status — ✅ Fixed: the comment now reads "Match the repository's 100-column limit"
Accuracy
F4 🟡 Medium Priority - "ruff format would rewrite much of both files" overstates its effect ✅ Fixed
- Document — commit message
- Location — commit paragraph 2; PR Summary, last bullet
- Issue —
ruff format --diff(0.15.17) removes 44 ofteams-copy's 233 lines, all quote-style changes, but only 10 ofunfilled-blocks.py's 388, where it explodes hand-wrapped conditions and tuples one item per line. "Much of both files" is false for the second, and the real reason to keep the formatter off is that its style conflicts with the scripts' hand-wrapped style, not the volume. - Suggestion — Reword both texts to the measured effect, e.g. "
ruff formatwould restyle both scripts, requoting all of teams-copy's strings and splitting hand-wrapped conditions one item per line". - Recommendation — Implement — the claim is checkable and wrong for one file, and the commit message is permanent once merged
- Concurred by — code-best-practices-reviewer, on the same two texts
- Verification —
uvx ruff@0.15.17 format --no-cache --diff <file> | grep -c '^-[^-]': 44 and 10, confirmed - Status — ✅ Fixed: the commit message and PR body now say
ruff formatwould restyle both scripts, requoting teams-copy's strings and re-wrapping the detector's hand-wrapped conditions
F5 🟢 Low Priority - The ticked mutation item names the wrong part of the spacer rule ✅ Fixed
- Document — PR body
- Location — Test plan, item 2
- Issue — The item says "with the spacer rule removed, 3 cases fail". The mutation run replaced
if not (is_list(previous) or is_list(block))withif True, which removes the list exception to the spacer rule; removing the spacer itself fails 2 different cases. - Suggestion — "…with the list exception to the spacer rule removed, 3 cases fail".
- Recommendation — Implement — a ticked item's evidence should name exactly what was run
- Verification — The orchestrator's own mutation command in this session used
sed 's/if not (is_list(previous) or is_list(block)):/if True:/', confirmed - Status — ✅ Fixed: the PR's mutation item now lists each mutant that was run
F6 ⚖️ Decision - How to settle the open CI item in the PR's test plan ✅ Fixed
- Document — PR body
- Location — Test plan, item 5 ("CI's super-linter run lists
teams-copyamong the files it hands to Ruff") - Issue — At its default log level super-linter lists no per-linter files, and the "Found Python script without extension" message drops to debug inside GNU parallel, so CI run 40123162318 on
ef29aa6shows only "Successfully linted PYTHON_RUFF". The routing is sound on source reading —CheckFileTypeinlib/functions/buildFileList.shmatches*"Python script"*and adds the file to the Ruff array, andfile --briefreportsteams-copyas a Python script — but no CI output yet showsteams-copyreaching Ruff, and the item as written can never be ticked. - Options
- (a) Reword to the source evidence and tick it. No further work, but the evidence is source reading rather than a run.
- (b) Re-run CI with debug logging (
gh run rerun 40123162318 --repo mattmenefee/dotfiles --debug) and grep forwithout extension: .*teams-copy, then reword the item to that evidence. No commit, but it is unverified whetherRUNNER_DEBUGreaches super-linter's container. - (c) Push a throwaway 101-column line into
teams-copy, confirmPYTHON_RUFFfails, then revert. Proves the routing end to end, at the cost of two commits on the branch (squashable). - (d) Leave the box open. No cost now, but
/ship-itholds it open indefinitely.
- Recommended — (a) per documentation-expert, since the source and
fileoutput fully determine the routing; code-best-practices-reviewer recommends settling it empirically, (b) then (c), because it is the pull request's central "no extra step" claim. - Divergence — documentation-expert filed it as a ⚖️ recommending (a); code-best-practices-reviewer filed it as 🟡 Medium, Implement, recommending a debug re-run or a throwaway commit.
- Status — ✅ Fixed: ruled (b), "Debug re-run".
gh run rerun 40123162318 --debugreached super-linter's container and settled the item in the negative: Ubuntu'sfilereportsteams-copyas "HTML document, Unicode text, UTF-8 text",CheckFileTypelogs "Failed to get file type" and the run logs "There are 1 items to lint for PYTHON_RUFF: …/unfilled-blocks.py". The defect this exposes is F25.
Correctness
F7 🟡 Medium Priority - The link-address case passes with link addresses unprotected ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — lines 80–81, "asterisks in a link address are not italics"
- Issue — In
a*b*ceach*borders a word character, so ITALIC's lookarounds already reject it. Removingkeep()from the<a href>opening ininline()still passes the suite. - Suggestion — Use
'See [docs](https://x.y/*a*/b) now\n', expecting'<p>See <a href="https://x.y/*a*/b">docs</a> now</p>'; the mutant rendershref="https://x.y/<i>a</i>/b". - Recommendation — Implement — the case gives false confidence today
- Verification — Mutant in a scratch copy:
a*b*crenders unchanged,*a*/brenders<i>a</i>inside the address, confirmed - Status — ✅ Fixed: the case uses
https://x.y/*a*/b; the link-protection mutant now fails it
F8 🟡 Medium Priority - The empty-draft case inherits a malformed people file, and lazy loading is never tested by name ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — lines 92–98
- Issue — Line 92 leaves malformed JSON in the people file, so "an empty draft stops" runs against it. Making the script load the file eagerly fails only that unrelated case, so the docstring's "Without the file, everything but mentions still works" is covered by case order alone.
- Suggestion — After the malformed-file case, add
expect "a draft without mentions ignores a malformed people file" '<p>Plain</p>' 'Plain\n', thenrm -f "${people}". - Recommendation — Implement — tests the lazy-load contract by name and removes the order dependence
- Status — ✅ Fixed: added "a draft without mentions ignores a malformed people file" and
rm -f -- "${people}"after it; the eager-load mutant fails that case
F25 ⚖️ Decision - Super-linter never hands teams-copy to Ruff in CI ✅ Fixed
- Document —
.github/workflows/ci.yml - Location — the
Run the lintersstep,VALIDATE_PYTHON_RUFF - Issue — Super-linter classifies an extensionless file by
file --brief. macOS'sfilecallsteams-copya Python script, but the libmagic in super-linter's image reports "HTML document", most likely from the HTML string literals in the script, soCheckFileTypegives up and the file joins no Python array. Ruff lints onlyunfilled-blocks.py, and the pull request's central claim —teams-copyis linted alongside it with no extra step — is false. Found by F6's debug re-run. - Options
- (a) Add a CI step that runs Ruff on
teams-copyby path, e.g.pipx run ruff==0.15.17 check home/.local/bin/teams-copy, reading the same.ruff.toml. Covers the file now; the pinned version must be bumped by hand to match super-linter's, or the two can disagree on default rules. - (b) Drop
teams-copyfrom the linting claim and lint only.pyfiles, correcting the commit message and PR body. No new machinery, but the untested-by-lint file stays the one with no extension. - (c) Rename the script to
teams-copy.pyso super-linter routes it by extension. Fully automatic, but it renames the command on PATH, and the user-wide CLAUDE.md and the docstring nameteams-copy.
- (a) Add a CI step that runs Ruff on
- Recommended — (a), because
teams-copyis the larger and more often changed script and a one-line step with a pinned version mirrors how this workflow already gives zsh files the syntax check super-linter cannot. - Verification —
gh run view 40123162318 --repo mattmenefee/dotfiles --logafter the debug re-run: "Detected file type for …/teams-copy: HTML document", "Failed to get file type for …/teams-copy" and "There are 1 items to lint for PYTHON_RUFF: …/unfilled-blocks.py", confirmed - Status — ✅ Fixed: ruled (a), "Explicit Ruff step".
ci.ymlgains "Lint teams-copy with Ruff", which runs the Ruff inside super-linter's own image onteams-copy, with the image tag read from the super-linter step'suses:line so the versions cannot drift (replacing a firstpipx run --spec ruff==0.15.17version that Dependabot could not keep in step); CI run 40125954265 on197f2fdreported "All checks passed!"
Test Coverage
F9 🟡 Medium Priority - Reading a draft from a path is never tested, nor a missing file ✅ Fixed
- Document —
test/test-teams-copy.sh - Location —
run(), line 31, and the Input section - Issue — Every case pipes the draft through stdin, yet
teams-copy draft.mdis the main documented usage. Replacingread_draft's path branch withsys.stdin.read()survives the suite, and theOSErrorbranch has no case. - Suggestion — Pass extra arguments through
run()as the siblingexpect_rundoes with"${@:5}", then add "a draft is read from a path" and "a draft that does not exist stops" (No such file or directory). - Recommendation — Implement — covers the main entry point and an error branch at little cost
- Status — ✅ Fixed:
run()passes extra arguments through; "a draft is read from a path" and "a draft that does not exist stops" added, and the path-ignored mutant fails both
F10 🟡 Medium Priority - The + and * bullets, the 1) marker and heading levels 2–6 are untested ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — Lists section (lines 63–71) and the heading case (lines 58–59)
- Issue — Dropping
+from[-*+], dropping)from[.)]and narrowing#{1,6}to#{1}each survive the suite;* itemas a list and#######falling back to a paragraph are untested. - Suggestion — Add cases for
+ a\n* b\n,1) a\n2) b\n,## T\n\n###### U\nand####### T\n. - Recommendation — Implement — each is in the docstring's Supported list or LIST_MARKERS
- Status — ✅ Fixed: four cases added; the
+,)and#{1}mutants each fail one
F11 🟢 Low Priority - Unescaping & in link addresses and mention names is untested ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — Inline markup and Mentions sections
- Issue — Dropping
html.unescapeinlink()double-escapes?a=1&b=2to&amp;; dropping it inmention()makes@[A&B]fail its lookup. Both survive. - Suggestion — Add
expect "an & in a link address is escaped once" '<p><a href="https://x.y/?a=1&b=2">l</a></p>' '[l](https://x.y/?a=1&b=2)\n'and an"A&B"key with a case expectingaria-label="A&B">A&B</mention>. - Recommendation — Implement — double escaping is the classic failure in an escape-first design
- Status — ✅ Fixed: link-address and
A&Bmention cases added; both unescape mutants fail
F12 🟢 Low Priority - Mentions inside code spans and NUL stripping are untested ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — Inline markup section
- Issue — Moving the mention substitution ahead of code-span protection makes
`@[Sam]`abort instead of rendering<code>@[Sam]</code>; removingtext.replace('\0', '')lets a NUL-0-NUL draft paste in a copy of the first code span. Both survive. - Suggestion — Add
expect "a mention inside a code span stays literal" '<p><code>@[Sam]</code></p>' '`@[Sam]`\n'and, optionally,expect "a NUL in the draft cannot inject kept markup" '<p><code>x</code> 0</p>' '`x` \00000\0000\n'. - Recommendation — Implement the code-span case; Defer the NUL case, which guards against a hostile draft rather than a likely one
- Status — ✅ Fixed: both cases added, NUL included; the mention-before-code-span and NUL-strip mutants each fail one
F13 🟢 Low Priority - Spaced asterisks are not tested to stay literal ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — "bold and italics", lines 74–75
- Issue — Removing ITALIC's
(?=\S)and(?<=\S)guards survives;2 * 3 * 4would render2 <i> 3 </i> 4. - Suggestion — Add
expect "spaced asterisks are not italics" '<p>2 * 3 * 4</p>' '2 * 3 * 4\n'. - Recommendation — Implement — one line for a regex that is easy to break
- Status — ✅ Fixed: "spaced asterisks are not italics" added; the guard-removal mutant fails it
F14 🟢 Low Priority - The people file's shape check is untested ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — Mentions section, after line 94
- Issue — Replacing the
isinstancecheck withif False:survives; a list or a numeric id then crashes with a traceback instead of "must be a JSON object". - Suggestion — Add
printf '{"Alex": 1}\n' >"${people}"andexpect_error "a people file with a non-string id stops" 'must be a JSON object' 'Hi @[Alex]\n'. - Recommendation — Implement — the only error path with no case
- Status — ✅ Fixed: "a people file with a non-string id stops" added; the
if False:mutant fails it
F15 🟢 Low Priority - --help is untested ⏸️ Deferred
- Document —
test/test-teams-copy.sh - Location — whole file
- Issue — The user-wide CLAUDE.md sends agents to
teams-copy --helpfor the supported Markdown, and nothing checks that it exits 0 and prints the Supported list. - Suggestion — Add one assertion that
--helpoutput contains^Supported:, incrementingn. - Recommendation — Defer — breaking it is unlikely; add it when the CLI options change
- Status — ⏸️ Deferred: ruled Defer by the user — add a
--helpcase when the CLI options change
F16 ℹ️ Observation - The stdin-is-a-terminal error cannot reasonably be tested in CI
- Document —
test/test-teams-copy.sh - Location —
parse_args'ssys.stdin.isatty()branch - Issue — Testing it needs a pseudo-terminal, and
scripttakes different arguments on BSD and util-linux; the branch is argparse's ownparser.error, exit 2, so a portable case would cost more than it protects.
Clarity and Structure
F17 🟢 Low Priority - expect neither checks the exit status nor shows stderr on failure ✅ Fixed
- Document —
test/test-teams-copy.sh - Location —
expect(), lines 35–40 - Issue — A crash shows in CI as an empty
actual:; the traceback goes to${dir}/stderr, which the EXIT trap deletes. A run that prints correct HTML but exits nonzero would pass. - Suggestion — Capture
rc=$?and require 0, asexpect_rundoes in the sibling test, and print the stderr file on any mismatch. - Recommendation — Implement — makes CI failures diagnosable without a local rerun
- Status — ✅ Fixed:
expectrequires exit status 0 and prints the stderr file on any mismatch; a crash mutant now shows its traceback in the failure
F18 🟢 Low Priority - The header's "control which one exists" misdescribes the mention cases ✅ Fixed
- Document —
test/test-teams-copy.sh - Location — lines 8–9
- Issue — "which one exists" suggests a choice among several people files; the cases decide whether the one file exists and what it holds.
- Suggestion — "…so the real people file is never read and the mention cases decide whether one exists and what it holds", then refill the comment.
- Recommendation — Implement — one-line wording fix
- Concurred by — documentation-expert, with the same rewording
- Status — ✅ Fixed: the header now says the mention cases "decide whether one exists and what it holds"
F19 🟢 Low Priority - The E501 explanation sits above line-length rather than the line it explains ✅ Fixed
- Document —
.ruff.toml - Location — lines 1–2
- Issue — The second sentence justifies
extend-select = ["E501", …]four lines further down. The first sentence alone is a single phrase and would take no final period. - Suggestion — Split it: a phrase above
line-length, and "Ruff's default rules leave out line length, so E501 is selected explicitly" directly aboveextend-select. - Recommendation — Implement — keeps each reason beside its setting
- Status — ✅ Fixed: split into a phrase above
line-lengthand the E501 reason directly aboveextend-select
F20 🟢 Low Priority - "teams-copy was never run at all" reads as absolute ✅ Fixed
- Document — commit message
- Location — paragraph 1, last sentence
- Issue — The paragraph is about CI, but out of context the sentence says the tool never ran anywhere.
- Suggestion — "teams-copy never ran in CI at all".
- Recommendation — Implement — a few words in permanent text
- Concurred by — code-best-practices-reviewer, with the same rewording
- Status — ✅ Fixed: the commit message now says "teams-copy never ran in CI at all"
F21 🟢 Low Priority - The reason for the top-level test directory leaves out its premise ✅ Fixed
- Document — commit message
- Location — last sentence of the commit body; the paragraph after the PR's Summary bullets
- Issue — "because homesick links everything under home/.local/bin onto PATH" skips that the obvious home for the test is beside
teams-copy. - Suggestion — "The test sits in a top-level test directory rather than beside teams-copy, because homesick links each file in home/.local/bin into ~/.local/bin, which is on PATH."
- Recommendation — Implement — states the premise the reason depends on
- Status — ✅ Fixed: the commit message and PR body now say the test sits "rather than beside teams-copy, because homesick links each file in home/.local/bin into ~/.local/bin, which is on PATH"
F22 ℹ️ Observation - A backslash in a draft must be doubled
- Document —
test/test-teams-copy.sh - Location —
run(), line 31 - Issue — Drafts pass through
printf '%b', so a Markdown backslash is written\\and\csilently truncates the draft. No current draft holds one, and theexpectcomment already says "with printf %b escapes", as the sibling test does.
F23 ℹ️ Observation - Ruff's cache now lands in the CI workspace
- Document —
.ruff.toml - Location — whole file
- Issue — A root
.ruff.tomlreplaces super-linter's bundled template, which setcache-dir = "/tmp/.ruff_cache", so Ruff writes.ruff_cachebeside the config, in CI and in a local run without--no-cache. It is harmless: the cache carries its own*.gitignore, and super-linter builds its file list from git before linting.
Staleness
F24 💡 Observation (optional action) - Nothing in teams-copy points to its new test
- Document —
home/.local/bin/teams-copy - Location — below the imports
- Issue — No README, CLAUDE.md or skill lists CI's test steps, so nothing is stale, but the test sits in an unexpected top-level directory. Optionally add
# Tested by test/test-teams-copy.sh, which CI runsbelow the imports, outside the docstring that is the published--helptext; CI catches a broken converter regardless.
Consolidated Summary
| Finding | Type | Category | Description | Document | Location | Recommendation | Group | Status |
|---|---|---|---|---|---|---|---|---|
| F1 | 🟢 Low | Formatting | Commit body not refilled to 72 columns | commit message | Paragraphs 1–3 | Implement | G2 | ✅ |
| F2 | 🟢 Low | Formatting | Mention line is 226 columns, MRI written twice | test/test-teams-copy.sh |
Lines 86–88 | Implement | G5 | ✅ |
| F3 | 🟢 Low | Consistency | "Matches" where sibling configs use the imperative | .ruff.toml |
Line 1 | Implement | G6 | ✅ |
| F4 | 🟡 Medium | Accuracy | ruff format effect overstated |
commit message | Paragraph 2; PR Summary | Implement | G2 | ✅ |
| F5 | 🟢 Low | Accuracy | Mutation item names the wrong part of the spacer rule | PR body | Test plan, item 2 | Implement | G2 | ✅ |
| F6 | ⚖️ Decision | Accuracy | How to settle the open CI test-plan item | PR body | Test plan, item 5 | Options (a) / (b) | G1 | ✅ |
| F7 | 🟡 Medium | Correctness | Link-address case passes with protection removed | test/test-teams-copy.sh |
Lines 80–81 | Implement | G3 | ✅ |
| F8 | 🟡 Medium | Correctness | Empty-draft case inherits a malformed people file | test/test-teams-copy.sh |
Lines 92–98 | Implement | G3 | ✅ |
| F9 | 🟡 Medium | Test Coverage | Draft-by-path and missing file untested | test/test-teams-copy.sh |
run(), Input |
Implement | G4 | ✅ |
| F10 | 🟡 Medium | Test Coverage | +/* bullets, 1) and h2–h6 untested |
test/test-teams-copy.sh |
Lists, headings | Implement | G4 | ✅ |
| F11 | 🟢 Low | Test Coverage | & unescaping in links and mentions untested |
test/test-teams-copy.sh |
Inline, Mentions | Implement | G4 | ✅ |
| F12 | 🟢 Low | Test Coverage | Mentions in code spans and NUL stripping untested | test/test-teams-copy.sh |
Inline markup | Implement | G4 | ✅ |
| F13 | 🟢 Low | Test Coverage | Spaced asterisks not tested as literal | test/test-teams-copy.sh |
Lines 74–75 | Implement | G4 | ✅ |
| F14 | 🟢 Low | Test Coverage | People file shape check untested | test/test-teams-copy.sh |
Mentions | Implement | G4 | ✅ |
| F15 | 🟢 Low | Test Coverage | --help untested |
test/test-teams-copy.sh |
Whole file | Defer | — | ⏸️ |
| F16 | ℹ️ Observation | Test Coverage | TTY error branch impractical to test | test/test-teams-copy.sh |
parse_args |
— | — | — |
| F17 | 🟢 Low | Clarity and Structure | expect ignores exit status and hides stderr |
test/test-teams-copy.sh |
expect() |
Implement | G5 | ✅ |
| F18 | 🟢 Low | Clarity and Structure | Header misdescribes the mention cases | test/test-teams-copy.sh |
Lines 8–9 | Implement | G5 | ✅ |
| F19 | 🟢 Low | Clarity and Structure | E501 reason sits away from extend-select |
.ruff.toml |
Lines 1–2 | Implement | G6 | ✅ |
| F20 | 🟢 Low | Clarity and Structure | "never run at all" reads as absolute | commit message | Paragraph 1 | Implement | G2 | ✅ |
| F21 | 🟢 Low | Clarity and Structure | test/ reason leaves out its premise |
commit message | Last sentence; PR body | Implement | G2 | ✅ |
| F22 | ℹ️ Observation | Clarity and Structure | Backslashes in drafts must be doubled | test/test-teams-copy.sh |
run() |
— | — | — |
| F23 | ℹ️ Observation | Clarity and Structure | Ruff's cache lands in the workspace | .ruff.toml |
Whole file | — | — | — |
| F24 | 💡 Observation | Staleness | No pointer from teams-copy to its test | home/.local/bin/teams-copy |
Below imports | — | — | — |
| F25 | ⚖️ Decision | Correctness | Super-linter never hands teams-copy to Ruff | .github/workflows/ci.yml |
Run the linters |
Options (a) | G7 | ✅ |
Pre-Merge Checklist
G1 ✅ — Settle how the CI item is evidenced
Closed by the debug re-run, whose answer opened G7.
- ✅ F6 - How to settle the open CI test-plan item (ruled (b) — the debug re-run showed Ruff never gets teams-copy)
G7 ✅ — Get teams-copy linted, or stop claiming it is
Closed by the explicit Ruff step, which CI ran on 197f2fd.
- ✅ F25 - Super-linter never hands teams-copy to Ruff (ruled (a) — explicit Ruff step)
G3 ✅ — Make the weak and order-dependent cases test what they name
Closed: each case now fails under the mutant it names.
- ✅ F7 - Use a link address whose asterisks the italics rule would otherwise take
- ✅ F8 - Add a lazy-load case and reset the people file after the malformed one
G4 ✅ — Fill the coverage gaps
Closed: every case added, each verified against its mutant.
- ✅ F9 - Pass arguments through
run()and test drafts read from a path - ✅ F10 - Test
+/*bullets,1)and heading levels 2–6 - ✅ F11 - Test
&unescaping in link addresses and mention names - ✅ F12 - Test a mention inside a code span (NUL case optional)
- ✅ F13 - Test that spaced asterisks stay literal
- ✅ F14 - Test the people file's shape check
G5 ✅ — Tidy the test harness and its comments
Closed: harness, long line and header fixed in the same edit.
- ✅ F17 - Check the exit status in
expectand show stderr on failure - ✅ F2 - Split the mention line and set the MRI once
- ✅ F18 - Reword the header's description of the mention cases
G6 ✅ — Tidy the .ruff.toml comments
Closed: both comments rewritten.
- ✅ F3 - Use the imperative "Match", as the sibling configs do
- ✅ F19 - Put the E501 reason above
extend-select
G2 ✅ — Correct the commit message and PR body
Closed by the amended commit e53efd9 and the edited PR body.
- ✅ F4 - Restate what
ruff formatwould change - ✅ F5 - Name the list exception as the mutation that was run
- ✅ F20 - Say teams-copy never ran in CI
- ✅ F21 - State why the test is not beside teams-copy
- ✅ F1 - Refill the commit body to 72 columns
Not recommended for this revision
- ⏸️ F15 - Test
--help(deferred — add it when the CLI options change)
The repository now carries two Python scripts, the /reflow block detector and teams-copy, and CI checked neither as Python. The detector at least had a fixture test; teams-copy never ran in CI at all. Super-linter now runs Ruff's linter, which picks up the .py detector. It does not reach teams-copy: super-linter routes an extensionless file by what `file` reports, and the `file` in its image classifies the script as an HTML document, so it joins no Python linter's list. A separate step runs the Ruff inside super-linter's own image on teams-copy by path, with the image tag read from the super-linter step, so the two runs share one Ruff version and its default rules. A root .ruff.toml, which both runs read, sets the repository's 100-column limit and adds E501, which Ruff's default rules leave out, plus the W and I rules. Both scripts pass as they stand. The formatter and mypy stay off. `ruff format` would restyle both scripts, requoting teams-copy's strings and re-wrapping the detector's hand-wrapped conditions, and neither script has the annotations a type checker needs. A fixture test for teams-copy runs it with --print-html, which skips the macOS clipboard, so the Markdown conversion is exercised on ubuntu-latest. It covers headings, every list marker and how lists group and number, spacer placement, inline markup and escaping, code spans and link addresses, mentions and each people-file error, and drafts read from stdin or a path. HOME points at a private directory so the real people file is never read. The test sits in a top-level test directory rather than beside teams-copy, because homesick links each file in home/.local/bin into ~/.local/bin, which is on PATH.
The repository's new .ruff.toml sets a 100-column limit, but the /reflow block detector was hand-wrapped, and many of its test's cases broke across two lines where one would fit. The detector is now formatted by Ruff 0.15.17 under that config, which only lays out code differently: the parsed syntax tree is unchanged. Each test case that fits in 100 columns now sits on one line; a case that would run past the limit stays split.
132663e to
f39445f
Compare
Summary
VALIDATE_PYTHON_RUFF), which lintsunfilled-blocks.py.teams-copyby path. Super-linter routes an extensionless file by whatfilereports, and thefilein its image classifiesteams-copyas an HTML document, so super-linter's Ruff run never sees it. The step runs the Ruff inside super-linter's own image, with the tag read from the super-linter step'suses:line, so the two runs cannot drift apart in version or default rules..ruff.toml, which both Ruff runs read. It sets the repository's 100-column limit and enablesE501, which Ruff's default rules leave out, plus theWandIrules. Both scripts already pass.test/test-teams-copy.sh, a fixture test in the same style astest-unfilled-blocks.sh. It runsteams-copy --print-html, which skips the macOS clipboard, and covers headings, every list marker, list grouping and numbering, spacer placement, inline markup and escaping, code spans, link addresses, mentions, each people-file error and drafts read from stdin or a path.HOMEpoints at a temporary directory, so the real people file is never read.unfilled-blocks.pywith Ruff under the new config, and join eachtest-unfilled-blocks.shcase that fits in 100 columns onto one line.ruff formatcheck and mypy off. The formatter would requote every string inteams-copy, and neither script has the type annotations mypy needs.The test lives in a top-level
test/directory rather than besideteams-copy, because homesick links each file inhome/.local/bininto~/.local/bin, which is onPATH.Test plan
bash test/test-teams-copy.shpasses all 30 cases locallyteams-copy, each of these mutants fails at least one case: removing link-address protection, loading the people file eagerly, ignoring the path argument, dropping the+or)list markers, restricting headings to#, dropping either&unescape, removing NUL stripping, handling mentions before code spans, removing the italics whitespace guards, removing the people-file shape check and a crash (whose traceback the failure now prints)uvx ruff@0.15.17 checkpasses both scripts with the new.ruff.toml, and reportsE501on a 106-column line under itunfilled-blocks.py: a debug re-run of CI on an earlier head logged "Detected file type for …/teams-copy: HTML document" and "There are 1 items to lint for PYTHON_RUFF"shellcheck test/test-teams-copy.shandyamllint -c .yaml-lint.yml .github/workflows/ci.ymlare cleanunfilled-blocks.pyparses to the same syntax tree as before,uvx ruff@0.15.17 checkandformat --checkpass on it, andbash home/.claude/skills/reflow/scripts/test-unfilled-blocks.shpasses all 38 cases;test-unfilled-blocks.shdiffers from before only in line breaksf39445f, including the teams-copy test and the Ruff step that runs super-linter's image onteams-copy