Conversation
Merge pull request #8 from neomjs/dev
tobiu
added a commit
that referenced
this pull request
Nov 17, 2019
Merge pull request #10 from neomjs/dev
18 tasks
6 tasks
neo-opus-4-7
pushed a commit
that referenced
this pull request
May 27, 2026
…letes 5/5 axis helpers (#12068 Sub 2 Phase 1, partial) Final axis helper of the 5-axis observability primitive (Epic #12065 Sub 2 #12068 Phase 1 Part A). Combined with prior commits e28e2d0 (ChromaManager pair) and 2953a37 (GraphService pair), this branch now exposes the complete 5-axis surface from Discussion #12062 §2.6: A. Chroma summary count (untenanted) - via existing tooling A2. Chroma graphDigested flag count - ChromaManager.getGraphDigestedCount [e28e2d0] Chroma undigested count - ChromaManager.getUndigestedSessionCount [e28e2d0] B. Graph SESSION node count - GraphService.getSessionNodeCount [2953a37] C. Per-session ENTITY-RELATION count - GraphService.getSessionEntityCount(sessionId) [2953a37] D. Topology-conflict count - TopologyInferenceEngine.getTopologyConflictCount [this commit] Implementation: counts lines matching the canonical conflict-entry suffix `(Source Session:` in the durable `sandman_handoff.md` file (matches the writer pattern at extractTopology line 83). Returns 0 on file-not-found, empty file, or read error — consistent with sibling axis-count helpers' graceful-degradation contract. Anchor & Echo JSDoc cites: - Epic #12065 + Sub 2 ticket #12068 - Discussion #12062 §2.6 axis-divergence framing - Discussion #12062 §2.11 state model context for the deeper "void-return silent failure" distinction - PR #12077 Sub 1 forensics runbook hypothesis #10 cross-reference (extractTopology returns undefined on both no-conflicts AND provider-error paths — a zero count here does NOT distinguish those two states; Sub 3's unified REM cycle + Sub 2 Part B state model close that distinction) Next on this lane: - MCP exposure: new `get_rem_pipeline_state` tool OR extend `manage_database` to surface the 5 axis counts - Unit tests for the 5 helpers (mock SQLite / Chroma / fs surfaces) - Doc scaffold at `learn/agentos/rem-state-model.md` - Open Phase 1 PR
This was referenced May 27, 2026
tobiu
added a commit
that referenced
this pull request
May 27, 2026
…overage (#12068 Sub 2 Phase 1a) (#12081) * feat(ai): add ChromaManager axis helpers — getUndigestedSessionCount/getGraphDigestedCount (#12068 Sub 2 Phase 1, partial) Phase 1 Part A first slice of Epic #12065 Sub 8 #12068. Adds 2 of the 5 axis-count helpers documented in the ticket prescription. Remaining 3 helpers (GraphService.getSessionNodeCount + getSessionEntityCount + TopologyInferenceEngine.getTopologyConflictCount) plus MCP exposure (`get_rem_pipeline_state` tool) plus doc scaffold land in the same PR across additional commits this lane. Why this slice exists (Discussion #12062 §2.6 anchor + GPT live V-B-A 2026-05-27 ~01:19Z empirical evidence): the REM pipeline has 5 distinct axes of state that DO and CAN diverge silently in production: A. Chroma summary count (sessions in `neo-agent-sessions` collection) A2. Chroma graphDigested flag count (sessions DreamService marked digested) B. Graph SESSION node count (sessions actually committed to Semantic Graph) C. Per-session ENTITY-RELATION count (extraction yield per session) D. Topology-conflict count (TopologyInferenceEngine output) Currently only axis A is exposed via MCP. GPT's empirical run showed 76× divergence between axes A and B (1,069 Chroma summaries vs only 14 graph SESSION nodes). This commit adds the Chroma-side getters (getUndigestedSessionCount = NOT axis A2; getGraphDigestedCount = axis A2) that bracket the digestion-progress axis. The graph-side counterpart (GraphService.getSessionNodeCount = axis B) lands in the next commit on this branch. Implementation note: ChromaDB filtering on missing/false metadata attributes is unreliable across versions; helpers mirror the in-memory filter pattern from DreamService.findUndigestedSessions (lines 94-119). Count is bounded by `summarizationBatchLimit` (default 2000) so it's "undigested-among-recent" not strict global — operator-facing diagnostic, not a scheduling input. Anchor & Echo JSDoc on both helpers cite: - Epic #12065 + Sub 2 ticket #12068 - Discussion #12062 §2.6 axis-divergence framing - GPT empirical evidence anchor (1069/14 divergence) - Counterpart helper cross-references for the pair semantic - Important divergence semantic on getGraphDigestedCount: positive flag count does NOT prove graph SESSION node exists; that's the substrate the axis-divergence detection is designed to surface. * feat(ai): add GraphService axis helpers — getSessionNodeCount + getSessionEntityCount (#12068 Sub 2 Phase 1, partial) Continuation of Epic #12065 Sub 2 #12068 Phase 1. This commit adds 2 of the remaining 3 axis-count helpers (the TopologyInferenceEngine helper lands in the next commit). Combined with the ChromaManager pair from commit e28e2d0, this branch now exposes 4 of the 5 axes documented in Sub 2 prescription: A. Chroma summary count (untenanted) - via existing tooling A2. Chroma graphDigested flag count - ChromaManager.getGraphDigestedCount [e28e2d0] Chroma undigested count - ChromaManager.getUndigestedSessionCount [e28e2d0] B. Graph SESSION node count - GraphService.getSessionNodeCount [this commit] C. Per-session ENTITY-RELATION count - GraphService.getSessionEntityCount(sessionId) [this commit] D. Topology-conflict count - TopologyInferenceEngine.getTopologyConflictCount [next commit] Implementation pattern: mirrors HealthService lines 812-816's existing `json_extract(data, '$.label') = 'SESSION'` + tenant-untagged COALESCE-guard SQL filter. Aligns with Memory Core's deployment-wide tenant-isolation semantic. Tenant-scoped variants (getSessionNodeCount({userId})) are a Phase 2 extension if operator deployments need per-tenant axis counts. getSessionEntityCount normalizes `sessionId` to the canonical `SESSION:<id>` uppercase-prefix per Discussion #12062 §2.6 convention (mirrors GraphService.normalizeGraphNodeId logic). Anchor & Echo JSDoc on both helpers cite: - Epic #12065 + Sub 2 ticket #12068 - Discussion #12062 §2.6 axis-divergence framing - HealthService precedent for the SQL filter shape - GPT empirical 76× divergence anchor (1069 Chroma vs 14 graph SESSION) - PR #12077 Sub 1 forensics runbook hypothesis #11 cross-reference (extraction silent failure surface that per-session entity count exposes) - Healthy-pipeline invariant: chroma.graphDigested ≈ graph.SESSION within batch-window tolerance * feat(ai): add TopologyInferenceEngine.getTopologyConflictCount — completes 5/5 axis helpers (#12068 Sub 2 Phase 1, partial) Final axis helper of the 5-axis observability primitive (Epic #12065 Sub 2 #12068 Phase 1 Part A). Combined with prior commits e28e2d0 (ChromaManager pair) and 2953a37 (GraphService pair), this branch now exposes the complete 5-axis surface from Discussion #12062 §2.6: A. Chroma summary count (untenanted) - via existing tooling A2. Chroma graphDigested flag count - ChromaManager.getGraphDigestedCount [e28e2d0] Chroma undigested count - ChromaManager.getUndigestedSessionCount [e28e2d0] B. Graph SESSION node count - GraphService.getSessionNodeCount [2953a37] C. Per-session ENTITY-RELATION count - GraphService.getSessionEntityCount(sessionId) [2953a37] D. Topology-conflict count - TopologyInferenceEngine.getTopologyConflictCount [this commit] Implementation: counts lines matching the canonical conflict-entry suffix `(Source Session:` in the durable `sandman_handoff.md` file (matches the writer pattern at extractTopology line 83). Returns 0 on file-not-found, empty file, or read error — consistent with sibling axis-count helpers' graceful-degradation contract. Anchor & Echo JSDoc cites: - Epic #12065 + Sub 2 ticket #12068 - Discussion #12062 §2.6 axis-divergence framing - Discussion #12062 §2.11 state model context for the deeper "void-return silent failure" distinction - PR #12077 Sub 1 forensics runbook hypothesis #10 cross-reference (extractTopology returns undefined on both no-conflicts AND provider-error paths — a zero count here does NOT distinguish those two states; Sub 3's unified REM cycle + Sub 2 Part B state model close that distinction) Next on this lane: - MCP exposure: new `get_rem_pipeline_state` tool OR extend `manage_database` to surface the 5 axis counts - Unit tests for the 5 helpers (mock SQLite / Chroma / fs surfaces) - Doc scaffold at `learn/agentos/rem-state-model.md` - Open Phase 1 PR * test(ai): add 20-case unit coverage for 5-axis REM observability helpers (#12068 Sub 2 Phase 1, partial) Cross-service consolidated spec at `test/playwright/unit/ai/services/rem-observability.spec.mjs` (precedent for the top-level `services/` location: existing `services-resilient-load.spec.mjs`). Single file covers all 5 axis helpers shipped across commits e28e2d0 + 2953a37 + 37ee6e5: ChromaManager.getUndigestedSessionCount — 4 tests ChromaManager.getGraphDigestedCount — 3 tests GraphService.getSessionNodeCount — 3 tests GraphService.getSessionEntityCount(sessionId) — 5 tests TopologyInferenceEngine.getTopologyConflictCount — 5 tests Total: 20 passing tests (980ms). Test patterns: - ChromaManager: stubbed via `getSummaryCollection = async () => makeFakeSummaryCollection(...)`; fake collection returns the `{ids, metadatas, documents}` shape Chroma's `.get({include, limit})` produces - GraphService: stubbed via `GraphService.db = makeFakeGraphDb(prepareImpl)`; follows the same `originalDb / restore` pattern as GraphService.TenantIsolation.spec.mjs (the canonical precedent) - TopologyInferenceEngine: uses REAL fs against per-test temporary files in `.tmp-rem-observability-test/` (not mocks) — simpler + more durable + exercises actual fs.promises.readFile failure modes (ENOENT) Notable assertion examples: - "the pair invariant" test asserts `undigested + digested == total within batch window` — proves the two ChromaManager helpers bracket the digestion-progress axis correctly - GraphService.getSessionNodeCount test asserts the SQL contains the HealthService-precedent tenant-untagged filter (`json_extract(data, '$.label') = 'SESSION'` AND `COALESCE(json_extract(data, '$.properties.userId'), '') = ''`) - getSessionEntityCount tests verify canonical `SESSION:` prefix normalization for both pre-prefixed AND bare sessionId inputs Graceful-degradation contract verified per helper: - Empty collection → 0 - Missing storage db → 0 - SQL exception → 0 - Empty / missing handoff file → 0 - Null/undefined/non-string sessionId → 0 This completes the Phase 1a code-deliverable scope for Sub 2 Part A (helpers + tests). Phase 1b (MCP exposure: `get_rem_pipeline_state` tool OR `manage_database` extension) is filed as separate scope to keep PRs incrementally reviewable. Phase 2 (Part B REM run/stage state model JSONL writer per Discussion §2.11 schema) is independently shippable once Phase 1a + 1b land. * fix(ai): correct Axis C graph contract — query target side + lowercase session: canonical per #12081 cycle-1 review (#12068) Addresses @neo-gpt PR #12081 cycle-1 CHANGES_REQUESTED. V-B-A on the review surfaced a two-axis defect in the original Axis C helper: 1. Direction wrong: original queried `source = ?` but `MemorySessionIngestor.mjs:226` writes `linkNodes(memoryNodeId, sessionNodeId, 'ORIGINATES_IN', 1.0)` — session is the TARGET. `SemanticGraphExtractor.mjs:88` LLM prompt directs "emit provenance edges linking them back to the source Memory or Session," confirming the inbound direction. 2. Case wrong: original normalized to uppercase `SESSION:` but the canonical SQLite-stored form is lowercase `session:`. Per `GraphService.normalizeGraphNodeId` (lines 489-505) + `MemorySessionIngestor.mjs:160`, uppercase `SESSION:` is normalized to lowercase before persistence. Querying with uppercase against SQLite returns 0 on real REM-ingested data. Net effect of original bug: helper would return 0 against production graph data even though sessions had inbound entity edges. Tests passed because the stubs encoded the WRONG contract assumption. Fix: - Query changed: `WHERE source = ?` → `WHERE target = ?` - Normalization now delegates to `GraphService.normalizeGraphNodeId` (canonical primitive), with bare-id input pre-prefixed via case-insensitive regex `/^(session|memory):/i` so the primitive sees a recognizable shape. - JSDoc rewritten: removed the false "canonical SESSION: uppercase per Discussion §2.6" claim. Now cites: * MemorySessionIngestor.mjs:226 (writer direction precedent) * SemanticGraphExtractor.mjs:88 (LLM prompt directive) * GraphService.normalizeGraphNodeId lines 489-505 (case primitive) * GraphService.mjs:405-408 (lazy-edge-queue uppercase consumption pattern that gets normalized) * Empirical anchor: cycle-1 review's defect-surfacing. Test updates (22/22 pass, was 20/20): - Updated "counts INBOUND edges" — verifies `target = ?` SQL + lowercase `session:` ID + cites writer-precedent comments - NEW "normalizes uppercase SESSION: input to canonical lowercase session:" — exercises the case-normalization path the lazy-edge queue triggers - NEW "real-substrate writer-contract vector" — simulates a real ORIGINATES_IN edge as MemorySessionIngestor:226 writes it (source = `memory:<id>`, target = `session:<id>`, both lowercase post-normalize); proves helper's filter alignment with writer's actual graph contract - Bare-id-normalize test updated for lowercase canonical Also fixed test parallelism race (unrelated to Axis C defect): - Tests previously used a shared in-repo tmpDir + beforeAll/afterAll mkdir/rm; Playwright fullyParallel mode caused workers to race the cleanup against still-running writes (ENOENT failures intermittent) - Switched to OS tmpdir (`os.tmpdir() + /neo-rem-observability-test`) + per-test crypto.randomUUID filename suffix via new `uniqueHandoffPath(suffix)` helper. No afterAll cleanup needed (OS tmpdir is auto-collected). Banked V-B-A discipline lesson (going to agent-private retrospective, not shared substrate): when authoring observability helpers, ALWAYS write at least one test vector that mirrors the actual writer's graph contract — stub-only tests can pass with INVERTED contract assumptions and produce a silent-failure helper that returns 0 against real production data. The cycle-1 review caught exactly this trap. --------- Co-authored-by: tobiu <tobiasuhlig78@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.