Context Intelligence data-quality & operability (Phase 2): trustworthy metrics, session attribution, durable fix, incomplete-label heal, blob reclaim - #70
Conversation
d055c33 to
d1be7c4
Compare
…longer MERGEs across orchestrator runs (also fixes I3)
- Iteration.node_id is now composed with the active orchestrator-run identifier: {session_id}::orch_run::{ts}::iteration::{N}, using the run disambiguator OrchestratorRun already tracks. Previously node_id was {session_id}::iteration::{N} with N restarting per run, so one Iteration node MERGEd across up to 15 runs (last-write-wins garbage on usage_input/usage_output/usage_cache_write/message_count; count(DISTINCT Iteration) undercount; run→Iteration→ToolCall overcount).
- The HAS_PART edge and the tool_call / content_block / skill_load cursors carry the new id (they treat it as opaque). The per-session iteration counter is intentionally NOT reset per run — that would collide ContentBlock node_ids ({session_id}::block::{N}::{idx}); run-uniqueness comes from the ::orch_run:: segment.
- This ends the last-write-wins collisions, so I3 (usage_cache_write junk) is fixed as a direct consequence. Forward-only: already-merged historical Iteration nodes are not retroactively split.
- Validated: server unit suite green (1772 passed / 2 skipped) + the delegation/skill integration test (6 passed); previously DTU-proven (new session max run-parents-per-Iteration = 1 vs 2 pre-fix in the same graph).
Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…operty (populate-if-missing) - EventRequest gains an optional top-level working_dir; post_events lifts it into the event data so it rides the existing pipeline to ensure_session_node, which sets Session.working_dir. Pairs with the bundle hook + upload tool now emitting working_dir (Phase-1 branch). - Populate-if-missing: fills the property from any event that carries a non-empty working_dir — including re-imports of pre-existing sessions via the upload tool — and never clobbers an already-set value; idempotent. Forward-only: sessions with no working_dir-bearing event stay null. - Startup is unchanged from base (no migration/rectification added here — the schema-version-awareness and server-side migration workflow are deferred to branch spike/self-service-upgrade for redesign). - Validated: server unit suite green (1772 passed / 2 skipped); previously DTU-proven end to end (live session populates Session.working_dir; imported JSONL populates it; re-import of an existing NULL session backfills it with no-clobber + positive control). Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…des (I5 remediation, non-destructive) Adds scripts/tag_legacy_pooled_iterations.py: a standalone, idempotent, dry-run+apply maintenance script that marks ONLY the confirmed-corrupt pre-fix Iteration nodes with `data_quality='legacy_pooled_pre_fix'`. Confirmed-corrupt = bare node_id (no `::orch_run::`) AND >=2 distinct OrchestratorRun HAS_PART parents (i.e. actually MERGEd across runs by the I5 bug). It deliberately does NOT touch single-run bare-id nodes (clean, ~93-95% of legacy), no-run-edge nodes (unconfirmable), or run-scoped nodes. This puts the corruption signal in the data itself, so a naive `MATCH (i:Iteration)` consumer can see it — not only skill users routing around it. Why tag and not delete: a council review + live-data verification (private-home-server 2,960/66,131 = 4.48%; team-shared 7,467/181,778 = 4.11%) showed a shape-based delete (`NOT node_id CONTAINS '::orch_run::'`) would destroy ~95% VALID single-run data — "old" != "corrupt". Only ~4% of legacy nodes are actually corrupt. The destructive DETACH DELETE of the tagged subset is a deferred, gated follow-up (dry-run count, all-producers-rolled, degree-batched, ContentBlock-orphan aware), not part of this commit. Adds a regression assertion to test_iteration.py: a multi-run session must produce Iteration nodes that each have exactly ONE distinct HAS_PART OrchestratorRun parent and distinct run-scoped node_ids (fails loudly if run-scoping is ever reverted, which would re-MERGE across runs). This is the recurrence guard — a test, not a runtime server check. Adds CHANGELOG.md (newest-first) documenting the data-quality work on this branch (I5 run-scoped node_id forward-only; I1 working_dir lift populate-if-missing forward-only; the tag script). No SCHEMA_VERSION/ledger/migration-runner machinery — the general self-service migration mechanism stays deferred to branch spike/self-service-upgrade. Validation: script --help ok; iteration tests 22 passed; full unit suite 1772 passed / 2 skipped. DTU-validated on synthetic data across all four categories (5/5 PASS): tags exactly the confirmed-corrupt nodes, leaves clean/no-run-edge/run-scoped untouched, idempotent (0 rows on re-apply). Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… rebuild (+ iteration_scope, schema_version baseline)
I5 gave Iteration.node_id a run-scoped shape, but that shape depended on
in-memory DataLayer2State (execution_start_ts, iteration_count) that was
rebuilt EMPTY on any worker (re)creation. Two paths triggered this: a
process crash-restart with an undrained tail, and a stale-session reap
(which had no recovery path at all). Either one regressed node_id back to
the pre-fix bare shape and re-pooled Iteration nodes across orchestrator
runs, reopening I5 and I3.
Fix: persist the full DataLayer2State + DataLayer3State cursor ATOMICALLY
with the queue offset — folded into the .offset record as a single
os.replace, with the legacy bare-int .offset still read for back-compat —
and restore it exactly once at the top of drain_worker, the single
worker-construction path that covers both crash-restart and stale-reap.
This also preserves E09/E14/E15/E10-E11 edges across a rebuild.
Phantom-cursor guard: a dead-lettered line's in-memory mutations are now
rolled back before commit, so the persisted cursor can never point at a
discarded node. Written failing-test-first.
iteration_scope ("run" | "unscoped") is stamped at all three Iteration
upsert sites, so a node can never be created without a scope value; logged
at INFO.
schema_version baseline (data points only — no upgrade/handling logic):
a SCHEMA_VERSION=1 constant is exposed on /version, and a
(:SchemaMeta{id:'singleton'}) node is created create-if-absent (ON CREATE
only) behind a uniqueness constraint. This write is STARTUP-ONLY — moved
out of ensure_neo4j_schema (which runs per-worker-flush) into
ensure_schema_version_baseline(), called once from the lifespan so it is
single-writer with no concurrency.
Validated: 1841 non-neo4j tests pass; the full tests/neo4j/ suite (85
passed) ran against a real Neo4j, including end-to-end crash-restart +
stale-reap (duplicate Iteration=0, run-scoped continuity, edge parity,
iteration_scope=run, exactly one SchemaMeta) and a 20-way concurrent
schema_version race resolving to a single node.
Co-authored-by: Amplifier <amplifier@microsoft.com>
…-once-at-end race
IncompleteSession was written once at session:end when the Session node
had no type label yet, and never revised. Forked sub-sessions drain in
independent per-session queues with no cross-session ordering, so a
child's session:end is often processed before its session:fork/
session:start → classify() sees no type → stamps IncompleteSession →
the later fork/start add the real terminal but never clear the stale
marker. Live bisect: ~52.8% of sessions carry the label, ~99.4% are
false positives (the node carries its own linked start/fork event);
genuine loss ~0.5%.
Part 1 (heal-forward, code): classify() now strips IncompleteSession on
EVERY start/fork transition via a single _heal_forward() normalizer
(invariant applied to the return value, not per-branch, so a future
branch can't skip it); the end branch is unchanged (still the real
signal for the genuine ~0.5%). Reuses existing set_labels remove
plumbing — no store/Cypher change. Order-independent. Stale comments
in session.py/_handle_end and delegation.py updated.
Part 2 (one-off backfill, scripts/relabel_incomplete_sessions.py):
standalone, out-of-band, idempotent script that clears IncompleteSession
from provably-false-positive nodes (real terminal type OR a linked
SessionStartEvent/SessionForkEvent), leaving the genuine ~0.5% untouched.
--apply is hard-gated behind a read-only reconciliation diagnostic
(refuses if any linked-but-untyped nodes exist, so the selector
assumption is verified per-DB before mutating). Batched CALL{} IN
TRANSACTIONS; touched-id undo-log with --restore; before/after
population summary; POST-DEPLOY GATE in the docstring (run only after
Part 1 is deployed+verified). SCHEMA_VERSION unchanged (no version
bump; handling deferred).
Validated: 1844 non-neo4j tests pass; full tests/neo4j/ 94 passed
against real Neo4j incl. 3 heal-forward (out-of-order end→fork/start
strips the label physically, no lattice disturbance) and 6 backfill
(clears false positives, retains genuine, idempotent, diagnostic-gate
refuses, restore round-trip).
Tracking: (internal tracker).
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ment IncompleteSession reconciliation Dockerfile now COPYs scripts/ into the runtime image (/app/scripts/). Previously the image copied only context_intelligence_server/, so the standalone out-of-band data-rectification scripts (relabel_incomplete_sessions.py, tag_legacy_pooled_iterations.py, repair_dual_labels.py, ...) were unreachable from a running container -- they could not be run against a live cloud/VM deployment as the migration model requires. Verified: image builds and `docker exec ... python3 scripts/relabel_incomplete_sessions.py --help` works in-image (runtime has python3, not python). CHANGELOG.md now carries a prominent "ACTION REQUIRED ON UPGRADE IF LEGACY DATA IS PRESENT" notice: this release changes IncompleteSession labeling; new events self-heal but historical graphs carry ~52.8% stale markers (~99% false positive) that are NOT auto-corrected; a one-off, out-of-band reconciliation (scripts/relabel_incomplete_sessions.py --dry-run then --apply, gated + idempotent) must be run once after deploy; migrations never run at startup; fresh graphs need no action. Tracking (internal tracker). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…(POST /admin/blobs/reclaim) The blob store (AsyncDiskBlobStore) has no delete/GC; disk under blob_path grows forever. This adds an admin-only, on-demand reclaim of blob files not referenced by any node in the graph. Single POST /admin/blobs/reclaim on the /admin router (require_admin), dry_run defaults TRUE — preview shows exactly what would be deleted; apply requires an explicit max_delete cap. One shared _select_orphans path for preview and apply; apply performs its own authoritative fresh scan. Orphan = on-disk ci-blob://<sid>/<key> absent from a GLOBAL, all-workspace scan of :Event.data (blobs are session-scoped, nodes are (node_id, workspace)-scoped — a per-workspace scan could delete another workspace's live data). Council-gated design (docs/plans/2026-08-12-blob-reclaim-endpoint-spec.md); three blockers closed: - B1 pinned the Event.data reference-carrier invariant with a regression test. - B2 replaced a broken URI regex (silently truncated on a `"`/non-ASCII session_id → happy-path data loss) with structural json.loads walk of $blob_ref, and removed the APOC dual-path. - B3 based in-flight safety on a durable undrained-queue gate (new QueueManager.is_fully_drained, survives kill -9) plus a live-worker check and a min_age_minutes floor of 15. Audit line per delete; idempotent. Validated: 1848 non-neo4j tests pass; tests/neo4j/test_blob_reclaim.py 12 passed against real Neo4j (cross-workspace safety, special-char extraction, undrained-queue/live-worker skip, dry/apply parity, idempotence, max-delete-required, B1 carrier invariant). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ps on un-migrated/unreachable graph state (6.7.1) INCIDENT: deploying the server (a restart) crash-looped the live host under systemd Restart=always because cold-start raised fatally on 1 un-migrated (:Node-lacking) legacy node (a PR #67 guard). On Azure Container Apps the private graph is unreachable to run `doctor --fix`, so this class of failure is an unacceptable hard outage. FIX (boot must NEVER crash-loop, council-gated): 1. ONE loud try/except boundary wraps the ENTIRE lifespan startup body -- setup_logging, Neo4j driver construction, schema init, untagged probe, schema_version baseline, recovery -- so no startup step can crash the worker; on any failure it logs loudly, records degraded/unknown health, and boots anyway. 2. setup_logging() is now resilient: configures a stdout console handler first/unconditionally, file handler + mkdir best-effort, never raises. 3. ensure_neo4j_schema no longer fatal at boot (fail_on_data_conflict=False) and reorders so the :Node index is dropped only AFTER the uniqueness constraint is created; if the constraint can't be created (duplicates) it creates a fallback idx_node_universal so the hot write-path MERGE keeps a NodeIndexSeek (degraded mode costs atomicity only, never the seek). 4. removed the two fatal raises (constraint conflict + untagged-count). 5. recovery iterates sessions defensively -- a corrupt .offset/dead-letter quarantines that one session, boot continues. 6. tri-state schema health (healthy|degraded|unknown; probe failure -> unknown, never coerced to green) surfaced on GET /status with schema_checked_at + degraded_reason; explicit prohibition (code comment + docs) against wiring it to a liveness/readiness probe. Write path UNCHANGED. Migration stays out-of-band (doctor --fix for reachable deployments; maintenance-mode/in-place-fix tracked in (internal tracker) for ACA). VERSION: 6.7.0 -> 6.7.1. VALIDATED: 1853 non-neo4j tests pass; neo4j EXPLAIN tests confirm fallback-index NodeIndexSeek in degraded mode and constraint-backed seek when healthy; NEW real-process boot tests (subprocess against unreachable Neo4j + unwritable log path) prove the server boots, serves GET /status (schema_health unknown/degraded), and stays up -- no crash loop. Independently re-verified via a real server process against an unreachable Neo4j. Tracking: relates to (internal tracker) (maintenance-mode/in-place-fix) and (internal tracker) (schema_version handling). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…h-wide carrier walk, not Event.data-only
POST /admin/blobs/reclaim decided orphans from Event.data ONLY. ci-blob://
refs also live on tool_input (ToolPreEvent/ToolPostEvent/ToolCall), prompt
(PromptSubmitEvent/PromptCompleteEvent/Prompt), and response
(OrchestratorRun). Those were safe today only by an UNENFORCED
pipeline-ordering invariant (every ref also landed in Event.data); a future
ingest change could silently make the GC delete a live blob.
FIX: compute the referenced set graph-wide over the known carrier
properties {data, tool_input, prompt, response} via a UNION ALL of four
single-property predicates (each touching exactly ONE property per row --
deliberately NOT an all-keys/all-node walk, to avoid the AllNodesScan stall
class this codebase is sensitive to). Each value is JSON-parsed and walked
structurally for $blob_ref (wrapper carriers); on parse failure, bare
ci-blob:// tokens are regex-extracted (plain-string carriers). Strict
SUPERSET of the old scan -- can only ever protect MORE blobs, never delete
more. Carrier list is an explicit module constant
(_BLOB_REF_CARRIER_PROPERTIES): adding a new carrier property in future
requires adding it here.
Makes orphan-detection correct by construction, not by pipeline ordering.
Closes the residual risk behind the endpoint spec's original "B1 --
Event.data is the complete reference carrier" (test-pinned) assumption.
VALIDATED: tests/neo4j/test_blob_reclaim.py 16 passed against real Neo4j
(12 original + 4 new: a blob referenced ONLY on ToolCall.tool_input / a
plain-string tool_input / Prompt.prompt / OrchestratorRun.response is now
protected -- each FAILS the old Event.data-only scan, PASSES now); 1853
non-neo4j pass. Equivalence dry-run on the live 242k-blob graph:
referenced_uris 242436 >= prior 242185 (superset), orphans_found 0
(non-regressive), scan completed in 19s (no stall).
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
max_delete had no validator, so a negative value would invert the cap via list slicing: candidates[: body.max_delete] with max_delete=-1 deletes all-but-one instead of enforcing a limit. This is a silent inversion of security intent, not caught by the apply-mode guard (which only checks if max_delete is None at apply-time). Fix adds Field(ge=1) to max_delete so <1 values are rejected at the schema boundary with 422 before the handler is invoked. None is still allowed (represents 'not yet decided', used in dry-run exploration). Tests cover: - max_delete=0 -> 422 (schema floor) - max_delete=-1 -> 422 (regression: inverted-cap blocker) - max_delete=1 -> accepted (the floor value is valid) From the PR review (blocking B-3). Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
The drain_worker retry loop (budget NOT exhausted, common path) replayed a failed batch without restoring cursor state. IterationHandler mutates iteration_count BEFORE its graph write, deriving iteration_id from the current counter value. On a transient flush failure (e.g. DeadlockDetected), the retry would replay the same batch without resetting the cursor, so iteration_count would advance again, creating a new (duplicate) ::iteration::N node for a batch that only ever committed once. This caused permanent counter skew: the counter was 2 but only 1 Iteration node was part of the committed graph. The existing snapshot_cursor()/restore_cursor() guard was wired only into the _handle_exhausted_batch path (budget-spent, give-up branch). Fix extends it to the common retry branch: snapshot the pre-batch cursor once per batch attempt (attempts==0) and restore it before each failed-attempt replay in the budget-not-exhausted retry branch. Tests drive the real process_event loop with a transient-then-success flush failure and assert exactly one Iteration node + unchanged iteration_count; also test a chained-failure (N>=2) case. Before fix: tests fail with duplicate node creation. After fix: tests pass. Proven non-vacuous. From the PR review (blocking B-2). Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…tenance channel Server now refuses ingest AND query with a structured maintenance-503 (Retry-After + JSON reason) while the :Node uniqueness constraint is absent. Gate lives in the drain loop (never latches; offset never advances while gated). Live TTL constraint probe self-clears without restart. /status advertises mode/started_at/elapsed. Allow-list keeps /status, /version, /admin/maintenance reachable with startup assertion. /admin/maintenance (POST triggers run_repair via private admin-driver path with atomic CAS single-flight, returns promptly; GET reports progress). Docs auth-fold (main:app→main:asgi_app). Stale AGENTS.md refreshed for this engagement. Addresses code review blocking item B-1 and must-fix items #1,#2,#3,#4,#5. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Add a validator rejecting blank/whitespace working_dir (None still allowed, unlike workspace). Session-node MERGE now uses coalesce to ensure an existing working_dir value is never clobbered across concurrent writers. The generic hot-path _NODE_MERGE_CYPHER is untouched (working_dir only flows through the Session inline MERGE), preserving the index seek optimization. Addresses code review W-3. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…list Add a mint-site tripwire (assert_carrier_registered) that fails closed if a ci-blob:// ref would be written to an unregistered carrier property. Make the allowlist the single source of truth shared by the mint path (blob_processor) and the reclaim scan (admin.py), removing a second hand-duplicated copy. Addresses code review W-5 (ship-now per human decision). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Bump server 6.7.1 -> 6.8.0 (minor: new /admin/maintenance endpoint). SCHEMA_VERSION stays 1 (no stored-shape change; the rectification dedup/backfill/constraint is structural, not a schema-version step). Establish the Amplifier-consumable, OUT-OF-BAND upgrade path (first schema-adjacent change to need it): - migrations/run.py: standalone CLI. --status (read-only: constraint present? untagged/duplicate counts? server + schema version) and --apply (idempotent rectification via the SAME neo4j_store.run_repair() that /admin/maintenance calls -- one mechanism, two transports). Self-declares from->to, never runs at startup. - migrations/manifest.yaml: lean machine-readable entry (server_version 6.8.0, schema_version 1, schema_affecting false, scripts, gating, verify). - CHANGELOG.md 6.8.0 entry; README "Upgrading" section (healthy = no action; degraded = run migrations/run.py --apply OR POST /admin/maintenance, self-clears without restart), README-reachable per the discoverability rule. Tests: tests/test_migrations_run.py (13 passed). Full non-neo4j suite 1939 passed. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Product doc for the WS-3 maintenance mode: mode state machine, structured 503 + Retry-After contract, /status maintenance fields, allow-list blast radius (which routes stay up vs 503 during maintenance, and why it is intentional), enter/complete log events, the live POST/GET /admin/maintenance endpoint contract, how to clear maintenance (POST /admin/maintenance or migrations/run.py --apply -- both call run_repair; server self-clears via live re-probe, no restart), and the hard warning against wiring the maintenance signal to a k8s/ACA liveness/readiness probe. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
degraded_reason was read from app.state.schema_degraded_reason (written once at boot) while mode/schema_health already came from the live TTL constraint probe. After an in-server repair the gate/mode de-latched correctly but degraded_reason kept asserting the stale "constraint absent" sentence -- a user-facing string that contradicted the live mode. Source it from the coordinator's live reason (the same value that feeds the 503 body), so /status and the 503 stay consistent and the reason clears when the graph is healthy again. No gate/mode behavior change. Found in WS-3 DTU validation. Unit test proves it self-clears across a live constraint-absent -> present flip with no restart. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Align AGENTS.md with what this branch actually implements: "out-of-band" migration/ rectification now has two explicitly-triggered channels sharing one mechanism (run_repair) -- the standalone migrations/run.py --apply and the admin-authenticated POST /admin/maintenance -- while the server still performs zero migration work automatically (assess + advertise + gate only). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…drop Close two coverage gaps from the review: - tag_legacy_pooled_iterations.py (374-line live-graph mutation, previously ZERO coverage): new tests/neo4j/test_tag_legacy_pooled_iterations.py proves the >=2-distinct-parents selector tags a pooled Iteration but never a clean single-parent one, that the --apply gate blocks writes on a dry run, and idempotence (a second apply tags zero additional rows). - idx_node_universal: assert run_repair DROPS the redundant fallback index after it successfully establishes the :Node uniqueness constraint (non-vacuous: the index existed before the call). Real Neo4j: 9 targeted tests pass; full tests/neo4j/ suite 119 passed, no regression. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
apply_relabel() commits per batch (CALL {} IN TRANSACTIONS), and the undo log
was written only after it returned -- so a mid-run crash left committed removals
with no undo record. Now run_apply() reads the full candidate node_id set
(read-only, same _FALSE_POSITIVE_MATCH selector), writes the undo log, THEN
mutates. The pre-mutation set is a superset of what gets removed and restore_ids
is idempotent over it, so a crash leaves a complete, safe undo record; on a clean
run candidate == touched so the reported count is unchanged.
Real Neo4j: new tests prove the undo log equals the pre-mutation candidate set
(not apply_relabel's return value) and that restore heals every node. 8 passed.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…le (W-4)
/version returned only the compiled-in SCHEMA_VERSION and nothing ever read the
stored :SchemaMeta, so a server/graph schema-version mismatch was undetectable.
Add a read-only neo4j_store.read_graph_schema_version() (separate from the write
path -- ensure_schema_version_baseline is untouched) and surface it on GET /status
as additive advisory fields: graph_schema_version (stored) and
schema_version_current (stored == compiled expected, or null). ADVISORY telemetry
only -- it does not gate, refuse, or migrate; full mismatch handling stays in
(internal tracker). /version is unchanged (still the cheap compiled constant).
Also fixes a stale test: test_allow_listed_paths_bypass_the_gate asserted
/admin/maintenance is "!= 503", which was a false alarm once WS-3c added the route
(an unconfigured admin key legitimately 503s). It now asserts specifically that the
maintenance GATE (Retry-After + {"status":"maintenance"}) never intercepts the
allow-listed route -- order-independent.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Scrub private issue-tracker references and a reviewer's name from code comments, CHANGELOG, AGENTS.md, and test docstrings. Substance (PR #67, commit 14a6d30, "tracked separately") is preserved; only the private tracker identifiers and personal name are removed. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…boot Rebase-integration fixes required where PR #70 (data-quality phase 2) and the work merged to main after it (PR #72/#73) touched the same lifespan region: - main.py: seed _sweep_task=None and queue_health="healthy" before the B1 deploy-safe boot boundary. The #73 periodic crash-recovery sweep task and the W-2 queue-health signal are both created INSIDE that boundary, so they must be seeded before it or the shutdown finally / a startup failure hits an unbound name. queue_health default is "healthy" per PR #70's W-2 contract (test_queue_recovery_success_leaves_queue_health_healthy). - test_queue_manager.py / test_main.py: update 5 QueueManager.commit() call sites in PR #73's tests to PR #70 I5b's 3-arg signature commit(sid, offset, cursor) (cursor has no default by design, spec 10.4). Cursor=None: these are queue-level tests, not cursor-durability tests. Preserves both feature sets: #73 bounded respawn + spool + sweep, and #70 deploy-safe boot + maintenance gate + W-2 queue health. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ve TTL probe) DTU E2E surfaced a live defect (PR-review watch-item #3, now reproduced): after a successful in-server repair the untagged-node half of schema_health/ mode stayed 'degraded' until a process restart. The constraint half was already de-latched via the TTL-cached probe; the untagged half was still pinned to the boot-time snapshot (app.state.schema_untagged_nodes / coordinator._boot_untagged), so /status kept reporting untagged_nodes>0 and mode=degraded forever, misleading operators and dashboards. Fix: give MaintenanceCoordinator a SECOND TTL-cached, single-flight probe over count_untagged_nodes (O(1) via Neo4j's counts store, so cheap on the cached path), independent from the constraint probe. _derive_mode now consults the LIVE untagged count (only when the constraint is present -- the only branch where it is load-bearing), and /status sources untagged_nodes/schema_health/ degraded_reason from that live signal instead of the boot snapshot. Seeded from the boot count at bind_driver so the first /status is unchanged, then self-clears within one probe TTL after an out-of-band repair -- no restart. No behavior change to the gate: degraded still never closes it (only mode=maintenance does). Adds A4b de-latch regression tests (+ non-vacuity); updates the two constraint TTL-cache tests whose exact hit-counts now include the untagged read. Full non-neo4j suite 2001 passed; neo4j suite 121 passed. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
d1be7c4 to
3a6f7af
Compare
|
Closing in favor of the stacked PR #79. PR #78 ( Everything of value here is carried forward in #79 (durable cursor + retry-dedup + run-id tiebreaker, IncompleteSession heal-forward, schema-version subsystem, maintenance mode + lease-armed auto-repair on boot, out-of-band migrations CLI, working_dir non-overwrite lift), re-homed onto #78's storage-correct primitives ( Review continues on #79, which is stacked on #78 — merge #78 first, then #79. |
…seat of #79 onto storage-API layer) Re-seat of PR #70's ingest-correctness work (originally #79 commit b946d5d) onto the storage-API-isolation layer. The durable per-record cursor is now persisted atomically with the offset ({"v":1,"offset","cursor"}); retry-dedup and the run-id tiebreaker on parallel handlers carry over unchanged. Seam adaptation for the storage-API layer: - The queue cursor methods (_write_offset_record, _read_offset_record, _read_committed_offset, commit(session_id, offset, cursor), read_cursor, is_fully_drained) land in the FileSystemQueueManager package (queue_manager/filesystem.py) rather than the former flat module; method bodies are byte-identical to #79's originals (verified by diff). - commit() gains a required cursor argument; all four registry.py call sites pass worker.services.snapshot_cursor(), and the whole tree was swept to confirm no 2-arg caller remains. - The offset record stays forward- and backward-compatible: the layer's existing tolerant reader extracts "offset" from a v1 record and ignores the cursor, so a rollback to the prior build reads the committed position correctly. Verified: 219 queue/cursor/services tests + 39 neo4j durability/tiebreaker tests pass. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
#79) Re-seat of PR #70's session-recovery work (originally #79 commit 7059e4b) onto the storage-API layer. Sessions left Incomplete by an earlier crash are healed forward on the next start/fork, and two graph-backfill scripts (relabel_incomplete_sessions, tag_legacy_pooled_iterations) rectify legacy state. Clean cherry-pick: no overlap with the storage-API seam. The two scripts are graph-only (Cypher) — no storage-artifact file operations, consistent with the storage-agnosticism rule. Verified: 154 session-handler tests + 14 neo4j heal-forward/relabel/tag tests pass. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…rrier-allowlist (re-seat of #79) Re-seat of PR #70's schema/data-integrity work (originally #79 commit 236fa69) onto the storage-API layer: - BLOB_REF_CARRIER_PROPERTIES in blob_processor is the single source of truth for which graph properties may carry a ci-blob:// reference; validated at import. - working_dir is never silently overwritten once set. - SCHEMA_VERSION marker + drift reporting (reported, not destructively enforced). Seam adaptation: blob_processor.py reconciled as disjoint regions — the layer's write()->BlobReference mint line and #79's carrier-allowlist block coexist untouched. No storage-artifact file operations introduced. The carrier-allowlist end-to-end test (tests/test_blob_carrier_allowlist.py) imports routers.admin's reference-scan and lands with the maintenance/reclaim commit that introduces admin.py. Verified: 42 status/blob_processor tests + 7 neo4j schema-version/working_dir tests pass. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…tral writer lease (re-seat of #79) Re-seat of PR #70's maintenance/reclaim work (originally #79 commit 6b3a6a0) onto the storage-API layer, with the storage-agnosticism gaps it exposed fixed rather than carried: - Blob-reclaim GC (/admin/blobs/reclaim) is now 100% protocol-based: scan() for discovery, BlobReference (no Path), fenced delete(uri, if_unmodified=ref). The raw glob / os.unlink / Path(settings.blob_path) / _OnDiskBlob are gone. - The destructive apply is single-flighted: a second concurrent apply is refused (409) before it scans, so two applies can never jointly exceed one operator's max_delete blast radius. dry-run is never blocked. - Boot reclaim enumerates the queue through a new QueueManager.session_keys() protocol method instead of globbing the queue directory, so the sweep works unchanged against any queue backend. - The writer-lease detector no longer does raw file I/O: lease persistence moves behind a new lease_store backend (protocol + filesystem + factory), the fourth storage backend alongside blob/queue/identity. The detector keeps its bounded single-thread I/O executor and reaches the lease only through the store. - A standing AST guard test asserts no module outside the four storage backend packages performs a storage-artifact file operation or reads a storage root path; it is a best-effort tripwire (proven red on a planted leak), not the proof. - Maintenance mode (gate + lease-armed auto-repair + /admin/maintenance), carrier-allowlist end-to-end test, and /status re-seat carry over from #79. Verified: 474 non-neo4j + 12 neo4j tests pass; the storage-boundary guard reports zero violations across the tree. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Re-seat of PR #70's migration CLI (originally #79 commit 8e7d9a0) onto the storage-API layer. `migrations/run.py` is an operator tool to diagnose (`--status`) or rectify (`--apply`) graph state out of band, reusing the SAME `run_repair` the server's `/admin/maintenance` endpoint and `doctor --fix` call (dedup -> :Node backfill -> constraint create). Safety by construction rather than by manifest: - The rectification is stateless and idempotent (all IF NOT EXISTS / MERGE), so a run killed midway is completed simply by re-running -- there is no manifest to tear and no resume state to corrupt. - It is a structural rectification only; SCHEMA_VERSION stays 1 -> 1, so a partial run can never leave status/version reporting a phantom-advanced version. - Graph-only (Cypher through the driver); no storage-artifact file operations. Verified: 13 CLI tests pass; the real-neo4j repair path is covered by tests/neo4j/test_node_identity_migration.py (3 passed). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…seat of #79 onto storage-API layer) Re-seat of PR #70's ingest-correctness work (originally #79 commit b946d5d) onto the storage-API-isolation layer. The durable per-record cursor is now persisted atomically with the offset ({"v":1,"offset","cursor"}); retry-dedup and the run-id tiebreaker on parallel handlers carry over unchanged. Seam adaptation for the storage-API layer: - The queue cursor methods (_write_offset_record, _read_offset_record, _read_committed_offset, commit(session_id, offset, cursor), read_cursor, is_fully_drained) land in the FileSystemQueueManager package (queue_manager/filesystem.py) rather than the former flat module; method bodies are byte-identical to #79's originals (verified by diff). - commit() gains a required cursor argument; all four registry.py call sites pass worker.services.snapshot_cursor(), and the whole tree was swept to confirm no 2-arg caller remains. - The offset record stays forward- and backward-compatible: the layer's existing tolerant reader extracts "offset" from a v1 record and ignores the cursor, so a rollback to the prior build reads the committed position correctly. Verified: 219 queue/cursor/services tests + 39 neo4j durability/tiebreaker tests pass. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
#79) Re-seat of PR #70's session-recovery work (originally #79 commit 7059e4b) onto the storage-API layer. Sessions left Incomplete by an earlier crash are healed forward on the next start/fork, and two graph-backfill scripts (relabel_incomplete_sessions, tag_legacy_pooled_iterations) rectify legacy state. Clean cherry-pick: no overlap with the storage-API seam. The two scripts are graph-only (Cypher) — no storage-artifact file operations, consistent with the storage-agnosticism rule. Verified: 154 session-handler tests + 14 neo4j heal-forward/relabel/tag tests pass. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…rrier-allowlist (re-seat of #79) Re-seat of PR #70's schema/data-integrity work (originally #79 commit 236fa69) onto the storage-API layer: - BLOB_REF_CARRIER_PROPERTIES in blob_processor is the single source of truth for which graph properties may carry a ci-blob:// reference; validated at import. - working_dir is never silently overwritten once set. - SCHEMA_VERSION marker + drift reporting (reported, not destructively enforced). Seam adaptation: blob_processor.py reconciled as disjoint regions — the layer's write()->BlobReference mint line and #79's carrier-allowlist block coexist untouched. No storage-artifact file operations introduced. The carrier-allowlist end-to-end test (tests/test_blob_carrier_allowlist.py) imports routers.admin's reference-scan and lands with the maintenance/reclaim commit that introduces admin.py. Verified: 42 status/blob_processor tests + 7 neo4j schema-version/working_dir tests pass. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…tral writer lease (re-seat of #79) Re-seat of PR #70's maintenance/reclaim work (originally #79 commit 6b3a6a0) onto the storage-API layer, with the storage-agnosticism gaps it exposed fixed rather than carried: - Blob-reclaim GC (/admin/blobs/reclaim) is now 100% protocol-based: scan() for discovery, BlobReference (no Path), fenced delete(uri, if_unmodified=ref). The raw glob / os.unlink / Path(settings.blob_path) / _OnDiskBlob are gone. - The destructive apply is single-flighted: a second concurrent apply is refused (409) before it scans, so two applies can never jointly exceed one operator's max_delete blast radius. dry-run is never blocked. - Boot reclaim enumerates the queue through a new QueueManager.session_keys() protocol method instead of globbing the queue directory, so the sweep works unchanged against any queue backend. - The writer-lease detector no longer does raw file I/O: lease persistence moves behind a new lease_store backend (protocol + filesystem + factory), the fourth storage backend alongside blob/queue/identity. The detector keeps its bounded single-thread I/O executor and reaches the lease only through the store. - A standing AST guard test asserts no module outside the four storage backend packages performs a storage-artifact file operation or reads a storage root path; it is a best-effort tripwire (proven red on a planted leak), not the proof. - Maintenance mode (gate + lease-armed auto-repair + /admin/maintenance), carrier-allowlist end-to-end test, and /status re-seat carry over from #79. Verified: 474 non-neo4j + 12 neo4j tests pass; the storage-boundary guard reports zero violations across the tree. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Re-seat of PR #70's migration CLI (originally #79 commit 8e7d9a0) onto the storage-API layer. `migrations/run.py` is an operator tool to diagnose (`--status`) or rectify (`--apply`) graph state out of band, reusing the SAME `run_repair` the server's `/admin/maintenance` endpoint and `doctor --fix` call (dedup -> :Node backfill -> constraint create). Safety by construction rather than by manifest: - The rectification is stateless and idempotent (all IF NOT EXISTS / MERGE), so a run killed midway is completed simply by re-running -- there is no manifest to tear and no resume state to corrupt. - It is a structural rectification only; SCHEMA_VERSION stays 1 -> 1, so a partial run can never leave status/version reporting a phantom-advanced version. - Graph-only (Cypher through the driver); no storage-artifact file operations. Verified: 13 CLI tests pass; the real-neo4j repair path is covered by tests/neo4j/test_node_identity_migration.py (3 passed). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Why this PR
Context Intelligence is meant to be the trustworthy record of what AI sessions did. This PR fixes a set of defects where that record was silently wrong — and got worse the more a session was used (more runs, more forks, more restarts) — plus gives operators a safe way to reclaim disk. Server-side only; the companion bundle work merged separately (#414, closed). Builds on #413 (closed — headless server, PR #67).
This PR now also incorporates the fixes for the code-review feedback (see the next section). The original Phase-2 sections follow it unchanged.
Review remediation — response to code review
The review found that two things previously reported as fixed did not actually hold, plus a cap-inversion and test gaps. Every finding was independently re-verified against the code, implemented, and validated against a real server + real Neo4j (real restart). All blocking and all worth-fixing items are now addressed in code — only the full schema-version enforcement/migration design remains out of scope (see W-4).
Blocking
Retry-Afterwhile the:Nodeuniqueness constraint is absent. The gate sits in the drain loop (never latches; a refused write never advances its queue offset → exactly-once replay). Health is a live, per-request TTL probe that self-clears without a restart once the graph is rectified./statusadvertisesmode/maintenance_started_at/maintenance_elapsed_seconds; enter/complete are logged./status,/version, and/admin/maintenancestay reachable through the gate.POST /admin/maintenanceendpoint runs the rectification on demand (atomic CAS single-flight, returns promptly with arun_id;GETreports progress) via a private admin-driver path. It shares one mechanism (neo4j_store.run_repair) with the standalonemigrations/run.py --apply— one repair, two transports.IterationHandler's mutable counter advanced each attempt (::iteration::1,::iteration::2, …). The phantom-cursor guard existed but was wired only into the post-budget path. Fix: the snapshot/restore guard now wraps the common retry branch too; a transient-then-succeeding flush produces exactly one Iteration node (test proven non-vacuous by revert-check).max_deletewas not a cap. Unvalidated, a negative value slicedcandidates[:-1]= delete-all-but-N. Fix:Field(ge=1);<1rejected with 422 at the schema boundary (verified live:-1/0→ 422,1→ 200).max_delete<=0, the docs-entrypoint auth tripwire, and the maintenance state machine.tag_legacy_pooled_iterations.py(374-line live-graph mutation, previously zero coverage) now hastests/neo4j/test_tag_legacy_pooled_iterations.pyproving against real Neo4j that its>=2 distinct parentsselector tags a pooled Iteration but never a clean single-parent one, that--applygates writes (a dry run mutates nothing), and idempotence.Worth fixing — all addressed
relabel_incomplete_sessions.py --applynow reads the full candidate node_id set (read-only, same selector), writes the undo log, then mutates. A mid-run crash therefore leaves a complete undo record; the pre-mutation set is a superset andrestore_idsis idempotent over it (clean run: candidate == touched, count unchanged). Real-Neo4j tests prove the log is sourced from the pre-mutation read, notapply_relabel's return value.queue_healthis now a separateapp.statesignal; arecovery_reconcile_dead/recover()failure setsqueue_health="degraded"instead of masquerading as a schema problem.working_dir"never overwritten" was Python-layer only. Fix: a blank/whitespace validator on the model, and a DB-levelcoalesce(n.working_dir, row.working_dir)on the Session MERGE so an existing value is never clobbered cross-writer. The generic hot-path node MERGE is untouched (working_dir only flows through the Session MERGE), preserving the index seek — proven with real-Neo4j tests.SCHEMA_VERSIONwas decorative; a mismatch was undetectable. Fix: a read-onlyneo4j_store.read_graph_schema_version()(separate from the write path —ensure_schema_version_baselineuntouched) now surfaces the stored:SchemaMeta.schema_versiononGET /statusasgraph_schema_versionplus an advisoryschema_version_current, so automation can detect server/graph drift. This is advisory telemetry, not a guard — it does not gate, refuse, or migrate; full mismatch enforcement/migration comparison remains out of scope./versionis unchanged (still the cheap compiled-in constant).idx_node_universalis dropped after a successfulrun_repair. Added: a non-vacuous real-Neo4j test (index present before → absent after the constraint is established)./status.degraded_reasonself-consistency (found during remediation testing). Fix:degraded_reasonnow derives from the same live probe asmode/schema_health(instead of a boot snapshot), so it clears after an in-server repair instead of asserting a stale "constraint absent."Remediation validation (real server + real Neo4j)
POST /eventsandPOST /cypher→ 503 +Retry-After+ structured body;/admin/maintenancereachable through the gate →POST /admin/maintenancerectifies (records_affectedreported) → same process, no restart, ingest resumes202and query200.kill -9during maintenance (no loss, no dup, offset lands at EOF only after repair); healthy graph = zero behavior change;/versionreports6.8.0;migrations/run.py --status/--applyidempotent.tests/neo4j/suite passes against real Neo4j (incl. the new tag-script,idx_node_universal-drop, and undo-log tests).Issues addressed — problem for the user, the fix, and the evidence
Iteration metrics corruption (I5/I3 + I5b)
Iteration.node_idso runs never collide (+ additiveiteration_scope); a durable handler cursor persisted atomically with the queue offset and restored on every worker rebuild (crash-restart AND stale-reap); a phantom-cursor guard for dead-lettered events. (Review remediation additionally extends that guard to the common retry-then-succeed path.)iteration_scope=run. Full suite green.working_dir not queryable (I1)
working_dirwas ~0% queryable in-graph.working_dirlifted onto the rootSessionnode, populate-if-missing, idempotent, forward-only. (Review remediation adds a blank validator + a DB-level non-overwrite guarantee.)IncompleteSession mislabeling
IncompleteSession, but ~99% were false positives (a forked-sub-session ordering race stamped the label and never cleared it) — poisoning session-health analytics and giving a false "we are losing events" signal.classify()strips the stale marker on every start/fork (single normalizer, so a future handler reorder cannot skip it); theendbranch is unchanged (the genuine ~0.5% signal). Plus a one-off, out-of-band, idempotent backfill (scripts/relabel_incomplete_sessions.py) that clears historical false positives, gated behind a read-only reconciliation diagnostic. Downstream graph-query skill guidance corrected (bundle). (Review remediation: undo log now written before the mutation; tag-script coverage added.)blob store unbounded growth
blob_pathgrew forever with no safe reclaim, heading toward out-of-disk failures on real deployments.POST /admin/blobs/reclaim— preview-first (dry_rundefault true), deletes only blobs unreferenced by any node in a global all-workspace scan; durable undrained-queue in-flight gate (survives kill -9) + live-worker check + mtime floor; structural$blob_refextraction (not a regex that would silently truncate and delete referenced blobs); explicitmax_deletecap and per-delete audit; idempotent. (Review remediation:max_deleteis now a real cap viaField(ge=1); a fail-closed carrier-allowlist tripwire added.)tests/neo4j/test_blob_reclaim.py— 12 tests green against real Neo4j, incl. cross-workspace safety, special-character extraction, crash/undrained-queue and live-worker protection, dry-run/apply parity, idempotence, max-delete-required.Maintenance mode + out-of-band upgrade path
SCHEMA_VERSION(onGET /version) + create-if-absent:SchemaMetasingleton remain. This PR lands the maintenance-mode + explicitly-triggered in-place-fix mechanism (POST /admin/maintenanceand the degraded-graph gate), the out-of-band upgrade runner + manifest (migrations/run.py+migrations/manifest.yaml), and a detectable schema-version drift signal on/status(advisory). Full drift-detection/comparison + enforcement logic remains out of scope.Deliberately NOT in this PR (tracked separately)
_no_session__*logs, stale-reap.log/.offsetleak, dead-letter auto-expiry, Neo4j retention — remain for later work; this PR resolves only the blob-store reclaim (Finding chore(deps): bump pygments from 2.19.2 to 2.20.0 #1).cost_usd— handled on the read side by the merged bundle graph-query skill (parse +toFloat+ pin to:LlmResponseEvent); no server change or data transformation needed.Operational — ACTION REQUIRED on upgrade if legacy data is present
Migrations never run at server startup. Out-of-band, preview-first steps after deploy (scripts ship in the image):
:Nodeconstraint / duplicates): the server boots into maintenance mode and gates ingest+query. Rectify withpython migrations/run.py --statusthen--apply(local/VM/direct-Neo4j) orPOST /admin/maintenance(network-reachable) — the server self-clears with no restart.docker exec <container> python3 scripts/relabel_incomplete_sessions.py --dry-runthen--apply.POST /admin/blobs/reclaim. SeeCHANGELOG.md,migrations/manifest.yaml, anddocs/maintenance-mode.md.Validation summary
tests/neo4j/suite passes against real Neo4j, incl. the end-to-end proofs above.