Improve explain command for unknown rules

[rigel08]

Summary

• suggest the closest rule code when explain receives an unknown rule
• explain why TG009 is deliberately not implemented
• provide a fallback message when no close rule exists
• add CLI regression tests for the new behavior

Tests

• pytest tests/test_cli.py -q
• pytest -q

263 passed, 15 skipped, 4 deselected.

[HimanshuJanbandhu]

Hi @rigel08 ? Can you comment on, what issue you are solving exactly, that way it's easier for me to track things and look at your PR!

[HimanshuJanbandhu]

#39
I see you are resolving this issue, can you once comment here! So that I can assign this to you.
Would add this shortly in the Contributors.MD.

[rigel08]

Hi @rigel08 ? Can you comment on, what issue you are solving exactly, that way it's easier for me to track things and look at your PR!

Hey @HimanshuJanbandhu, yep, this PR is for #39. It improves the explain command when an unknown rule is passed by suggesting the closest rule, handling the TG009 case, and adding a fallback when there isn't a close match.

Feel free to assign #39 to me. Thanks!

[HimanshuJanbandhu]

Hi I can't assign it to you, until you comment on that issue!
Open the link, #39 and leave a comment below the issue!

[HimanshuJanbandhu]

Thanks for picking up #39 — the shape is right and the tests cover the cases the issue asked for. Four things to fix before merge, left inline: the prefix fallback can never fire on its own, the tie-break picks the wrong code for the most likely typos, the retired-rule path writes to stderr while returning 0, and the rule count is hardcoded twice.

One thing outside the diff, worth folding in here: the --ignore/--select validation at cli.py:145 still warns unknown rule code TG009 and does not consult the new RETIRED_RULES table, so the two paths now disagree about what TG009 is. Pre-existing behaviour, but this PR is what makes it look inconsistent.

[HimanshuJanbandhu]

The prefix fallback can never fire on its own. It is nested inside if suggestion:, so it can only refine a difflib hit, never produce one.

explain TG gets no suggestion at all, even though all 13 codes start with TG — difflib scores it 0.571, just under the 0.6 cutoff:

>>> difflib.get_close_matches("TG", RULES, n=1)
[]

Hoisting the prefix lookup above the guard fixes it:

prefix_matches = [c for c in RULES if c.startswith(normalized_code)]
if prefix_matches:
    suggestion = [prefix_matches[0]]
else:
    suggestion = difflib.get_close_matches(normalized_code, RULES, n=1)

Heads up while you do it: test_explain_unknown_rule_suggests_closest_code passes only because of this branch — difflib alone ranks TG014 first for TG01, not TG010. Reordering must keep that test green for the right reason.

[HimanshuJanbandhu]

Wrong suggestion on the most likely typos. get_close_matches breaks score ties by descending candidate order, which biases toward the highest code:

Input	Suggested	Almost certainly meant
TG02	TG012	TG002
TG03	TG013	TG003
TG2	TG012	TG002

Both candidates score identically (0.889 for TG02), so the later one wins. The prefix branch cannot rescue these either — TG02 is not a prefix of TG002.

Two options: zero-pad a numeric-looking input to three digits before matching, or keep n=3 and tie-break on the lowest code. Either way it is worth a test with TG02, since that is the typo a reader of the rules table is most likely to make.

[HimanshuJanbandhu]

Returns 0 but writes nothing to stdout. The explanation goes to stderr, so:

$ torch-preflight explain TG009 > out.txt
$ echo $?
0
$ cat out.txt
$

Exit 0 with an empty file, and anything capturing via command substitution sees success and no text. Every other EXIT_OK path in this function prints to stdout via Console().

The exit code is the right call — the question was answered — but then the answer belongs on stdout. Suggest printing through Console() like the success path, and asserting on capsys.readouterr().out in test_explain_retired_rule.

[HimanshuJanbandhu]

Hardcoded rule count, here and again at line 211. RULES is already imported in this module, so len(RULES) costs nothing and cannot go stale:

f"Run `torch-preflight rules` to see all {len(RULES)}."

This is the same drift #40 is about, and implementing TG009 someday would make the message wrong in two places at once.

[HimanshuJanbandhu]

Re-reviewed at 654d357. All four findings from the last round are addressed — the prefix lookup is hoisted so explain TG now suggests TG001, the retired-rule path prints to stdout and the test asserts on .out, and the count is len(RULES) in both branches. 26 tests pass; I ran the CLI from a worktree of this head.

One new regression came in with the tie-break fix, left inline: min() runs over all difflib candidates rather than only the tied ones, so TG13 and TG14 now suggest TG012 — both were correct on 337f898. Fix and verification table in the comment.

Still open from last round, your call whether it belongs in this PR: the --ignore/--select validation at cli.py:146 warns unknown rule code TG009 without consulting RETIRED_RULES, so the two paths keep disagreeing about what TG009 is.

[HimanshuJanbandhu]

This fixes the tie case but regresses the non-tie case. min() runs over all three candidates, not just the tied ones, so the similarity score is discarded entirely and the lowest-numbered candidate always wins:

$ torch-preflight explain TG13
help: did you mean TG012? ...     # difflib ranked TG013 first
$ torch-preflight explain TG14
help: did you mean TG012? ...     # difflib ranked TG014 first

Both were correct on 337f898 — this commit is what breaks them. Every teens typo now lands on TG012.

The tie-break needs to apply only among candidates sharing the top score:

scored = [
    (difflib.SequenceMatcher(None, normalized_code, c).ratio(), c)
    for c in suggestions
]
best = max(score for score, _ in scored)
suggestion = min(
    (c for score, c in scored if score == best),
    key=rule_number,
)

Run against the registry, that gives:

Input	Suggested
TG	TG001
TG02	TG002
TG03	TG003
TG2	TG002
TG11	TG011
TG13	TG013
TG14	TG014
ZZZ999	none

Worth pulling the int(rule_code[2:]) if ... else inf lambda out into a named rule_number() helper while you are in here — it is doing enough work to deserve a name, and it will be used twice.

A test case with TG13 next to the existing TG02 one would have caught this, and pins both directions against the next change to this heuristic.

[HimanshuJanbandhu]

Correct fix for the stdout finding — thanks.

Minor, take it or leave it: Console.print() parses [...] as rich markup, so a future RETIRED_RULES entry containing brackets would have them silently eaten or raise on a bad tag. Today's TG009 text has none. console.print(..., markup=False) makes it inert without changing anything you can see now.

[HimanshuJanbandhu]

Any updates on this @rigel08 ?