Conversation
… tests Update three stale documentation sections in implementation.md: 1. Replace 'Fallback to blocking input()' with the current threaded _start_input_reader_thread() approach (daemon thread, SimpleQueue, _FALLBACK_EOF sentinel, lazy initialization). 2. Rename _FileChangeHandler → FileChangeHandler, correct module to interactive.py, update code snippet to use WATCHDOG_DEBOUNCE_SECS and match current signatures. 3. Update observer section to reference start_observer()/stop_observer() in interactive.py instead of inline Observer() construction. Add test_implementation_md_symbols_exist_in_expected_modules to tests/test_docs.py verifying that key symbol names referenced in implementation.md actually exist in their stated modules. Closes #1063 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates implementation.md to reflect the current interactive-loop implementation (threaded stdin fallback + watchdog observer helpers), and adds a unit test to prevent future documentation drift by asserting referenced symbols still exist.
Changes:
- Refreshes
implementation.mdsections for the threaded_start_input_reader_thread()fallback and watchdog observer lifecycle (start_observer()/stop_observer()). - Updates the file watcher handler documentation to use
FileChangeHandlerandWATCHDOG_DEBOUNCE_SECSas implemented ininteractive.py. - Adds
test_implementation_md_symbols_exist_in_expected_modulesto assert key documented symbols exist and (for public ones) are exported via__all__.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/test_docs.py |
Adds a symbol-existence test intended to keep implementation.md aligned with actual module symbols/exports. |
src/copilot_usage/docs/implementation.md |
Updates stale interactive-loop documentation for stdin fallback threading and watchdog observer/handler structure. |
- Correct fallback trigger wording to match actual failure modes (non-selectable stdin on Windows, detached stdin in tests) - Accurately document daemon thread lifecycle limitations (no explicit teardown; may outlive _interactive_loop call) - Strengthen symbol-existence test to verify doc attributes each symbol to the correct module file, not just symbol presence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
Contributor
There was a problem hiding this comment.
Low-impact documentation update with meaningful symbol-existence tests. Changes update three stale implementation.md sections to match current code and add a test preventing future drift. No business logic, API, or data model changes. Auto-approving for merge.
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 #1063
Changes
Updates three stale documentation sections in
src/copilot_usage/docs/implementation.md:Fallback input section — Replaced the outdated "Fallback to blocking
input()" section with the current threaded_start_input_reader_thread()approach: daemon thread,queue.SimpleQueue,_FALLBACK_EOFsentinel, lazy initialization on firstValueError/OSError.FileChangeHandler section — Renamed
_FileChangeHandler→FileChangeHandler, corrected its module tointeractive.py, updated the code snippet to useWATCHDOG_DEBOUNCE_SECSconstant and match current typed signatures.Observer section — Updated to reference
start_observer()/stop_observer()ininteractive.pyinstead of inlineObserver()construction incli.py.Testing
Added
test_implementation_md_symbols_exist_in_expected_modulestotests/test_docs.pythat verifies key symbol names referenced inimplementation.mdactually exist in their stated modules:FileChangeHandlerexported fromcopilot_usage.interactive_start_input_reader_threadaccessible incopilot_usage.clistart_observer/stop_observerexported fromcopilot_usage.interactiveWATCHDOG_DEBOUNCE_SECSexported fromcopilot_usage.interactiveThis prevents future renames from silently drifting out of sync with the docs.
Warning
The following domains were blocked by the firewall during workflow execution:
astral.shpypi.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.