Skip to content

fix: attribute recorded sessions to the working directory they ran in - #94

Merged
Salil Das (sadlilas) merged 5 commits into
mainfrom
fix/session-working-dir-attribution
Sep 1, 2026
Merged

fix: attribute recorded sessions to the working directory they ran in#94
Salil Das (sadlilas) merged 5 commits into
mainfrom
fix/session-working-dir-attribution

Conversation

@colombod

@colombod Diego Colombo (colombod) commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

What this ships

The Context Intelligence server records the working directory each session ran in and stores it on the Session node, so it can be queried afterwards.

What it fixes

The hook has emitted working_dir as a top-level envelope field since the client-side change in August, and post_events has been persisting it verbatim with every event — but the drainer discarded it when parsing the queue line, so it never reached the graph.

Note on scope: workspace is already the slugified working directory, so grouping sessions by project works today. What was missing is the exact path — slugging is lossy (/home/user/my-app and /home/user/my/app are indistinguishable once slugged), and the only inverse available is a best-effort approximation. This recovers the real path.

Behavior

  • working_dir is read off the durable queue line, not bound to the in-memory worker. That is what makes it survive a restart: a drainer respawned by crash recovery or dead-letter replay never sees the original HTTP request, but it replays the same bytes. No changes to either recovery path were needed.
  • It is threaded as an explicit keyword to the Session-node write rather than injected into datadata is stored verbatim as a blob on every Event node, so injecting it there would duplicate the path per event.
  • Populate-if-missing: written when the node has no value, never overwritten once set. Enforced twice — in ensure_session_node, and again at the Neo4j MERGE via coalesce(n.working_dir, row.working_dir), so the rule also holds for concurrent writers and replayed batches, not just for writers that went through this process's cache.
  • Because the backfill also applies to nodes that already exist, re-importing a session recorded before this change fills the value in.
  • Sessions that never report a working directory leave the property unset. None means "not reported", which is not the same as the empty path; whitespace-only values are rejected at ingest.
  • The value lives on the Session node only. Every Event is one HAS_EVENT hop away, so there is no per-event copy on the highest-volume write path.

Tests

Full suite: 1967 passed, 4 skipped — including all 79 live-Neo4j tests against a real container. ruff format/check clean; pyright error count unchanged from main.

New coverage:

  • the envelope field surviving to the durable queue line (and staying out of data)
  • a fresh worker draining a persisted line — the crash-recovery shape
  • backfill of a Session node that predates working_dir
  • non-overwrite when an event reports a different folder
  • live-Neo4j gates proving the coalesce holds under a second writer, and that a row carrying no working_dir does not null an existing value
  • _parse_line normalization (absent and "" both yield None)

Each was mutation-checked: reversing the coalesce, dropping the queue-line read, and removing the backfill branch each fail the corresponding test.

Note on the second commit

Ownership of this PR transferred, and the mechanism was reworked. The first commit bound working_dir to the SessionWorker at creation time, which lost it on every restart (crash recovery and dead-letter replay create workers through call sites that had no access to it) and could never backfill, since ensure_session_node's node-exists branch returned early. The second commit moves the read to the queue line and adds the DB-level coalesce, and reverts what that made unnecessary: the get_or_create signature change, the HookStateService constructor param, and the per-Event-node stamp.

docs/architecture/03-graph-model.dot documents the new Session property; the PNG is regenerated.

The server now records the working directory each session ran in and
stores it on the session and on each of its events, so it can be queried
afterwards. The working directory was already being sent, but the server
was discarding it, so recorded activity could not be tied back to the
project or folder it came from.

The value is taken from the first event that reports one and is never
overwritten once set, so re-importing earlier sessions fills it in
safely. Sessions that never report a working directory leave the field
empty.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…replay

The original mechanism bound working_dir to the in-memory SessionWorker at
creation time, losing it on every server restart because crash recovery and
dead-letter replay paths created workers without it. It also could never
backfill pre-existing sessions, contradicting the stated design intent.

This rework reads working_dir off the durable queue line instead
(registry._parse_line). The value was always persisted verbatim in post_events,
so reading it there makes crash recovery and dead-letter replay correct without
any changes to those paths. The working directory is now threaded as an explicit
keyword through the processing pipeline (not injected into the data blob).

ensure_session_node now applies populate-if-missing on both branches: writes on
new nodes and backfills pre-existing nodes without one, never overwriting
already-set values. neo4j_store uses coalesce on the Session MERGE to enforce
'never re-attribute an attributed session' as a database invariant that holds
across concurrent writers and replayed batches.

Reverted unnecessary changes from the original: the get_or_create signature
change, HookStateService constructor param, and per-Event-node stamps in the
data handler (Session node holds the value; every Event is one hop away, so a
per-event copy costs strings on the highest-volume write path for no query gain).

All 79 live-Neo4j tests pass; ruff and pyright clean.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ibution

PR #92 rewrote the drain loop to return Records with their own start/end cursors,
moving the terminal-check logic into _process_batch returning (safe_count, terminal_at)
and dispatching session:end twice deliberately.

This branch threads session working directory attribution through that same rewritten
code. Conflict resolution strategy: accepted main's entire drain/terminal logic, then
re-applied working_dir threading on top by:

- _parse_line now returns working_dir as a fourth element
- _process_batch and _handle_exhausted_batch forward it to _process_one via rec.raw
- Updated five test files that stub process_event with fixed positional signatures
  to accept the new keyword argument
- Updated one direct _parse_line call site to unpack four values

All 2037 tests pass (7 skipped), ruff clean, mutation checks correct.
…anly

SessionRegistry._parse_line now returns a 4-tuple including the session
working directory. This test was unpacking only the first three values,
causing a type mismatch caught by Pyright. Fixed by unpacking the fourth
value into _working_dir.

The test was previously skipped via @_requires_seeds, making the mismatch
invisible to the normal test suite. Verified by synthesizing the missing
seed fixture locally and confirming the test passes.

Generated with Amplifier (https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@sadlilas
Salil Das (sadlilas) merged commit c9600b9 into main Sep 1, 2026
3 checks passed
@sadlilas
Salil Das (sadlilas) deleted the fix/session-working-dir-attribution branch September 1, 2026 14:21
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.

2 participants