fix(tinytorch): tito package reset is unreachable - #2015
Closed
Shashank-Tripathi-07 wants to merge 1 commit into
Closed
fix(tinytorch): tito package reset is unreachable#2015Shashank-Tripathi-07 wants to merge 1 commit into
Shashank-Tripathi-07 wants to merge 1 commit into
Conversation
Bug
'tito package reset' (with or without piped input) does nothing but
print a generic help panel every time, regardless of input. The
documented example in PackageCommand's own help text, 'tito package
reset --force', silently does nothing at all.
Root cause
ResetCommand was written as if it would be registered as its own
top-level 'tito reset <subcommand>' command: its name property returns
"reset", and its own help text says 'tito reset progress --backup'.
It is actually only ever mounted at 'tito package reset'. By the time
argparse reaches this class's own add_subparsers() call, both the
'package' and 'reset' tokens are already consumed by the parent
parser, so a bare 'tito package reset' never has a SUBCOMMAND token
left to match, and args.reset_command is always unset. The only way
to actually trigger a reset was the undocumented double-noun 'tito
package reset package --force'.
Fix
- Added a --force argument directly on the reset parser itself (not
just on each sub-subparser), so it's available for a bare
invocation.
- Bare 'tito package reset [--force]' (no SUBCOMMAND) now defaults to
resetting the package, matching PackageCommand's own documented
example and this command's stated top-level purpose ("Reset
tinytorch package to clean state"). It still shows the other reset
types (all/progress/milestones/config) for discoverability, then
proceeds with the actual package reset (still gated behind its own
confirmation prompt unless --force is passed).
- 'tito package reset package --force' (the old double-noun form)
still works unchanged.
Testing
Verified against a fresh dev install with all 20 modules exported:
- 'tito package reset', answering "n": shows the confirmation prompt
and correctly cancels. Before the fix, this printed only the
self-referential help panel and never reached a confirmation at all.
- 'tito package reset --force': correctly removes all 20 generated
package files, matching the documented example exactly.
- 'tito package reset package --force': still works as before.
profvjreddi
added a commit
that referenced
this pull request
Aug 31, 2026
…MMAND #2015 added --force to the parent reset parser so the bare form could skip its prompt, but each subparser already defines its own --force on the same dest. argparse writes a subparser's default over the parent's value, so 'tito package reset --force package' silently dropped the flag and then blocked on input(), which is an EOFError under a script or in CI. Give the parent flag its own dest and OR the two in run(), so --force works before or after the SUBCOMMAND token. run() also has to set args.force itself: the bare form parses no subparser, so the attribute does not otherwise exist and _reset_package() would raise AttributeError.
profvjreddi
added a commit
that referenced
this pull request
Aug 31, 2026
Land four more contributor PRs, each audited by running it rather than reading it, with the defects found repaired here: #2015 bare 'tito package reset' now actually resets #2023 conftest validates all 20 module exports #2080 link-check counts from the failure sections only #2092 Jupyter server reuse + start/resume desync recovery Three of the four had a real defect that only surfaced under execution: #2015 --force before the SUBCOMMAND token was silently dropped by the subparser default, then blocked on input() (EOFError in CI) #2023 the module-20 registry path never resolved, so every pytest run on a fully-exported tree printed a false 'not exported' warning, and two of the PR's own tests asserted the wrong value #2092 the pid check returned True for any live process on macOS, so a recycled pid would permanently block launching Jupyter #2080 needed no repair: its three claims check out against real lychee 0.23.0 output.
Contributor
|
Thanks @Shashank-Tripathi-07. Integrated into dev, CI green. One repair on the way in. Each subparser already defines its own Gave the parent flag its own dest and OR the two in |
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.
Bug
tito package reset(with or without piped input) does nothing but print a generic help panel every time, regardless of input. The documented example inPackageCommand's own help text,tito package reset --force, silently does nothing at all.Root cause
ResetCommandwas written as if it would be registered as its own top-leveltito reset <subcommand>command: itsnameproperty returns"reset", and its own help text saystito reset progress --backup. It is actually only ever mounted attito package reset. By the time argparse reaches this class's ownadd_subparsers()call, both thepackageandresettokens are already consumed by the parent parser, so a baretito package resetnever has a SUBCOMMAND token left to match, andargs.reset_commandis always unset. The only way to actually trigger a reset was the undocumented double-nountito package reset package --force.Fix
--forceargument directly on the reset parser itself (not just on each sub-subparser), so it's available for a bare invocation.tito package reset [--force](no SUBCOMMAND) now defaults to resetting the package, matchingPackageCommand's own documented example and this command's stated top-level purpose ("Reset tinytorch package to clean state"). It still shows the other reset types (all/progress/milestones/config) for discoverability, then proceeds with the actual package reset (still gated behind its own confirmation prompt unless--forceis passed).tito package reset package --force(the old double-noun form) still works unchanged.Testing
Verified against a fresh
devinstall with all 20 modules exported:tito package reset, answering "n": shows the confirmation prompt and correctly cancels. Before the fix, this printed only the self-referential help panel and never reached a confirmation at all.tito package reset --force: correctly removes all 20 generated package files, matching the documented example exactly.tito package reset package --force: still works as before.Test plan
tito package reset, confirm it shows a real confirmation prompt (not just a help panel)tito package reset --force, confirm it actually removes exported package filestito package reset package --force, confirm the old form still works