Conversation
Rename _DEFAULT_BASE → DEFAULT_SESSION_PATH (public, exported) in parser.py and import it in cli.py, replacing the hardcoded duplicate. Update existing tests to patch the new name in both modules. Add tests asserting the constant value and __all__ membership. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR removes a duplicated default session-state directory path in the CLI by reusing a single canonical constant from the parser module, preventing silent divergence between the Watchdog observer path and session discovery.
Changes:
- Exported
DEFAULT_SESSION_PATHas a publicFinalconstant fromcopilot_usage.parserand included it inparser.__all__. - Updated
copilot_usage.cli._interactive_loopto useDEFAULT_SESSION_PATHinstead of a hardcodedPath.home() / ".copilot" / "session-state"literal. - Updated and added tests to patch and assert the new exported constant.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/copilot_usage/parser.py |
Introduces/export renaming to DEFAULT_SESSION_PATH and uses it for discovery/cache roots. |
src/copilot_usage/cli.py |
Deduplicates the default session path by importing and using DEFAULT_SESSION_PATH. |
tests/copilot_usage/test_cli.py |
Updates monkeypatching to patch DEFAULT_SESSION_PATH in both parser and CLI modules. |
tests/copilot_usage/test_parser.py |
Adds guards verifying DEFAULT_SESSION_PATH value and presence in parser.__all__. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
microsasa
deleted the
fix/850-dedupe-default-session-path-9ace1a89fe449e2d
branch
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 #850
Summary
cli._interactive_loopduplicated the default session-state path (Path.home() / ".copilot" / "session-state") instead of reusing the constant fromparser.py. This created a latent bug: if the canonical default changed in one place but not the other, the Watchdog file observer would silently monitor the wrong directory.Changes
parser.py: Rename_DEFAULT_BASE→DEFAULT_SESSION_PATH(publicFinalconstant), add to__all__cli.py: ImportDEFAULT_SESSION_PATHfrom parser, replace hardcoded literaltest_cli.py: Update two existing tests to monkeypatch the new name in bothparserandclimodulestest_parser.py: AddTestDefaultSessionPathwith two guards:Path.home() / ".copilot" / "session-state"parser.__all__No behaviour change — pure DRY refactor.