Repository navigation
Some cleanup - #102
Merged
Merged
Some cleanup#102
Conversation
Lgt2x
reviewed
Oct 8, 2026
Thibault-Pelletier
approved these changes
Oct 8, 2026
Thibault-Pelletier
left a comment
Thibault-Pelletier
left a comment
Contributor
There was a problem hiding this comment.
Tested with trame-slicer and overall looks good to me.
I added a few comments on the tests. Like most Claude generated tests, a lot of them are too tightly coupled with implementation and may add some strain during maintenance.
If possible, it would be nice to trim them a bit to make them more easily human readable.
asyncio.get_event_loop() warns on 3.12 and raises on 3.14 when no loop is running, which broke create_task() before the server starts. Add asynchronous.get_event_loop() mirroring wslink's loop selection and use it in create_task, open_browser and the server exit path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Run pytest on Python 3.10, 3.12 and 3.14 and include the version in the job name - Use coverage source_pkgs so a local trame_server directory can not shadow the installed package - Set ruff target-version to py310 to match requires-python Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Import Callable, Iterable and Iterator from collections.abc - Use builtin type instead of typing.Type - Make zip() strictness explicit (strict=False keeps current behavior) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use parenthesized context managers now that ruff targets py310. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
clear_state_client_cache() raised KeyError for keys that were never sent to the client. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A response for a cancelled request (e.g. after a timeout) raised InvalidStateError and a system message without a matching request raised AttributeError. Both stopped the listen loop and disconnected the client. In-flight entries are now always removed on response. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Add ConnectionStatus IntEnum (DISCONNECTED, CONNECTING, CONNECTED) which stays compatible with the previous 0/1/2 values - connect() waits for the authentication response before reporting CONNECTED, so client_id is always available once connected - connect() raises when the connection or authentication fails and always resets its state, so a failed attempt no longer leaves the client stuck in CONNECTING - call_trigger() raises ConnectionError when not connected Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a child server with a prefix, the untranslated names did not match the cached keys, so nothing was cleared and unchanged values were never sent again. Translate them like force_state_push does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- client: server errors, subscription callback errors (with traceback) and websocket errors are logged as errors, close events as debug - protocol: JS errors without handler are logged as errors and missing triggers as warnings Warnings and errors still reach stderr when no logging is configured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TRAME_ARGS="--banner --no-http" leaked into every later test, so their servers ran without static routes, and trame_net.log was written to the repository root. Use monkeypatch and tmp_path instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The aiohttp router reordering hack no longer worked with aiohttp>=3.10, which resolves routes through an index the hack never updated, so a "/" route added in on_server_bind was shadowed by wslink's redirect. wslink 2.5.8 registers its static routes on start, so routes added in on_server_bind take precedence on any aiohttp version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use a raw WsLinkSession to observe what the server pushes instead of corrupting the client state cache or inspecting private attributes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
I think we are in a good spot now. Let me know if you are good with me merging it? |
Thibault-Pelletier
approved these changes
Oct 9, 2026
Contributor
|
Yes, you can go ahead and merge it 👍 |
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
3 participants
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.
No description provided.