fix(server): correct MessageData annotations and materialize dynamic fields (#49)#80
Open
ff225 wants to merge 1 commit into
Open
fix(server): correct MessageData annotations and materialize dynamic fields (#49)#80ff225 wants to merge 1 commit into
ff225 wants to merge 1 commit into
Conversation
…fields (#49) device_id/message_id are str (not int), message_content is a dict (not str), and timestamp is a float (not str) at the only construction site in inference_protocol.py. The post-decode attributes were also plain class variables without annotations, so they never became real dataclass fields despite being treated as instance state everywhere they're used.
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.
Summary
MessageData's core field annotations didn't match runtime types:device_id/message_idarestr(decoded from bytes ininference_protocol.py),message_contentis adict, andtimestampis afloat(<din the wire format), notint/stras declared.received_timestamp,avg_speed,latency,synthetic_latency,payload_size,offloading_layer_index,layer_output,device_layers_inference_time,device_cpu_percent) were unannotated class variables, so@dataclassnever turned them into real fields even though every caller treats them as instance state.Scoped to the typing-correctness part of #49 (annotations + materializing the dynamic attributes as fields). Left
get_latency's string-coercing signature and the broader "adopt TypedDict/slots for recurring config structures" follow-up out of scope — no single obvious target and no behavior change needed there.Note:
payloadstaysstr— the only construction site (inference_protocol.py) always passes""and no code readsmessage_data.payload, so there's no evidence it's ever actuallybytestoday; flagging in case that's stale intent from before the protocol refactor.Test plan
tests/unit/test_message_data.py: new tests assert the core field annotations match runtime types, the previously-dynamic attributes are realdataclasses.fields, and they default toNone.uv run pytestfast suite (217 passed, 1 skipped)ruff check/ruff format --checkon changed files