Skip to content

Opus review - #356

Merged
rijesha merged 6 commits into
developfrom
opus_review
Jul 7, 2026
Merged

Opus review#356
rijesha merged 6 commits into
developfrom
opus_review

Conversation

@rijesha

@rijesha rijesha commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens struct-frame’s multi-language SDK/parsers by fixing edge-case dispatch/resync behavior and expanding the negative/diagnostics regression suite to keep implementations aligned across languages.

Changes:

  • Fix handler dispatch skipping in TS/JS SDKs when a handler unsubscribes during message dispatch; add a regression test for concurrent requests resolving from a single response.
  • Unify/expand negative-test scenarios across languages (diagnostic counters, resync behavior for minimal/IPC profiles, boundary-split sweeps, and livelock regressions).
  • Improve SDK robustness/perf across languages (C# send buffer pooling, C# reconnect loop, Python BaseTransport send serialization, C++ transport thread shutdown safety, and generator support for min_size).

Reviewed changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/ts/test_request_response_sdk.ts Adds regression coverage for concurrent predicate-less requests resolving from a single response.
tests/ts/test_negative.ts Adds new uniform negative/diagnostics/buffer-mode regression scenarios for TS.
tests/rust/src/test_negative.rs Expands Rust negative tests to match cross-language scenario parity (incl. diagnostics + package corruption).
tests/rust/src/main.rs Adjusts interop helper to ignore invalid drain events and only consider valid frames.
tests/run_tests.py Compiles/runs new C++ compile-only SDK header test in CI runner logic.
tests/py/test_sdk.py Adds Python transport send-serialization regression test.
tests/py/test_negative.py Adds new uniform negative/diagnostics/buffer-mode regression scenarios for Python.
tests/NEGATIVE_TESTS.md Updates documentation to reflect expanded uniform scenario set and counts per language.
tests/js/test_negative.js Adds new uniform negative/diagnostics/buffer-mode regression scenarios for JS.
tests/csharp/TestNegative.cs Adds new uniform negative/diagnostics/buffer-mode regression scenarios for C#.
tests/csharp/TestBaseTransport.cs Adds reconnect retry-until-success regression test for C# BaseTransport.
tests/cpp/test_sdk_headers_compile.cpp New compile-only test ensuring sdk_embedded.hpp compiles standalone.
tests/cpp/test_negative.cpp Adds new uniform negative/diagnostics/buffer-mode regression scenarios for C++.
tests/coverage_spec.py Updates coverage specification tables/text for expanded uniform negative scenarios.
tests/c/test_negative.c Adds new uniform negative/diagnostics/buffer-mode regression scenarios for C.
tests/api_stability/nuget/PublicAPI.Shipped.txt Updates shipped public API surface for new/updated C# members.
src/struct_frame/ts_gen.py Updates TS codegen to emit min_size consistent with other languages.
src/struct_frame/py_gen.py Updates Python codegen to include min_size in MessageInfo.
src/struct_frame/js_gen.py Updates JS codegen to emit min_size consistent with other languages.
src/struct_frame/csharp_gen.py Updates C# codegen to include MinSize in generated MessageInfo.
src/struct_frame/cpp_gen.py Updates C++ codegen to include min_size in generated MessageInfo.
src/struct_frame/boilerplate/ts/struct-frame-sdk/struct-frame-sdk.ts Fixes dispatch iteration by snapshotting handler list to avoid skipping handlers.
src/struct_frame/boilerplate/ts/frame-profiles.ts Improves parser statuses, buffer-mode resync, wedge-escape, and buffer-mode diagnostics parity.
src/struct_frame/boilerplate/rust/frame_profiles.rs Aligns Rust BufferReader drain contract + adds diagnostics/wedge-resync logic.
src/struct_frame/boilerplate/py/struct_frame_sdk/websocket_transport.py Adapts sync WebSocket transport to BaseTransport _send_impl template method.
src/struct_frame/boilerplate/py/struct_frame_sdk/udp_transport.py Adapts UDP transport to BaseTransport _send_impl template method.
src/struct_frame/boilerplate/py/struct_frame_sdk/transport.py Adds send serialization via lock and introduces _send_impl template method.
src/struct_frame/boilerplate/py/struct_frame_sdk/tcp_transport.py Adapts TCP transport to BaseTransport _send_impl template method.
src/struct_frame/boilerplate/py/struct_frame_sdk/serial_transport.py Adapts serial transport to BaseTransport _send_impl template method.
src/struct_frame/boilerplate/py/frame_profiles.py Adds min_size, improves statuses/resync, uses memoryview for perf, adds buffer-mode diagnostics parity.
src/struct_frame/boilerplate/js/struct-frame-sdk/struct-frame-sdk.js Fixes dispatch iteration by snapshotting handler list to avoid skipping handlers.
src/struct_frame/boilerplate/js/frame-profiles.js Mirrors TS parser status/resync/wedge-escape and diagnostics parity improvements.
src/struct_frame/boilerplate/csharp/Framework/Types/MessageInfo.cs Adds MinSize to MessageInfo to support unified length-error semantics.
src/struct_frame/boilerplate/csharp/Framework/Sdk/Transports/SerialTransport.cs Ensures link-closed is surfaced when read loop ends unexpectedly.
src/struct_frame/boilerplate/csharp/Framework/Sdk/Transport.cs Fixes reconnect loop retry semantics and improves concurrency visibility of connection state.
src/struct_frame/boilerplate/csharp/Framework/Sdk/StructFrameSdk.cs Uses ArrayPool to reduce per-send allocations; avoids unnecessary frame clone when no listeners.
src/struct_frame/boilerplate/csharp/Framework/Framing/BufferReader.cs Improves IPC (no-start-bytes) resync behavior for unknown msg IDs.
src/struct_frame/boilerplate/csharp/Framework/Framing/AccumulatingReader.cs Adds wedge-escape/resync and buffer-mode diagnostics parity with stream mode.
src/struct_frame/boilerplate/cpp/struct_frame_sdk/transport.hpp Makes reconnect hook virtual and connection state atomic for cross-thread correctness.
src/struct_frame/boilerplate/cpp/struct_frame_sdk/struct_frame_sdk.hpp Adjusts no-start-byte resync to advance by one byte instead of discarding all.
src/struct_frame/boilerplate/cpp/struct_frame_sdk/serial_transport.hpp Adds basic null/connection guards before connect/send.
src/struct_frame/boilerplate/cpp/struct_frame_sdk/network_transports.hpp Adds safe io_thread shutdown helper and improves error->close surfacing; renames ASIO serial config to avoid header collisions.
src/struct_frame/boilerplate/cpp/frame_profiles.hpp Aligns statuses/resync/wedge-escape and diagnostics parity across modes; improves IPC resync behavior.
src/struct_frame/boilerplate/cpp/frame_base.hpp Adds min_size to MessageInfo for unified length-error semantics.
src/struct_frame/boilerplate/c/frame_profiles.h Improves IPC resync, adds wedge-escape, caches constants for perf, and aligns buffer-mode diagnostics semantics.

Comment thread tests/run_tests.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@rijesha

rijesha commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix failing tests

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@copilot fix failing tests

Fixed in commit that addresses three interconnected issues causing the C++ compilation failure:

  1. src/struct_frame/cpp_gen.py: Changed generated .structframe.hpp files to use #include <frame_base.hpp> (angle-bracket) instead of #include "frame_base.hpp" (quoted). Angle-bracket form uses the -I search path rather than the current file's directory, preventing the generated copy from shadowing the SDK's canonical copy.

  2. tests/run_tests.py: For SDK test compilation (test_sdk_subscribe, test_sdk_headers_compile), sdk_dir is now placed before gen_dir in -I flags, so <frame_base.hpp> resolves to the same physical file that the SDK's own headers use — eliminating the ODR violation.

  3. tests/cpp/test_sdk_subscribe.cpp: Removed the explicit #include "../../generated/cpp/frame_profiles.hpp" (the SDK headers already provide ProfileStandardConfig, FrameEncoderWithCrc, etc.).

@rijesha

rijesha commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot a test table name was changed in commit efa5724.
This is causing the build to fail:

============================================================
ENVELOPE SDK TESTS

Traceback (most recent call last):
File "/home/runner/work/struct-frame/struct-frame/test_all.py", line 21, in
exit_code = run_tests_main()
^^^^^^^^^^^^^^^^
File "/home/runner/work/struct-frame/struct-frame/tests/run_tests.py", line 2837, in main
success = runner.run(
^^^^^^^^^^^
File "/home/runner/work/struct-frame/struct-frame/tests/run_tests.py", line 2715, in run
self.run_sdk_tests()
File "/home/runner/work/struct-frame/struct-frame/tests/run_tests.py", line 1813, in run_sdk_tests
_record(test_row, "cpp", success, stdout, stderr,
File "/home/runner/work/struct-frame/struct-frame/tests/run_tests.py", line 1778, in _record
table_data[test_name][lang_id] = success

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@copilot a test table name was changed in commit efa5724.
This is causing the build to fail:

...

Fixed in the latest commit. test_sdk_headers_compile was missing from SDK_APPLICABILITY in tests/run_tests.py, so table_data didn't have a row for it and _record raised a KeyError. Added the entry (C++ only) and all tests now pass.

@rijesha

rijesha commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot test coverage issue.

2026-07-06T23:58:37.9335354Z ##[group]Run python tests/gen_test_coverage.py --check
python tests/gen_test_coverage.py --check
shell: /usr/bin/bash -e {0}
env:
SF_DOTNET_TFM: net10.0
pythonLocation: /opt/hostedtoolcache/Python/3.11.15/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.11.15/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.11.15/x64/lib
DOTNET_ROOT: /usr/share/dotnet
Coverage spec validation problems:

  • SDK count mismatch for 'Python test_sdk.py run_test assertions': spec claims 31 but file has 32 (pattern: 'run_test\(' in tests/py/test_sdk.py). Update _SDK_COUNT_CLAIMS and the §6 caption in coverage_spec.py.

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@copilot test coverage issue.

2026-07-06T23:58:37.9335354Z ##[group]Run python tests/gen_test_coverage.py --check...

Fixed in the latest commit. Updated _SDK_COUNT_CLAIMS in tests/gen_test_coverage.py to match the actual count of run_test( calls in tests/py/test_sdk.py (31 → 32). python tests/gen_test_coverage.py --check now passes cleanly.

@rijesha
rijesha merged commit 0794710 into develop Jul 7, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants