Orchestrata Ultimate › Plans › session-handler-split-design

SessionHandler Cohesion Split — Design

Source: plans/session-handler-split-design.md at 8a2cb42ad · last changed 2026-08-05 · first committed 2026-08-05 · 302 lines.

Prose pending.

Plan text

SessionHandler Cohesion Split — Design

Status: DESIGN ONLY (RUL-022 — no implementation in this artifact) Branch: feat/cxp-direct-transport · Date: 2026-08-05 · Author: analyst (design phase) Subject: src/orchestrata_ultimate/gateway/handlers/session.py — SessionHandler (class span 80–687). Trigger: reviews/code-review-2026-08-05-064151.txt — [WARN] CLASS_TOO_LONG (607 lines > 200) + [WARN] CLASS_TOO_MANY_METHODS (24 methods), plus two [INFO] METHOD_NO_SELF_ACCESS on _build_list_query / _shape_get_result. Contract diagram: plans/contracts/session-handler-split.mmd.

Every as-built claim below was grounded via the AST toolkit (scripts/serialize_codebase_ast.py {symbols,refs,source}), never by hand-reading .py files. Rulings consulted: RUL-003 (env/root read at call time, no literal fallbacks — the _project_slug call-time contract), RUL-011 (session root injected, never inferred), RUL-022 (design before build), RUL-023 (wired is done). Governing user rulings: refactor by cohesion, not LOC; no mocks; 5-min test cap; MVP discipline — no new features, no schema changes.

Reviewer threshold facts (measured, decisive): scripts/code-review.py:677 fires CLASS_TOO_LONG at > MAX_CLASS_LENGTH (=200); :943 fires CLASS_TOO_MANY_METHODS at len(methods) > 10. SessionHandler's frozen public surface is 11 methods (7 handle_* bound into the RPC registry + 3 set_* seams + __init__), so CLASS_TOO_MANY_METHODS cannot be fully cleared without breaking the public RPC surface. This design reduces 24→13 methods and 607→~300 lines and treats the remainder as a structural residual (§2.3), per the cohesion-not-LOC ruling.

1. AS-BUILT INVENTORY (AST-verified)

1.1 The 24 methods — line, responsibility bucket, @traces

def-start lines from symbols; traces read from source gateway.handlers.session.SessionHandler. Class-level: @traces(spec=014, story=US-004, reqs=[REQ-014-051,-053,-054,-056,-059,-061,-063,-064])

• @traces(spec=016, story=US-013, reqs=[REQ-016-245..252]).

Module level (outside the class): _resolve_session_root (:33, constructor concern, stays), _deserialize_session (:690, KG row → dict — moves with the KG bucket), _neo4j_value_to_dict lives in gateway/handlers/utils.py:8 (shared sibling-module precedent this design reuses).

Schema-drift contract: the coalesce(s.id, s.session_id) as session_id projection appears in four query texts (handle_active inline, _build_list_query, _build_get_query) plus the s.id/session_id dual-write in _build_open_query. It is a single contract scattered across the class — the strongest cohesion argument for one KG-contract module.

1.2 Callers — the production wiring chain (via refs, not guessed)

• Construction: gateway/rpc.py:95 — RPCHandler.__init__ → self._session = SessionHandler(pubsub, session_root=session_root).
• Seam fan-out (all on RPCHandler): set_processor (rpc.py:155–165, session at :164) · set_graph_repository (rpc.py:167–189, session at :184) · set_memory_engine (rpc.py:191–204, session at :204, @traces REQ-010-032).
• Boot callers of those seams: gateway/server.py:368 (set_processor), :422 (set_memory_engine), :459 (set_graph_repository).
• RPC registry: gateway/rpc.py _build_method_registry binds bound methods — session.list/get/active/open/close/events/facts.subscribe → handle_*. The bound methods are captured at registry build, so all 7 handle_* must remain methods on SessionHandler — moving any of them changes the production dispatch table.
• Private-helper callers: every one of the 12 helpers + _execute_cypher has call sites only inside session.py (refs: 1 use each, except _execute_cypher with its 5 in-class uses; the knowledge/projects handlers define their own same-named _execute_cypher — no sharing).

1.3 Test reach-in audit (the behavior floor)

76-test floor (counted via symbols --pattern test_): tests/gateway/handlers/test_session.py (35) + tests/gateway/test_session.py (20) + tests/generated_gateway/test_handlers_session_generated.py (21).

refs scans of all 15 private helper names and the 5 private attribute names (_graph_repository, _processor, _memory_engine, _session_controller, _pubsub) against the three floor files: zero hits. The floor exercises only the public surface

• the three set_* seams. The floor passes unchanged under this design.

Four additional test files import SessionHandler (adjacent scope): tests/gateway/test_handlers_behavioral.py, test_memory_engine_wiring.py, test_publisher_wiring.py, test_rpc_behavioral.py. Private reach-ins among them — all in test_rpc_behavioral.py only:

2. EVALUATING THE STARTING RECOMMENDATION

Recommendation: thin SessionHandler facade + extract SessionKgOps (query building + persistence + coalesce contract) + SessionEventOps (broadcasts). Verdict: directionally right; refined on one point with threshold evidence.

2.1 Refinement — pure query builders become module functions, not SessionKgOps methods

A single SessionKgOps carrying all 9 KG methods plus the 2 not-yet-extracted inline query literals would measure ~270 lines / 12+ methods — it would re-trip both WARNs it exists to fix (thresholds: >200 lines, >10 methods). The evidence-backed cut:

• Pure Cypher builders → module-level functions in the new _session_kg.py (6 functions, no state, explicit args). This is exactly what the reviewer's two METHOD_NO_SELF_ACCESS INFOs already recommend for _build_list_query / _shape_get_result, and matches the in-repo precedent of shared module-level helpers in gateway/handlers/utils.py (_neo4j_value_to_dict) and Cypher-builder modules (knowledge_graph/relationships.py). Module functions trip no CLASS_* rule.
• Stateful I/O + identity → class SessionKgOps (8 methods, ~135 lines — under both thresholds).

2.2 SessionEventOps — kept, with the ownership consequence stated

It removes only 2 methods, but it takes sole ownership of _processor and the session.events wire-payload shapes. Cost: the single mechanical assertion update in §1.3 (outside the 76-floor, explicitly flagged). Accepted.

2.3 Honest residual (numbers, not adjectives)

Post-split SessionHandler: 13 methods (11 frozen public + _end_active_session + _migrate_close_memories — both are the handler's own lifecycle/engine-seam domain; moving them would be metric-chasing) and ~300 lines (7 documented RPC endpoints + seams). Both WARNs therefore shrink but persist: 24→13 methods (limit 10), 607→~300 lines (limit 200). Full clearance is impossible while the RPC registry binds 7 bound methods + 3 seams on this class. User-decision item (surfaced, not packeted): code-review.py's flat >10 methods / >200 lines thresholds flag every RPC facade in gateway/handlers/ by construction; if the residual WARNs are unacceptable, the fix is review-policy (facade-aware thresholds), not further splitting of this class.

3. THE CONTRACT (see plans/contracts/session-handler-split.mmd)

New homes (naming precedent: private sibling modules _policy_gate.py, _scope_guard.py):

• src/orchestrata_ultimate/gateway/handlers/_session_kg.py — the entire session KG contract in one file: 6 pure builder functions + _deserialize_session + class SessionKgOps.
• src/orchestrata_ultimate/gateway/handlers/_session_events.py — class SessionEventOps.

All moves are verbatim (byte-identical Cypher strings, docstrings, log messages, timeout values); the only new code is glue (constructor lines, available, the two extracted-literal builder wrappers, forwarding setters). @traces decorators move with their methods, ids unchanged. No new Tracer composition (deliberate deviation from the analyst charter default: behavior-preserving split, no new features; the existing module logger calls move verbatim with persist_*).

3.1 Module functions — _session_kg.py (patternRole: leaf; pure)

The coalesce(s.id, s.session_id) schema-drift contract now lives only in this module.

3.2 class SessionKgOps — stateful KG I/O + identity (composes: SessionController, AsyncGraphRepository)

SessionKgOps(session_controller: SessionController, graph_repository: Any = None)
  set_graph_repository(graph_repository: Any) -> None          # 016-246 (from :175)
  available -> bool          [property, new one-liner]         # 016-246
  project_slug() -> str                                        # 016-245 (from :120; call-time root read preserved — RUL-003)
  session_key(session_id: str | int) -> str                    # 016-245 (from :133)
  async execute(query: str, params: dict | None = None, *, write: bool = False) -> list[dict[str, Any]]
                                                               # 016-246 (from _execute_cypher:660)
  async persist_open(session: Any, goals: Any, portfolio: str, purpose: str) -> None
                                                               # 016-249 (from :447; wait_for(10.0) + warning verbatim)
  async persist_close(session_id: str, summary: Any, carryover: Any) -> None
                                                               # 016-250 (from :536; query text via build_close_query)

Holds _graph_repository (sole owner; verified zero external readers). Holds the live SessionController reference and reads only .root, at call time, never cached.

3.3 class SessionEventOps — broadcast ops (composes: MessageProcessor)

SessionEventOps(processor: Any = None)
  set_processor(processor: Any) -> None                        # 016-246 (from :151)
  async broadcast_opened(session: Any, purpose: str) -> None   # 016-249 (from _broadcast_open_event:470)
  async broadcast_closed(session_id: str, migrated: int) -> None
                                                               # 016-250 (from _broadcast_close_event:583)

Sole owner of _processor; the no-processor guard moves with the methods.

3.4 SessionHandler — existing, MODIFIED (13 methods; public surface byte-compatible)

• __init__: adds self._kg = kg_ops or SessionKgOps(self._session_controller) and self._events = event_ops or SessionEventOps(processor) (new optional kg_ops= / event_ops= DI params, defaulting to real construction — rpc.py:95 unchanged); drops self._graph_repository / self._processor.
• set_processor → forwards to self._events.set_processor (signature unchanged).
• set_graph_repository → forwards to self._kg.set_graph_repository (signature unchanged).
• set_memory_engine, _end_active_session, _migrate_close_memories, handle_events, handle_facts_subscribe: unchanged.
• handle_active/list/get: guard becomes if not self._kg.available:; query via the module builders; execution via await asyncio.wait_for(self._kg.execute(...), 10.0) — envelopes, error strings, and timeouts stay in the handler, byte-identical.
• handle_open: await self._kg.persist_open(...) + await self._events.broadcast_opened(...).
• handle_close: await self._kg.persist_close(...) + await self._events.broadcast_closed(...).

Class-level @traces (both 014 and 016 sets) stay on SessionHandler unchanged.

3.5 Wiring statement (Hard Rule 8 — built is not done, WIRED is done)

No new wiring is required, and that is the point of the facade: the production chain gateway/server.py:{368,422,459} → RPCHandler.{set_processor, set_memory_engine, set_graph_repository} (rpc.py:155–204) → SessionHandler.set_* → composed ops, and RPCHandler.__init__ (rpc.py:95) → SessionHandler.__init__ → real SessionKgOps / SessionEventOps construction. The new modules are production-reachable the moment session.py imports them; scripts/check_wiring.py (RUL-023) sees them through the existing importer.

4. BEHAVIOR-PRESERVATION CONSTRAINTS

1. 76-test floor passes UNCHANGED — verified feasible: zero private reach-ins (§1.3).
2. One flagged mechanical update, outside the floor: tests/gateway/test_rpc_behavioral.py::test_set_processor_with_real_processor, line 125: rpc._session._processor is processor → rpc._session._events._processor is processor. This is the complete list; no other test in the repo touches a moved private name.
3. Parity invariants (reviewer checklist for every packet): byte-identical Cypher (all four coalesce sites + HAS_SESSION MERGE + close-update), identical response envelopes and error strings ("Neo4j unavailable", "Query timeout", "Missing session_id", "Missing purpose", "PubSub unavailable"), identical asyncio.wait_for timeouts (10.0), identical logger.warning texts ("session.open KG write failed: %s" / "session.close ..."), identical session.events broadcast payload shapes, _project_slug still a call-time read.
4. No schema changes, no new features, no new deps. RPC registry untouched.

5. BUILD PACKETS (refactorer-sized, <5 min each, behavior-preserving)

Shared gate (the floor + the only reach-in file; 5-min cap):

timeout 300 python scripts/run_tests_streaming.py \
  tests/gateway/handlers/test_session.py tests/gateway/test_session.py \
  tests/generated_gateway/test_handlers_session_generated.py \
  tests/gateway/test_rpc_behavioral.py -j 4

Each packet ends green on its gate before the next starts; P1→P2→P3 are ordered (P2 consumes P1's module; P3 is independent of P2 but sequenced to keep diffs reviewable).

6. SEQUENCING / MERGE-ADJACENCY

git status: src/orchestrata_ultimate/gateway/handlers/session.py is modified and uncommitted — it carries the 2026-08-05 SET-2 work (coalesce fix + the 12 cohesion helpers + traces) that this design decomposes. All packets land strictly AFTER the SET-2 commit; starting earlier would entangle the split with the un-reviewed helper pass and make both diffs unreviewable. No other in-flight work touches gateway/handlers/session.py or gateway/rpc.py (the kernel-dispatch-reply design's gateway edits are in gateway/handlers/cxp.py / gateway/server.py — disjoint files; only test_rpc_behavioral.py is shared surface, and only P3's one-line update touches it).

Related

• Plans
• Orchestrata Ultimate — project index
• Workflow
• Tasks