Skip to content

feat(client): expose public single-event ingestion API - #124

Merged
Diego Colombo (colombod) merged 2 commits into
microsoft:mainfrom
bkrabach:feat/public-event-ingestion
Sep 20, 2026
Merged

Diego Colombo (colombod) merged 2 commits into
microsoft:mainfrom
bkrabach:feat/public-event-ingestion

Conversation

@bkrabach

@bkrabach Brian Krabach (bkrabach) commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Applications currently have to mount a telemetry hook or import private uploader code to submit their own events. This adds build_event_payload and AsyncCIClient.ingest, reusing existing auth/error contracts while leaving routing, consent, outboxes and retry policy to the caller. The Level 1 builder lives in context_intelligence.events and is re-exported from the package root. Invalid custom payloads raise CIClientError(error_type="invalid_payload") before auth or HTTP. The package version is 0.2.0. The optional [client] extra supplies HTTP transport. The client preserves both real server receipts (queued and duplicate) and never retries or follows redirects.

Scope / guardrails

The client→server /events seam is exercised through real HTTP, static-key authorization and Neo4j. Existing write-side hook fan-out (fanout.py, dispatchers, logging handlers), query behavior, bundles, agents, skills, modes and server code are unchanged. Blocking credential refresh stays off the event loop, and caller-owned payloads are snapshotted before that refresh.

Verification

  • Module tests: 195 passed in modules/tool-context-intelligence-query, using PYTHONPATH=../.. uv run --frozen pytest -q to exercise this checkout. Its existing lock otherwise imports an older client without the existing CIClientError class.
  • Top-level tests: 891 passed with uv run --frozen pytest tests -q. Bare root discovery also collects independently packaged module suites and currently fails on missing module dependencies and colliding tests.conftest names; the intended root suite is selected explicitly.
  • Ruff check and format check: clean.
  • Pyright: 0 errors, 0 warnings.
  • Full bundle validation: scripts/validate-full.sh exited 0 at validation_mode: full; effective PASS WITH SUGGESTIONS after adjudicating the known mode-advertising and standalone README false positives. Existing cosmetic agent-description warning is unchanged. The current recipe runner forces the CLI interpreter; a temporary hatchling PYTHONPATH overlay was needed despite the helper’s PATH-based venv. The separate wheel build covers the recipe’s missing pip wheel dry-run.

uv build --no-sources produced both sdist and wheel. The wheel's [client] extra installed in a fresh environment and passed isolated python -I imports of both APIs.

Real evidence on seams (not mock-only)

  • Five real seam scenarios passed against server 43973967ff4bc9a02422814a8c00dfce7624a76d and official Neo4j 5.26.22/APOC: first acceptance (queued), duplicate recognition, exactly one matching graph event, explicit HTTP 401, and connection refusal. The tests' receipt doubles match the observed server responses.

Captured synthetic request/response and scores, provenance and limitations, and scripts/verify-event-ingestion.py make the run repeatable. Static-key auth was exercised live; Entra refresh uses strategy tests without claiming a live Azure run.

Docs & diagrams

  • Diagrams: N/A — bundle structure is unchanged; the validator's unrelated regenerated overview images are excluded.
  • README documents the additive public contract, explicit occurrence/session identity, timestamp requirements, acceptance vs indexing, and caller-owned delivery policy. SKILL and agent files are unchanged because their contracts did not change.
  • Convention files: N/A — no new convention is imposed. Environment workarounds and the server deduplication limitation are captured in the lane evidence; the existing documented mode-advertising false positive remains untouched.

Notes / follow-ups

At the tested server revision, idempotency_cache.check_and_store precedes durable append (main.py:1501–1533). If that append fails, a later duplicate receipt alone cannot prove the first event persisted. This client preserves that distinction rather than claiming exactly-once behavior. Correcting that server failure window needs a separate server change and fault-injection test; this run did not simulate a disk failure or server crash.

@colombod Diego Colombo (colombod) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review with an independent DTU run. Requesting changes on two items; everything else is a follow-up or a note.

Short version: the pattern is right, this is the correct seam for applications composing context-intelligence as a library, and the evidence packet is stronger than most things that merge here. One defect is real, new, and sits on a supported path. One is bookkeeping. One namespace decision calcifies on release.


1. Independent verification

Suites (run against d4a1172)

Check Result
Root uv run --frozen pytest tests -q 880 passed
ruff check / ruff format --check clean, 178 files
pyright 0 errors, 0 warnings
modules/hook-context-intelligence 700 passed
modules/tool-context-intelligence-query (PYTHONPATH=../..) 195 passed
modules/tool-context-intelligence-upload 553 passed, 1 failed

The single upload-module failure (test_ground_truth_parity.py::test_runtime_sweep_data_parity, a JSONDecodeError) reproduces identically on a clean origin/main worktree at 8789404 — pre-existing, not caused by this PR.

DTU run

Launched this repo's own .amplifier/digital-twin-universe/profiles/context-intelligence-backend.yaml — real Neo4j Community 5.26.22 + standalone context-intelligence-server v6.10.0 (neo4j_connected: true, static-key auth), SERVER_REF=43973967 (server repo HEAD). Destroyed after the run.

Your five checks reproduce independently:

{"passed": true, "checks": {"first_acceptance": true, "duplicate_receipt": true,
 "one_graph_event": true, "auth_fails_loudly": true, "network_fails_loudly": true}}

Nine further scenarios this PR did not live-test, run against that backend:

1 working_dir supplied              OK     queued
2 no data.session_id                OK     queued, session_id: null
3 no data.timestamp                 CIERR  http_status 400 "data.timestamp is required..."
4 custom envelope+key+extra field   OK     queued
5a hook-shaped envelope via ingest  OK     queued
5b new envelope, same v1 key        OK     duplicate
6 unserializable payload (set)      RAW    TypeError escapes    <-- see blocker B1
7 NaN in payload                    RAW    ValueError escapes   <-- see blocker B1
8 timeout=-1 vs real server         CIERR  timeout, ZERO requests reached the server
9 graph: exactly 1 node per accepted event; the 400'd event never landed; total 4

Scenario 5 is the most useful new evidence. The hook's own envelope (carrying working_dir: "/hookdir") posted first returned queued; the new builder's envelope for the same logical event — a different envelope shape, no working_dir key at all — returned duplicate. The aci-event-v1 key genuinely dedupes across both builders at a live server, and the envelope-shape divergence does not break it. The PR proves this only by comparing hashes in a unit test; it is now proven end-to-end.

Scenario 2 also confirms a real server returns session_id: null, so the receipt validator's allowance for that is required behaviour rather than dead permissiveness.

Note in passing: this PR tested against server HEAD 43973967 (2026-09-16), while our canonical DTU profile still pins 766a9691 (2026-07-14). Your evidence is against a newer server than our own DTU suite defaults to. The stale profile pin is a pre-existing repo issue, not yours.


2. Blockers

B1 — the documented error contract is broken by new code, on a supported path

context_intelligence/client.py:940

body = json.loads(json.dumps(payload, allow_nan=False))   # <- ABOVE the try:

The JSON preflight sits outside ingest's try:. A set, a datetime or a NaN in a caller-supplied envelope escapes as a raw TypeError / ValueError — live-proven in probes 6 and 7 above — while README.md:233-237 tells application authors "Failures use CIClientError".

This is not misuse. The API explicitly blesses custom server-compatible envelopes that bypass build_event_payload's own validation, so this is the supported path. A host that writes except CIClientError: around its outbox takes an unhandled crash in production on a payload it constructed itself.

One subtlety on the fix: do not simply move the encode inside the existing try. That block's except ValueError at client.py:975 would relabel a caller's NaN as decode_error / "invalid event receipt" — blaming the server for the caller's payload. It needs its own except and its own error_type.

A verified patch is in §5.

B2 — version not bumped

pyproject.toml:3 still reads version = "0.1.3" while this PR adds a public API and a new [client] extra, so two materially different artifacts answer to the same version. modules/hook-context-intelligence pins the library @main, so consumers resolve by ref today — which is exactly why the bump is cheap now and confusing later. Suggest 0.2.0 in the same commit.


3. Please decide before merge (not asking for a reshape)

S1 — is context_intelligence.upload the right home?

context_intelligence/__init__.py:3-15 defines the package's own three-level taxonomy. build_event_payload is squarely Level 1 — pure transform, no I/O. The upload/ namespace was reserved in writing for the Level 3 session uploader currently in modules/tool-context-intelligence-upload/ (CLI entry point, progress tracking, graph construction). The replacement docstring now has to open by disclaiming what the package does not contain.

The seam also reads as two halves imported at two different depths:

from context_intelligence import AsyncCIClient                  # top level, in __all__
from context_intelligence.upload import build_event_payload     # subpackage, not exported

context_intelligence/events.py, re-exported from the top level, would pair with the endpoint (POST /events) and with ingest, and would leave upload/ free for the migration its previous docstring promised.

Cost now: git mv, one __all__ line, and four references (README.md:207, scripts/verify-event-ingestion.py:21,96, one test import) — nothing else in the repo imports it, verified. Cost after release: a permanent alias shim.

Either outcome is fine — accept upload deliberately and say so in the PR, or rename. I'd just rather it be a decision than a default.


4. The pattern itself — endorsed

The mechanism/policy cut is in the right place, and the hook is the evidence. _DestinationDispatcher (logging_handler.py:283) bundles five separable concerns — a capacity-dropping queue (:342, :472-493), backoff with jitter (:39-48), an auth-only circuit breaker (:704-741), a sustained-failure escalation clock (:560-613), and gitignore-style fan-out. Every one is a tuned choice a host application would legitimately make differently. Correctly left out.

Also worth stating, because it pre-empts the obvious objection: the hook's queue is not a durable outbox — it drops on overflow (:493) and skips hard failures to keep the worker moving (:914-917); durability comes from events.jsonl, a separate concern. So there was no reusable outbox to promote. "Too much policy pushed onto callers" does not hold here.

One thing that genuinely is mechanism and is missing (follow-up, not a blocker): _classify_http_outcome (logging_handler.py:160-197). "Is 401 retryable?" is not a host preference — it is a non-obvious fact about this server's protocol: 401 transient (token refresh), 403 permanent, 404/410 meaning the endpoint is undeployed rather than the payload malformed, 408 pulled out of the 4xx catch-all. Every host will re-derive that table and get 401 wrong. Worth filing classify_ingest_failure(err) -> "retry" | "permanent" now while the reasoning is fresh.


5. Verified patch for B1 + B2

Applied to d4a1172 in a scratch worktree, run, then reverted — 885 passed (880 + 5 new), ruff clean, format clean, pyright 0 errors.

# context_intelligence/client.py  --  in ingest(), replacing lines 937-941
url = f"{self._server_url}/events"
# JSON encoding is also a strict preflight: no NaN or Infinity on the wire.
# It gets its OWN except block: folding it into the request try below would
# relabel a caller's unserializable envelope as "decode_error", blaming the
# server for the caller's payload.
try:
    if not isinstance(payload, dict):
        raise TypeError("event payload must be an object")
    body = json.loads(json.dumps(payload, allow_nan=False))
except (TypeError, ValueError) as exc:
    raise CIClientError(
        "event payload is not a JSON-encodable object",
        error_type="invalid_payload",
        url=url,
    ) from exc

plus the widened enum on CIClientError.error_type (client.py:73-74), and:

# tests/test_event_ingestion.py
@pytest.mark.parametrize(
    "payload",
    [[], "not-an-envelope", {"data": {"unserializable": {1, 2}}},
     {"data": {"value": float("nan")}}, {"data": {"value": float("inf")}}],
)
async def test_unusable_caller_envelope_is_a_client_error_not_a_raw_exception(payload):
    def handle(request):
        pytest.fail("No request is made for an unusable envelope")

    with transport(handle), pytest.raises(CIClientError) as caught:
        await AsyncCIClient("http://server.invalid", "synthetic-key").ingest(payload)
    assert caught.value.error_type == "invalid_payload"

Deliberately not pushed to your branch. docs/lanes/public-event-ingestion/evidence/live.json pins client_source_sha256 for context_intelligence/client.py; I confirmed the committed hash matches the file today, and the patch changes it to f3861b02…. A third party editing that file would silently turn a verifiable provenance record into a stale one. Landing this means re-running scripts/verify-event-ingestion.py and re-attesting the evidence, and that should be the author's signature, not a reviewer's. Also, invalid_payload extends a documented public enum — your call on the name.


6. Follow-ups — file, don't block

  1. _auth_headers (client.py:909-914) catches only ValueError; a real Entra ClientAuthenticationError escapes unclassified. Pre-existing across all 10 call sites including cypher:1025 — fixing it here would widen the blast radius across the whole client. But ingest's README newly advertises that contract to external authors, so it matters more now than it did.
  2. timeout<=0 yields zero HTTP attempts (live-proven, probe 8); timeout=None waits unbounded. Pre-existing AsyncCIClient.__init__ gap.
  3. The parity test (tests/test_event_ingestion.py:31-55) asserts key equality for one input, and the two implementations already diverge outside it: the hook coerces workspace=None -> "" and hashes it (hook/upload.py:15,20) while the helper rejects blank (upload/__init__.py:27-28); the helper passes allow_nan=False (:37), the hook does not (hook/upload.py:12). Neither is a wire break — the helper is the stricter, newer one — but "compatible with hook v1" is a claim about a function currently backed by a single anecdote. Parametrizing over ~5 shapes is three lines in a file this PR already touches.
  4. Single-source the aci-event-v1 scheme. Worth noting the duplication is unforced: the hook already depends on the root library (modules/hook-context-intelligence/pyproject.toml:11, imported at module scope in config_resolver.py:11-12). What holds it together today is that .github/workflows/ci.yml has no path filters, so a hook-only edit still runs the parity test.
  5. classify_ingest_failure (see §4).
  6. ingest returns untyped dict[str, Any]. A 4-line IngestReceipt TypedDict with Literal["queued","duplicate"] is a source-compatible widening — free later, and it gives callers exhaustiveness on the one branch that can lose an event.
  7. Per-call httpx.AsyncClient. Measured on the DTU: 19.42 ms/event vs 13.31 ms pooled — 1.46x, +6.1 ms on plain HTTP over loopback; worse over TLS to a remote host. Consistent with cypher:1026 and fetch_blob:1091, so this is a client-wide lifecycle decision, not a reason to special-case ingest.
  8. README.md:240-243 describes duplicate as "recognition of an existing key" directly beside the retry advice, without repeating that at this server revision a duplicate is not evidence of durable acceptance. The precise warning is in docs/lanes/public-event-ingestion/README.md:68-76 — moving one sentence up to where the wrong outbox transition actually gets made would close it.
  9. except ValueError (client.py:975) maps both a JSON decode failure and the shape rejection at :958 to decode_error. Both are ambiguous-delivery so outboxes behave correctly; the label is just slightly untrue on a public API.
  10. canonical.encode() (upload/__init__.py:40) vs the hook's .encode("utf-8") — byte-identical today, but it is a hash input, so worth making explicit.
  11. tests/test_package_skeleton.py:126-133 still asserts the old placeholder docstring substring. It passes only because the new docstring happens to contain tool-context-intelligence-upload.
  12. AGENTS.md:96-99 reads "ANY ... networking / auth edit -> a real DTU run + the evaluation harness", while :71 says "DTU or an equivalent live run". Those two sentences disagree, and this PR sits exactly in the gap. §1 above discharges it either way, but the wording is worth reconciling so the next author is not guessing.
  13. modules/tool-context-intelligence-upload::test_runtime_sweep_data_parity fails on main — unrelated, flagging for visibility.

7. Things I looked at and would explicitly not ask for

  • A middle layer (outbox / retry scheduler / spooler). Promote one only after two independent host apps converge on the same need.
  • Sync ingest. It is not a wrapper: _http_post (client.py:133-169) swallows every exception and returns None on all three backends, destroying exactly the "definitely rejected" vs "ambiguous, do not discard" distinction an outbox needs, and there is no _http_post_strict. Separate work. Worth one README line noting asyncio.run(client.ingest(p)) so the absence does not read as an oversight.
  • More live evidence. The existing packet clears the seam bar.
  • Anything about the server's check-before-append window. Correctly found, correctly scoped to the server repo, correctly not papered over in the client.

8. Credit

  • The idempotency-key compatibility claim is true, and now proven at a live server across two different envelope shapes.
  • Finding and documenting the server-side idempotency_cache.check_and_store-before-durable-append window (main.py:1501-1533) rather than silently working around it — and refusing to promote duplicate into a durability claim — is exactly the right call.
  • Every failure mode I probed against a real server behaved correctly and loudly: 400 with a useful detail, 401, connection refusal, redirect not followed.

Happy to re-review immediately once B1/B2 land and the evidence is regenerated.

@bkrabach

Copy link
Copy Markdown
Collaborator Author

Diego Colombo (@colombod), this is ready for re-review at 79a6162253c7f8c66e5d2af7e9c8ffcdf38865dd. The follow-up commit addresses your blockers and namespace recommendation:

  • B1: invalid caller envelopes now raise CIClientError(error_type="invalid_payload") before authentication or HTTP. They cannot be mislabeled as server receipt/decode errors.
  • B2: package version is now 0.2.0.
  • S1: the pure builder lives in context_intelligence.events and is re-exported from the package root, leaving the upload namespace available for its intended purpose.

The real server/Neo4j evidence was regenerated after these changes. I rechecked today that both recorded client-source hashes still match this exact head; all five captured seam checks passed. I also reran the focused ingestion suite today: 34 passed. All current-head CI jobs and the CLA check are green.

The scope remains the additive event-ingestion library API. This does not change the session logging hook, on-disk session format, session reconstruction/resume, or server implementation. Routing, consent, delivery queues and retry policy remain with callers. The follow-up topics from your review remain outside this PR's scope.

Could you please re-review the corrected head? Thank you for the independent verification and specific fixes.

@colombod Diego Colombo (colombod) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 79a6162253c7f8c66e5d2af7e9c8ffcdf38865dd. All three items addressed — approving.

Confirmed this head is byte-identical to the one I independently verified, so everything below applies to exactly the commit you asked me to look at.

The three items

B1 — fixed, and the test is better than the patch I offered. The preflight now has its own try and its own error_type, with the reasoning preserved in a comment so the next reader does not re-fold it into the request block. Your test asserts three things where mine asserted one: that the error is classified, that the credential strategy is never consulted, and that the request never reaches the network — plus a datetime case I had missed. Verified live against a real server:

6 unserializable payload (set)   CIERR  type=invalid_payload   (was: raw TypeError)
7 NaN in payload                 CIERR  type=invalid_payload   (was: raw ValueError)

B2 — 0.2.0. Build confirms amplifier_bundle_context_intelligence-0.2.0-py3-none-any.whl, shipping context_intelligence/events.py.

S1 — took the rename, and did the better version of it. context_intelligence/events.py at Level 1, re-exported from __all__, and upload/__init__.py restored to its original placeholder rather than left as a stub. That does not just relocate the function, it un-reserves the namespace — and it incidentally closes follow-up #11, since test_package_skeleton.py:126-133 now asserts a docstring that is true again instead of passing on a coincidence.

Also picked up unprompted — appreciated, none of these were asked for as conditions: #3 (parity test parametrized across five JSON shapes, with working_dir="/hookdir" on the hook side to prove the key ignores it), #8 (the README now states a duplicate cannot prove durable acceptance, positioned next to the retry advice where the wrong outbox transition gets made), #10 (.encode("utf-8") made explicit), and the asyncio.run(...) note so the absent sync API reads as deliberate.

Verification at this head

Provenance re-attested — the reason this was yours to land rather than mine:

context_intelligence/client.py   committed 62de48c0…  actual 62de48c0…  MATCH
context_intelligence/events.py   committed e513d778…  actual e513d778…  MATCH

The harness now hashes events.py rather than upload/__init__.py, and executed_at moved to 2026-09-19T03:08:44Z. The evidence packet is internally consistent again.

Check Result
Root pytest tests -q 891 passed
tests/test_event_ingestion.py 34 passed — matches your count
modules/hook-context-intelligence 700 passed
modules/tool-context-intelligence-query 195 passed
ruff check / format --check / pyright clean · 179 files · 0 errors, 0 warnings
uv build --no-sources sdist + wheel at 0.2.0, wheel contains context_intelligence/events.py
Clean install, no [client] extra build_event_payload imports and works; AsyncCIClient fails loud with the documented ImportError
CI at head all 12 jobs green incl. CLA

Fresh DTU — since client.py changed, I re-ran the seam rather than trusting the previous run. context-intelligence-backend.yaml, real Neo4j Community 5.26.22 + context-intelligence-server v6.10.0 at server revision 43973967. Your five checks pass, and the nine extra probes from my first review re-run with only the two intended changes:

1 working_dir supplied              OK     queued
2 no data.session_id                OK     queued, session_id: null
3 no data.timestamp                 CIERR  http_status 400 "data.timestamp is required..."
4 custom envelope+key+extra field   OK     queued
5a hook-shaped envelope via ingest  OK     queued
5b new envelope, same v1 key        OK     duplicate
8 timeout=-1 vs real server         CIERR  timeout, zero requests reached the server
9 graph: 1 node per accepted event, total 4; the 400'd event never landed

Cross-compatibility (5a/5b) is intact, graph counts are identical to the pre-fix run, and nothing else moved. No regression.

One housekeeping note for the record: in my first review I flagged modules/tool-context-intelligence-upload::test_runtime_sweep_data_parity failing locally. It also fails on a clean origin/main, and the tool-context-intelligence-upload CI job is green at this head — so it is environment-dependent on my machine and unrelated to this PR in either direction. Nothing for you to do here; noting it so the earlier mention does not linger as an open question.

Scope

Agreed, and no argument from me. The follow-ups from my first review — _auth_headers catching only ValueError (#1), timeout<=0 (#2), single-sourcing the v1 key (#4), classify_ingest_failure (#5), IngestReceipt (#6), per-call AsyncClient (#7), the decode_error label (#9), and the AGENTS.md:71 vs :96-99 contradiction (#12) — are all either pre-existing across the whole client or genuinely separable. Worth issues; not worth widening this PR.

Of those, the one I would file first is #5, classify_ingest_failure. The knowledge in _classify_http_outcome (logging_handler.py:160-197) — 401 transient because a token needs refreshing, 403 permanent, 404/410 meaning the endpoint is undeployed rather than the payload malformed, 408 pulled out of the 4xx catch-all — is protocol fact, not host preference. Every application building on ingest will re-derive that table, and 401 is the one they will get wrong. Worth capturing while the reasoning is still fresh.

Approving

The mechanism/policy split is right, the seam evidence is real and now re-attested against the corrected source, and the namespace is where it belongs. This is a good unlock for applications composing context-intelligence as a library.

Thanks for the thorough turnaround — and for regenerating the evidence rather than letting the hashes drift.

@colombod
Diego Colombo (colombod) merged commit 9d9759d into microsoft:main Sep 20, 2026
12 checks passed
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