Skip to content

fix: seed a vehicle discovered mid-dispatch from its triggering config event - #30

Closed
Bre77 wants to merge 2 commits into
mainfrom
fm/pts-config-event-seed-race
Closed

fix: seed a vehicle discovered mid-dispatch from its triggering config event#30
Bre77 wants to merge 2 commits into
mainfrom
fm/pts-config-event-seed-race

Conversation

@Bre77

@Bre77 Bre77 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Intent

  • listen() dispatches each event over a sorted snapshot of _listeners.values() taken before iterating, so a callback that adds a listener mid-dispatch can't mutate the dict out from under the loop.
    • That snapshot has a gap: a generic public listener that discovers an uncached VIN and calls get_vehicle() while handling that VIN's own config event registers a new internal config-sync listener that the current snapshot already passed by. It never sees the event that triggered its own creation, so the vehicle stays at fields={}/preferTyped=None until (if ever) a later config event arrives.
  • Fix: after the snapshotted dispatch completes, give any listener added during it one more pass at the same event, so a mid-dispatch-created vehicle is seeded correctly rather than left empty.
    • tests/test_stream_lifecycle.py adds a regression test reproducing the race (fails without the fix) and asserting the newly discovered vehicle's fields/preferTyped come from the triggering event.

Bre77 added 2 commits August 12, 2026 15:21
A generic listener that discovers an uncached VIN via get_vehicle()
while handling that VIN's config event registers a new internal
listener after listen() has already snapshotted the registry for the
current event. The new vehicle's config-sync listener then misses the
very event that revealed its config, leaving it at fields={}/
preferTyped=None until (if ever) a later config event arrives.
@Bre77 Bre77 added the fm Opened by a Firstmate crewmate label Aug 12, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

if not self.stream.connected:
return False
topics = self.stream.topics
return topics is None or SseTopic.CONFIG in topics

P2 Badge Wait for the config snapshot before trusting cached state

After a reconnect, connected becomes true as soon as the SSE response is installed, before its connect-time config snapshot is dispatched. If the server-side config changed while disconnected and an add_field() or prefer_typed() call runs in that window—for example from a connection listener—the stale pre-disconnect record can match the request, causing the method to skip its PATCH; the later snapshot then exposes the mismatch but does not retry the request. Track whether the current connection's config snapshot has been consumed before allowing the no-op optimization.

AGENTS.md reference: AGENTS.md:L21-L21

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Bre77

Bre77 commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

Superseded by #32 - a simpler fix (populated flag + lazy fetch, no second dispatch pass) per captain direction. Closing.

@Bre77 Bre77 closed this Aug 12, 2026
Bre77 added a commit that referenced this pull request Aug 12, 2026
… fetch (#32)

add_field/prefer_typed no longer gate their no-op skip on connection/topic
state (_record_is_live). Instead, TeslemetryStreamVehicle tracks whether
its record has ever been populated (by a push event or a REST fetch); an
unpopulated vehicle awaits a single-flight get_config() before deciding,
a populated one trusts the optimistically-updated record outright. A
disconnect notification unpopulates the record so a call landing in the
reconnect window (before the next connection's config snapshot arrives)
refetches instead of trusting stale pre-disconnect data.

This replaces the mechanisms proposed in #30 (a second dispatch pass for
listeners added mid-event) and #31 (per-connection snapshot tracking) with
a smaller, self-contained fix scoped to the vehicle itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fm Opened by a Firstmate crewmate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant