Skip to content

rework state to remove annoyances and inconsistensies, prep for Stage and UI APIs#278

Merged
timkpaine merged 9 commits into
v3from
tkp/staterework
May 26, 2026
Merged

rework state to remove annoyances and inconsistensies, prep for Stage and UI APIs#278
timkpaine merged 9 commits into
v3from
tkp/staterework

Conversation

@timkpaine

Copy link
Copy Markdown
Member

This PR has a comprehensive rework of the State declaration, in preparation for the new Stage and UI features in v3.

We expose 2 APIS:

  • State as annotation on channel
  • set_state method on the channels object

This removes the annoying an error-prone s_ prefix, and allows for multiple state accumulation via checked alias channels.

@timkpaine timkpaine added the type: feature Feature requests label May 22, 2026
… and UI APIs

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
@arhamchopra

arhamchopra commented May 22, 2026

Copy link
Copy Markdown
Collaborator

This was something that was on my mind for a while, but never got around to doing it.
In the keyby argument, we can only pass a direct member of the structs currently.
Can we extend the keyby to support DOT (".") paths?
i.e

class Sub(csp.struct):
    id: int

class MyStruct(csp.Struct):
    sub: Sub

keyby = sub.id

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Comment thread csp_gateway/server/demo/omnibus.py Outdated
@github-actions

github-actions Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Test Results

628 tests  +628   620 ✅ +620   7m 14s ⏱️ + 7m 14s
  1 suites +  1     8 💤 +  8 
  1 files   +  1     0 ❌ ±  0 

Results for commit 8abadad. ± Comparison against base commit e81e130.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.09091% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.24%. Comparing base (e81e130) to head (8abadad).

Files with missing lines Patch % Lines
csp_gateway/server/gateway/csp/channels.py 77.50% 27 Missing ⚠️
csp_gateway/server/web/app.py 52.94% 8 Missing ⚠️
csp_gateway/server/web/routes/state.py 71.42% 4 Missing ⚠️
csp_gateway/server/gateway/csp/factory.py 91.66% 1 Missing ⚠️
csp_gateway/server/gateway/csp/state.py 96.15% 1 Missing ⚠️
csp_gateway/server/modules/web/mount.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               v3     #278      +/-   ##
==========================================
- Coverage   87.33%   87.24%   -0.09%     
==========================================
  Files         142      142              
  Lines       14452    14572     +120     
==========================================
+ Hits        12621    12713      +92     
- Misses       1831     1859      +28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

timkpaine added 5 commits May 22, 2026 12:28
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Show keyby/indexer in API docs for state
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>

# Conflicts:
#	csp_gateway/server/gateway/csp/channels.py
#	csp_gateway/server/web/app.py
#	csp_gateway/server/web/routes/state.py
- Resolve dotted attribute paths in DefaultState/DuckDBState insert so keyby can reference nested struct members (e.g. keyby='sub.id')

- Make --junitxml explicit in Makefile and switch upload-artifact/publish paths to 'junit.xml' so CI test results are reported (previously '**/junit.xml' produced 0 tests in the PR comment)

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Comment thread csp_gateway/server/gateway/gateway.py
Comment thread csp_gateway/server/gateway/csp/channels.py Outdated
@texodus texodus marked this pull request as ready for review May 22, 2026 17:58
timkpaine added 2 commits May 22, 2026 15:22
…_channels

- channels.py: docstring referenced old set_state(edge, alias, keyby, indexer) signature; correct it to set_state(field_or_edge, keyby, indexer=None)

- module.py: drop dynamic_state_channels() — no longer consumed by Gateway after the state rework. State on dynamic channels is now wired via channels.set_state() inside Module.connect, which is exercised by test_dynamic_channels

- test_gateway.py: drop the matching dynamic_state_channels override; suite still passes (482 server tests)

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
…pendent

The prior commit (fbf0d46) wrongly removed Module.dynamic_state_channels() under the assumption it was dead code. It actually serves a real purpose: it lets a module declare which of its dynamic_channels will have state, so that *other* modules' connect() may call get_state() on that name regardless of the order in which modules are connected.

This commit restores the API and fixes the underlying order dependence using the same DelayedEdge pattern already used for channel data:

- module.py: re-add dynamic_state_channels() -> Optional[Set[str]] on the Module ABC, with docstring describing the order-independence guarantee.

- channels.py:

  * Add _pending_state_element_types: Dict[str, type] and _delayed_state_edges: Dict[(field, indexer), DelayedEdge] private attrs.

  * Add _declare_dynamic_state(field, element_type) to pre-register a state name with its element type.

  * get_state: if the field is pre-declared but not yet wired, return a DelayedEdge of ts[State[T]] (lazily created and cached).

  * _wire_state_edge: if a DelayedEdge was previously handed out for (field, indexer), bind it to the freshly-built state node so consumers' edges resolve transparently.

  * set_state: clear the pending entry once the owning module wires the state for real.

- factory.py: in ChannelsFactory.build(), before invoking each module's connect(), walk all enabled modules and call channels._declare_dynamic_state(name, T) for every name returned by Module.dynamic_state_channels() (T is unwrapped from List[T] when applicable). This guarantees ordering independence between get_state and set_state across modules.

- test_gateway.py:

  * Re-add the dynamic_state_channels() override on MySetModuleDynamicChannels.

  * Add parametrized test_dynamic_channels_state_module_order_independence covering both [setter, getter] and [getter, setter] module orderings; both pass via DelayedEdge binding.

Server suite: 484 passed, 7 skipped, 1 xfailed (was 482; +2 for the new order test).

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
@timkpaine timkpaine requested a review from ptomecek May 23, 2026 15:56
@timkpaine timkpaine merged commit e44bc09 into v3 May 26, 2026
10 of 12 checks passed
@timkpaine timkpaine deleted the tkp/staterework branch May 26, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature Feature requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants