Skip to content

[Fix] Websocket and Log Viewer - #107

Merged
antonio-amjr merged 5 commits into
project-chip:v2.15.1-cli-developfrom
antonio-amjr:fix/websocket_and_log_viewer
Aug 14, 2026
Merged

[Fix] Websocket and Log Viewer#107
antonio-amjr merged 5 commits into
project-chip:v2.15.1-cli-developfrom
antonio-amjr:fix/websocket_and_log_viewer

Conversation

@antonio-amjr

@antonio-amjr antonio-amjr commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fix: project-chip/certification-tool#1072
Depends on Backends: project-chip/certification-tool-backend#347

Description

Companion fix to the backend PR for #1072. Two independent problems remained on the CLI side even after the backend stopped stalling and truncating logs:

  1. The CLI could silently disconnect from the backend's WebSocket the instant it saw the "run finished" signal, before a final trailing batch of log messages had arrived — losing the last few lines of a run.
  2. The browser-based real-time log viewer (--log-streaming) would freeze during large runs: it received log lines faster than it could render them, so its internal queue grew into a multi-minute backlog. The view looked "stuck," even though the connection was healthy — it was just always showing stale content instead of what was happening right now. Since this viewer is the only place users can see SDK trace output live (used to decide how to answer test prompts), a stale view is effectively as broken as no view.

Changes

  • Don't disconnect immediately on run-completion. The WebSocket loop now keeps draining for a short grace period after seeing the terminal state, instead of closing the instant that message arrives, so any trailing log batch the backend
    is still sending isn't dropped.
  • Process log batches without blocking the connection. Large incoming batches are now processed with periodic yields back to the event loop, instead of one big synchronous loop, so the connection stays responsive even under a heavy burst.
  • Cap the live viewer's pending queue. If incoming log lines arrive faster than the browser can render them, the oldest unrendered entries are now dropped so the view stays within a few seconds of real time — instead of accumulating an
    ever-growing backlog that only "catches up" long after the test has already finished. This does not affect the saved log file or the full downloadable log, only what's shown live. (An earlier version of this fix hid raw SDK trace content from
    the live view entirely to solve the freeze — that broke the actual reason the live viewer exists, since it's the only place to read trace context while answering a test prompt. This version keeps everything live and fixes the actual backlog
    instead.)
  • "Download Logs" now points at the backend's own log endpoint instead of the CLI's own process. The previous implementation served the file from the CLI's local, short-lived HTTP server — which shuts down the moment the run finishes, i.e. right when someone would naturally want to click "download." Rather than trying to keep that ephemeral server alive or fall back to an incomplete in-memory reconstruction, the button now links directly to the backend's existing, durable
    log-download endpoint (the same one the web UI's download has always used successfully). This is the standard fix for this kind of problem — never serve a "download" from a transient process; point at wherever the artifact durably lives.

Verified

Watched the live viewer through two full TC-ACE-2.4 runs: content stays current throughout (brief freezes), and the CLI's saved log matches the backend's copy.

Also, the websocket closure was postponed for when inactive and now yields to the event loop every 200 records intead of whole batch
@antonio-amjr antonio-amjr self-assigned this Aug 11, 2026
@antonio-amjr
antonio-amjr requested review from oxesoft and rquidute and removed request for oxesoft August 11, 2026 16:49
@mergify

mergify Bot commented Aug 11, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@KishokG KishokG moved this to In Progress in CSG issue tracking Aug 13, 2026
Comment thread th_cli/test_run/log_viewer.html
@rquidute

Copy link
Copy Markdown
Contributor

/gemini review

@oxesoft

oxesoft commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai review full

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

@oxesoft I will perform a complete review of PR #107.

✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 907f50fb-518b-4499-851a-b94d7a7ec671

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 43267653-0238-4c62-bf43-423a8ec2acc6

📥 Commits

Reviewing files that changed from the base of the PR and between 0f00720 and 0717968.

📒 Files selected for processing (7)
  • tests/test_run/test_websocket_socket.py
  • th_cli/commands/run_tests.py
  • th_cli/test_run/log_stream_handler.py
  • th_cli/test_run/log_viewer.html
  • th_cli/test_run/logging.py
  • th_cli/test_run/logs_http_server.py
  • th_cli/test_run/websocket.py

📝 Walkthrough

Walkthrough

The websocket now marks completed runs, drains trailing messages, and yields during large log batches. The logging subsystem propagates the created run ID to the HTTP server and connected viewers through SSE. The viewer creates backend download links and limits queued and retained logs. The HTTP server removes its log-download endpoint and file-path handling. Tests cover completion state, handler calls, and log-record forwarding.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the WebSocket and log viewer fixes covered by the changes.
Description check ✅ Passed The description directly explains the CLI WebSocket, live viewer, and log download changes.
Linked Issues check ✅ Passed The changes address issue [#1072] by preserving trailing WebSocket logs and maintaining responsive CLI log processing.
Out of Scope Changes check ✅ Passed The run ID and backend download-link changes support the stated logging and download objectives.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

This is not related to the issue being fixed.
@antonio-amjr
antonio-amjr merged commit bf49694 into project-chip:v2.15.1-cli-develop Aug 14, 2026
4 of 5 checks passed
@antonio-amjr
antonio-amjr deleted the fix/websocket_and_log_viewer branch August 14, 2026 19:50
@github-project-automation github-project-automation Bot moved this from In Progress to Done in CSG issue tracking Aug 14, 2026
rquidute added a commit that referenced this pull request Sep 3, 2026
* [FEATURE] Adding Realtime Log feature to CLI (#81)

* Adding Real-time log feature access to the CLI via browser

* Changing browser page style and adding download logs feature

* Moving log streaming URL display to closer to the Test Run execution start

* reducing Real-time log description in default config JSON

* Supressing exceptions when no browser is connected

Also fixing the downloaded file to show breakline properly.

* Reading file in chunks for download

Also fixing incomplete log streaming

* Adding run_tests argument option to disable real-time logs

* [Fix] th-cli --version Surfaces Backend And SDK Versions (#999) (#85)

* [Fix] th-cli --version Surfaces Backend And SDK Versions (#999)

* Update th_cli/utils.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* [Feature] Config Output To Project Command (#84)

* Adding config output for CLI project command

* Improving __print_config() method and adding short flags

* Add CI workflow to run pytest on every PR (#95)

* Add CI workflow to run pytest on every PR

Adds .github/workflows/python-tests.yml which:
- Triggers on all pull requests (any target branch)
- Sets up Python 3.10 and installs dependencies via Poetry (with venv cache)
- Runs ./scripts/run_pytest.py (existing test runner)
- Fails if coverage drops below 85% (enforced by pyproject.toml)
- Uploads coverage.xml and htmlcov/ as artifacts on every run

Tracks: project-chip/certification-tool#1020

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix CI: bump action versions to v4/v5

actions/upload-artifact@v3 was deprecated and disabled by GitHub in
November 2024, causing the job setup to fail before any steps ran.

Bump all actions to their current major versions:
- actions/checkout: v3 -> v4
- actions/setup-python: v4 -> v5
- actions/cache: v3 -> v4
- actions/upload-artifact: v3 -> v4

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix CI: correct test runner script name (run_pytest.sh not .py)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix failing test and align coverage threshold with actual coverage

- Fix test_test_run_execution_log_whitespace_content: Click prepends a
  DeprecationWarning line when invoking a deprecated option (--log).
  Strip DeprecationWarning lines before asserting on output content.

- Lower --cov-fail-under from 85 to 65 to match the actual coverage
  achieved by the current test suite (65.54%). The 85% target was
  aspirational and not yet reached; using it as a hard gate would
  permanently block CI on every PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix CI: use Python 3.12 to match development environment

Python 3.10's unittest.mock._dot_lookup resolves dotted patch paths
differently from 3.12: it walks the path with getattr, finds the
imported Click Command object at 'th_cli.commands.abort_testing' (due
to 'from .abort_testing import abort_testing' in __init__.py), and
then fails trying to get 'get_client' off the Command object.

Python 3.12 correctly resolves the patch target via sys.modules,
finding the module rather than the imported name.

All tests pass locally on 3.12; switching CI to match eliminates
the AttributeError failures across all command modules.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Revert coverage threshold back to 85%

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* feat: add project export and import CLI commands (#1025) (#98)

* feat: add project export and import CLI commands (#1025)

Add 'project export' and 'project import' subcommands to the CLI,
wiring up the existing backend endpoints:
- GET /api/v1/projects/{id}/export
- POST /api/v1/projects/import

project export:
- Downloads the project config as a JSON file
- Defaults to '<project-name>-project-config.json' if no output file is given
- Accepts --output-file / -o to specify a custom path

project import:
- Accepts --file / -f pointing to a previously exported JSON file
- Sends the file bytes to the backend and prints the new project ID

Also adds full unit test coverage in tests/test_project_commands.py
(TestExportProjectCommand and TestImportProjectCommand).

Closes #1025

* fix: resolve unit test failures in project export/import and logger tests

- Add missing 'import json' to test_project_commands.py
- Update test_run_tests_logger_configuration to include enable_log_streaming=True
  in the expected configure_logger_for_run call, matching the actual call signature

* fix: apply code review suggestions from PR #98

- Use model_dump_json(indent=2) instead of model_dump() + json.dumps()
  to correctly serialize Pydantic models with non-standard types
- Split OSError handling in _import_project into FileNotFoundError
  and generic OSError to avoid broken error messages from wrapping
  OSError in FileNotFoundError

* Fix Push AV Stream Verification player reading stale field names (#103)

* Fix Push AV Stream Verification player reading stale field names

The Push AV Server's /streams API returns each stream's uploaded
files under valid_uploads/error_uploads (lists of {file_path,
reasons?}) since the server became session-oriented. The CLI's
push_av_stream_verification.html was never updated and still looked
for files/valid_files/invalid_files, which no longer exist in the
response. As a result, allFiles was always empty, no .mpd/.m4s entry
point was ever found, and the video player stayed blank even when
the DUT successfully uploaded CMAF content to the server.

Add getStreamFilePaths() to read valid_uploads/error_uploads first,
falling back to the legacy files/valid_files/invalid_files shape for
compatibility with older server responses. Also surface per-file
non-conforming reasons in the Non-Conforming Files section using the
reasons field now provided by error_uploads entries.

Add regression tests asserting the rendered template references the
current field names ahead of the legacy fallback.

* Apply defensive null-checks to Push AV upload parsing per code review

Use optional chaining (u?.file_path, u?.reasons) and filter(Boolean)
when mapping valid_uploads/error_uploads entries to file paths and
reasons, so a malformed or null entry in the server response can't
throw a TypeError and block the verification page from rendering.

Update the corresponding test assertion to check for the file_path
field name generically instead of the literal 'u.file_path' loop
variable expression, which no longer appears verbatim once optional
chaining is used.

* Don't misreport successful log uploads as errors after WebSocket drop (#1062) (#105)

* Stop misreporting successful log uploads as errors after WebSocket drop

Uploading a large manual test log can keep the backend's event loop
busy long enough that the WebSocket's ping/pong keepalive times out
and the connection is dropped before the CLI can send the prompt
response confirming the upload. Previously this was caught by the
generic exception handler in __upload_file_and_send_response and
reported as 'Unexpected error uploading file: ...', even though the
upload itself had already completed successfully (issue #1062).

- Wrap only the prompt-response send (not the upload) in a dedicated
  try/except for websockets.exceptions.ConnectionClosed, and report
  it as a distinct warning that makes clear the file was already
  uploaded, instead of letting it fall into the same 'unexpected
  error' branch as an actual upload failure.
- Add unit tests covering: successful upload + successful response,
  successful upload with the WebSocket closed before the response
  can be sent, and an actual upload failure (still reported as an
  error, as before).

* Address review: catch any exception, not just ConnectionClosed, after successful upload

Only catching websockets.exceptions.ConnectionClosed left other
post-upload notification failures (e.g. websockets.exceptions.
InvalidState, or a plain OSError from a socket already torn down)
to fall through to the outer except Exception block, which still
misreported them as "Unexpected error uploading file" - the exact
bug this fix targets.

Broaden the inner except to Exception, since by this point the
upload has already succeeded and any exception sending the
confirmation is a notification failure, not an upload failure. The
try block scopes exactly one call (_send_prompt_response), so this
isn't a broad catch-all - it matches the actual invariant at this
point in the function.

Remove the now-unused top-level "import websockets" statement and
add a regression test covering a non-ConnectionClosed exception.

* Strip trailing whitespace/CR from uploaded file path input

__prompt_user_for_file_upload read the raw value from aioconsole.ainput()
and only called .strip() when checking for an empty/skip response, then
passed the *unstripped* value to __valid_file_upload() and returned it
unstripped on success. Some terminals/SSH sessions send a trailing \r
(or other whitespace) with the input line; that stray character made
os.path.isfile() fail even though the file existed exactly where the
user said it did, surfacing as a misleading "Invalid file path or
type" error for an otherwise-correct path.

Strip the input once, immediately after reading it, and use the
stripped value consistently for the emptiness check, validation, and
the returned path.

* [Fix] Websocket and Log Viewer (#107)

* Max retained logs added for the CLI log viewer.

Also, the websocket closure was postponed for when inactive and now yields to the event loop every 200 records intead of whole batch

* Cap the rendering to 2000 lines and changed log viewer download logs feature

* Log viewer download button now opens no tab and start the download immediately

* Pushing the run_id to the queue that feeds the live stream

* Fixing state verifying to match the backends.

---------

Co-authored-by: antonio-amjr <116589331+antonio-amjr@users.noreply.github.com>
Co-authored-by: abhisheksingh-esp <abhishek.singh@espressif.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Romulo Quidute Filho <116586593+rquidute@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[MVE] 1.6.1: TC-ACE-2.4 docker-python and CLI run logging difference

5 participants