diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 07686338..64e02490 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -18,6 +18,132 @@ entry. See `CONTRIBUTING.md` § Releases & changelog. ## [Unreleased] +### Added — structured dataset ingestion (CSV import) for the Knowledge Graph (#430) + +- New `KnowledgeGraph` surface (`ingestDataset`, `listDatasets`, `getDataset`, + `queryDatasetRows`, `deleteDataset`) backed by a relational sidecar — + `datasets` + `dataset_rows` tables (migration `0029_datasets.sql`) — NOT a + graph-node explosion: individual rows never become graph nodes, only one + `Dataset` node (`PluginEntity`, `system='dataset'`) is created per dataset + for recall/citation linking. Implemented in both `@omadia/knowledge-graph-neon` + (real SQL, parameterized JSONB filters/aggregates) and + `@omadia/knowledge-graph-inmemory` (full parity, not a stub). +- `POST /api/v1/datasets` (multipart CSV upload), `GET /api/v1/datasets`, + `GET /api/v1/datasets/:id`, `GET /api/v1/datasets/:id/rows`, + `DELETE /api/v1/datasets/:id` — ACL pattern mirrors `/api/v1/memory` + (session-derived owner, no anonymous access). +- CSV attachments in chat now import as a queryable dataset instead of being + silently truncated at the existing 20,000-char text cap + (`attachmentExtract.ts`'s `MAX_TEXT_CHARS`). +- New `query_dataset` native tool: `list_datasets` / `get_schema` / + `query_rows` (a constrained filter+aggregate DSL — never raw SQL from the + model), always paginated/aggregated server-side. +- Every imported row runs through the existing C0 regex PII-detector + baseline (`@omadia/plugin-privacy-guard`) before being persisted — the + same masking pipeline that already protects free-text user prompts. +- Admin UI (upload/schema/delete page under `web-ui/app/admin/`) is + intentionally NOT part of this change — see the PR description. +- Fixup: `inferColumnType` (`datasetImport.ts`) no longer types a column as + `'number'` when any value has a leading zero (`'0301234567'`, `'01234'`) — + such columns are zero-padded identifiers (phone numbers, postal codes), + not numbers. Previously `Number()` silently dropped the leading zero + (data corruption) AND the column skipped the mandatory C0 privacy scan + because number-typed columns are assumed to have no free-text surface. + Both bugs are fixed by keeping such columns `'string'`-typed, which + restores the scan and preserves the value verbatim. +- Fixup (round 2, adversarial cross-vendor review): the chat-attachment CSV + auto-ingest path (`orchestrator.ts`'s `ingestAttachments`) was writing + `ownerOmadiaUserId` from the turn's raw channel-native id (Teams AAD oid, + …) instead of the canonical `omadiaUserId` uuid the KG's ACL routes + filter on. `ChatTurnInput` gains an optional `channelIdentity` field + (`{ channelKind, channelUserId }`, populated only by + `createOrchestratorDispatcher` for channel kinds the KG model has a + mapping for); the CSV-import call site now resolves it via + `KnowledgeGraph.resolveOrCreateChannelIdentity` before using it as the + dataset owner, and declines the KG-import branch (falling back to the + plain-text attachment path) rather than guess for a channel it can't map. +- Fixup (round 2): per-cell CSV truncation (`MAX_CELL_CHARS` in + `datasetImport.ts`) is still applied but is no longer silent — + `parseCsv`/`buildDatasetFromCsv`/`importCsvDataset` now return a + `truncation: { truncatedCellCount, truncatedColumns }` alongside + `privacyScan`, surfaced in the `POST /api/v1/datasets` response and in the + chat-ingest tool-result note. +- Fixup (round 2): `NeonKnowledgeGraph`'s `contains` dataset filter now + escapes `%`, `_`, and `\` in the filter value before wrapping it for + `ILIKE ... ESCAPE '\'`, so a literal `%`/`_` in the value matches literally + instead of being treated as a SQL wildcard — matching the in-memory + backend's literal substring `.includes()` semantics. +- Fixup (round 2): `InMemoryKnowledgeGraph`'s grouped dataset query now caps + results at 200 groups (sorted by aggregate value descending, nulls last), + matching `NeonKnowledgeGraph`'s existing `LIMIT 200` — an unbounded + group-by could otherwise blow the turn token budget through the + in-memory backend only. +- Scope correction: this change addresses #430's CSV import/query path. + #430's own triage acceptance criteria also call for an admin + upload/schema/delete UI, which is deliberately not part of this change — + see Phase 14 in `docs/middleware-agent-handoff.md` §13 for the tracked + follow-up. +- Fixup (round 3, adversarial review): `InMemoryKnowledgeGraph`'s + `matchesDatasetFilter` compared `eq`/`neq`/`contains` filter values with + no type coercion (`value === filter.value`), while + `NeonKnowledgeGraph`'s `buildDatasetFilterClause` already coerced + `filter.value` to the target column's declared type + (`::numeric`/`::text`) before comparing. Concrete failing case: a + `number` column `amount` storing `250` (a JS number) with + `query_dataset` filter `{column:'amount', op:'eq', value:'250'}` (a JSON + string — the tool's Zod schema permits this regardless of column type or + op) matched on Neon but silently returned `totalMatched: 0` on the + in-memory backend for the identical logical query. Fixed by coercing + `filter.value` against the row value using the column's schema-declared + type, mirroring Neon's cast choice exactly (`Number(...)` for a + `number` column, `String(...)` otherwise; `contains` now also coerces a + non-string `filter.value` to a string before the substring check instead + of rejecting it). Regression test added in + `middleware/test/inMemoryKnowledgeGraph.test.ts` reproducing the exact + case above plus the `neq`/`contains` mirrors. +- Fixup (round 5, adversarial review): round 2's channel-identity fix only + covered the IMPORT path (`ingestAttachments`) — `QueryDatasetTool.handle` + still resolved the viewer as `turnContext.current()?.userId`, the RAW + channel-native id, never the canonical `omadiaUserId` a channel turn's + dataset was actually stored under. Net effect: a dataset imported via + Teams/Slack/Telegram chat could never be found again by `list_datasets` / + `get_schema` / `query_rows` from that same chat — the exact "query + ingested datasets" requirement #430 exists for. Fixed by resolving the + canonical id ONCE per turn (`resolveTurnOwnerIdentity`, new + `TurnContextValue.resolvedOmadiaUserId`) in both `runTurn` and + `chatStream` (the latter is what channel adapters actually call — + previously it never populated any per-turn user identity at all for the + `query_dataset`/dataset-ACL purpose), and pointing both `QueryDatasetTool` + and `ingestAttachments` at that single shared value instead of each + re-deriving it. Regression test in `queryDatasetTool.test.ts` simulates a + channel turn's raw-id-at-write-vs-read mismatch end-to-end. +- Fixup (round 6, adversarial review): round 1's `LEADING_ZERO_RE` fix only + matched an UNSIGNED leading zero (`/^0\d/`), so a signed zero-padded value + like `-0123`/`-0456` still passed `NUMBER_RE` (which allows an optional + leading `-`) without tripping the leading-zero guard — the exact same + corruption-plus-scan-bypass defect as round 1, just missed for the signed + case. Fixed by widening the pattern to `/^-?0\d/`, which still correctly + excludes a bare `0`/`-0` or a `0.x`/`-0.x` decimal (those are followed by + nothing or a `.`, not another digit). Regression test added in + `datasetImport.test.ts` with signed zero-padded values proving the column + types as `'string'`, the value round-trips with sign and leading zero + intact, and the privacy scan runs on it. +- Fixup (round 7, adversarial review): `POST /api/v1/datasets` was the only + one of the five dataset route handlers (`middleware/src/routes/datasets.ts`) + with no `try/catch` around its core call (`importCsvDataset`). Since + Express 5 auto-forwards async rejections to its default error handler and + this app registers no global JSON error middleware, an unexpected THROWN + error during import (e.g. a transient Postgres error inside + `NeonKnowledgeGraph.ingestDataset`) fell through to Express's default + handler and returned an HTML error page instead of the `{code, message}` + JSON envelope every other dataset endpoint already returns via + `mapErrorToHttp`. Fixed by wrapping the handler's `importCsvDataset` call + in the same `try/catch` + `mapErrorToHttp` pattern the other four + handlers use — the existing, already-handled `{ok: false, reason}` + not-ok/privacy-rejection return path is unchanged. Regression test added + in `datasetsRoute.test.ts` with a graph whose `ingestDataset` throws, + asserting the route returns a JSON `{code, message}` body. + ### Fixed — orchestrator no longer offers or invokes a not-yet-authenticated plugin's tools (#474) - A native plugin (`ctx.tools.register` from `activate()`) whose own diff --git a/docs/middleware-agent-handoff.md b/docs/middleware-agent-handoff.md index 543376b8..d7e4a5f6 100644 --- a/docs/middleware-agent-handoff.md +++ b/docs/middleware-agent-handoff.md @@ -671,6 +671,47 @@ durch `runTurn({ ..., viewer })`. Tests: `test/conductorBuilder.test.ts` (Digest-Sichtbarkeit inkl. pending/fremd-privat, Proposal-Vetting, Malformed-Blocks, No-Proposal-Regression). +### Dataset-Routen + `query_dataset`-Tool (#430) + +Neue REST-Oberfläche `src/routes/datasets.ts`, gemountet unter +`/api/v1/datasets` (ACL-Pattern wie `/api/v1/memory` — +`req.session.omadia_user_id`, kein anonymer Zugriff): + +- `POST /api/v1/datasets` — multipart CSV-Upload (`multer`, ein File pro + Request, `MAX_UPLOAD_BYTES` = 25 MB). +- `GET /api/v1/datasets` — Liste der eigenen Datasets. +- `GET /api/v1/datasets/:id` — Schema + Metadaten eines Datasets. +- `GET /api/v1/datasets/:id/rows` — paginierte Roh-Zeilen. +- `DELETE /api/v1/datasets/:id` — Dataset löschen. + +Dieselbe Pipeline (`importCsvDataset` aus +`harness-orchestrator/src/datasetImport.ts`) läuft auch automatisch beim +CSV-Chat-Attachment-Pfad in `orchestrator.ts`'s `ingestAttachments` (ersetzt +dort den bisherigen 20.000-Zeichen-Text-Cutoff für CSVs) — siehe §7 für die +Knowledge-Graph-seitige Implementierung. + +Neues natives Tool **`query_dataset`** (`tools/queryDatasetTool.ts`), +registriert wie die übrigen Orchestrator-Tools in §3's Orchestrator-Setup: +`list_datasets` / `get_schema` / `query_rows` gegen eine eingeschränkte +Filter/Aggregat-DSL (nie rohes SQL vom Modell), Ergebnisse immer +server-seitig paginiert/aggregiert bzw. auf 200 Gruppen gecappt. + +**Identity-Resolution (Fixup Runde 5):** für einen Channel-Turn (Teams/ +Slack/Telegram) ist `ChatTurnInput.userId` die RAW channel-native id, NICHT +die kanonische `omadiaUserId` uuid. `resolveTurnOwnerIdentity` +(`resolveTurnOwnerIdentity.ts`) löst sie EINMAL pro Turn auf (via +`KnowledgeGraph.resolveOrCreateChannelIdentity`, wenn `input.channelIdentity` +gesetzt ist — sonst fällt sie auf `input.userId` zurück, das für HTTP/CLI- +Turns bereits kanonisch ist) und legt sie in +`TurnContextValue.resolvedOmadiaUserId` ab — einmal in `runTurn` (non- +streaming) und einmal in `chatStream` (der Pfad, den +`createOrchestratorDispatcher` für Channel-Turns tatsächlich aufruft). +`QueryDatasetTool` und `ingestAttachments` lesen beide ausschließlich dieses +Feld für die Dataset-ACL (niemals das rohe `TurnContextValue.userId`) — vorher +schrieb der Import-Pfad unter der kanonischen id, während der Query-Pfad die +rohe id las, sodass ein Channel-User sein eigenes gerade importiertes Dataset +nie wiederfinden konnte. + --- ## 4. Migration Managed Agents → Lokal @@ -883,6 +924,35 @@ Wird vom Orchestrator aufgerufen, wenn der User auf prior art verweist. End-to-End verifiziert: der Orchestrator nutzt das Tool von selbst, ohne dass man ihn zwingt. +### Structured Datasets — CSV Import (#430) + +Separate Ablage neben dem eigentlichen Graph — bewusst KEINE Graph-Node- +Explosion pro Zeile (Node-Properties sind GIN-indexiert, siehe +`ingestEntities`-Doku). Relationale Sidecar-Tabellen `datasets` + +`dataset_rows` (Migration `packages/harness-knowledge-graph-neon/src/ +migrations/0029_datasets.sql`); pro Dataset genau EIN `Dataset`-Graph-Node +(`PluginEntity`, `system='dataset'`) für Recall/Zitation. + +- **Interface:** `KnowledgeGraph.{ingestDataset,listDatasets,getDataset, + queryDatasetRows,deleteDataset}` (`plugin-api/src/knowledgeGraph.ts`), + implementiert in `@omadia/knowledge-graph-neon` (echtes SQL) UND + `@omadia/knowledge-graph-inmemory` (volle Parität, kein Stub). +- **Import:** `POST /api/v1/datasets` (multipart CSV, `src/routes/ + datasets.ts`) sowie automatisch bei CSV-Chat-Attachments + (`attachmentExtract.ts`'s `isCsvAttachment` branch in `orchestrator.ts`'s + `ingestAttachments` — ersetzt den bisherigen 20.000-Zeichen-Text-Cutoff + für CSVs). +- **Privacy:** jede importierte Zeile läuft vor dem Schreiben durch den + bestehenden C0-Regex-Baseline-Detector (`@omadia/plugin-privacy-guard`'s + `createBaselineDetector`/`maskPrompt`) — dieselbe Pipeline, die + Freitext-User-Prompts schützt. Nur `string`/`date`-Spalten werden + gescannt (Details + Kosten-Hinweis in `datasetImport.ts`'s Modul-Doc). +- **Query:** `query_dataset`-Tool (`tools/queryDatasetTool.ts`) — eine + eingeschränkte Filter/Aggregat-DSL (nie rohes SQL vom Modell), immer + server-seitig paginiert/aggregiert. +- **Admin-UI:** bewusst NICHT Teil dieser Änderung — siehe PR-Beschreibung + von #430 für die Begründung; offener Folge-Task. + --- ## 8. Skills @@ -920,6 +990,14 @@ Migration. Statt dessen überschreibt der Preamble in Funktioniert in der Praxis. Falls ein Sub-Agent dennoch curl-Muster produziert, Skill selbst anpassen. +### Cross-Referenz: `query_dataset` (#430) ist kein Skill + +AGENTS.md's Doku-Regel ordnet "Neue Route / Tool / Sub-Agent" §3 **und** +§8 zu. #430's `query_dataset`-Tool ist ein natives Orchestrator-Tool ohne +eigenen `skills//SKILL.md`-Ordner — es gehört also inhaltlich nicht +in "Aktuelle Skills" oben. Referenz statt Duplikat: volle Doku in §3 +("Dataset-Routen + `query_dataset`-Tool") und §7 (Knowledge-Graph-Schicht). + --- ## 9. Tests (63 Stück, alle grün) @@ -1302,6 +1380,18 @@ gekettet, weil `requires` beim Boot enforced wird): docs-RFC (diese PR) omadia-ui-Orchestrator-Consumer. Details + per-PR-Doc-Pflichten in §15 des RFC. +### Phase 14 — Admin-UI für Dataset-Upload/Schema/Delete (#430 Follow-up) + +Der #430-Scope (CSV-Import + `query_dataset`-Tool, siehe §3 und §7) deckt +absichtlich **keine** Admin-UI ab — Upload/Schema-Browse/Delete bleibt +API-only (`POST/GET/DELETE /api/v1/datasets*`, siehe §3). #430's eigene +Triage-Acceptance-Criteria verlangen aber genau diese UI; der Branch +schließt das Issue deshalb NICHT, sondern "addresses" es — ein +Folge-Issue für die Admin-UI-Seite (`web-ui/app/admin/datasets/` o.ä., +Upload-Dropzone + Schema-Tabelle + Zeilen-Preview + Delete-Bestätigung, +Pattern analog zur bestehenden Package-Upload-Seite) ist offen zu +erfassen. + --- ## 14. Commands (vom `middleware/`-Dir aus) diff --git a/middleware/package-lock.json b/middleware/package-lock.json index c2d593f1..57d3afd8 100644 --- a/middleware/package-lock.json +++ b/middleware/package-lock.json @@ -4389,6 +4389,12 @@ "dev": true, "license": "MIT" }, + "node_modules/csv-parse": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/csv-parse/-/csv-parse-7.0.1.tgz", + "integrity": "sha512-+2z7Ar0APQ7Uu6fX4cn+pitRmxjZ1WPBcGmZFKmA74FCyi7Et/XZx8cjNQ5CjbZ4HCOxXCOpRBYvYH08Qa003A==", + "license": "MIT" + }, "node_modules/dayjs": { "version": "1.11.21", "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.11.21.tgz", @@ -9369,6 +9375,7 @@ "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", + "csv-parse": "^7.0.1", "mammoth": "^1.8.0", "pdf-parse": "^2.4.5" }, @@ -9385,6 +9392,7 @@ "@omadia/memory": "*", "@omadia/orchestrator-extras": "*", "@omadia/plugin-api": "*", + "@omadia/plugin-privacy-guard": "*", "@omadia/usage-telemetry": "*", "@omadia/verifier": "*", "pg": "^8.13.0", diff --git a/middleware/package.json b/middleware/package.json index f25d2923..108beece 100644 --- a/middleware/package.json +++ b/middleware/package.json @@ -20,14 +20,14 @@ ], "scripts": { "preinstall": "node scripts/check-node-version.mjs", - "build": "npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsc && node scripts/copy-build-assets.mjs", + "build": "npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsc && node scripts/copy-build-assets.mjs", "start": "node dist/index.js", - "dev": "node scripts/ensure-native-abi.mjs && npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsx watch --ignore './.memory/**' --ignore './.uploaded-packages/**' --ignore './data/**' --ignore './dist/**' --ignore './packages/*/dist/**' --ignore './seed/**' src/index.ts", + "dev": "node scripts/ensure-native-abi.mjs && npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsx watch --ignore './.memory/**' --ignore './.uploaded-packages/**' --ignore './data/**' --ignore './dist/**' --ignore './packages/*/dist/**' --ignore './seed/**' src/index.ts", "dev:clean": "node scripts/dev-clean.mjs && npm run dev", "ensure-native-abi": "node scripts/ensure-native-abi.mjs", "lint": "eslint src/ packages/plugin-api/src/ packages/llm-provider-api/src/ packages/llm-provider/src/ packages/llm-adapter-anthropic/src/ packages/llm-adapter-openai/src/ packages/harness-ui-helpers/src/ packages/harness-channel-sdk/src/ packages/harness-diagrams/src/ packages/harness-memory/src/ packages/harness-memory-postgres/src/ packages/harness-embeddings/src/ packages/harness-knowledge-graph-inmemory/src/ packages/harness-knowledge-graph-neon/src/ packages/harness-usage-telemetry/src/ packages/harness-orchestrator-extras/src/ packages/harness-verifier/src/ packages/harness-orchestrator/src/ packages/harness-plugin-web-search/src/ packages/harness-plugin-quality-guard/src/ packages/harness-plugin-privacy-guard/src/ packages/harness-plugin-office/src/ packages/omadia-ui-orchestrator/src/ packages/omadia-ui-channel/src/ packages/harness-plugin-plan-runner/src/", "lint:fix": "eslint src/ packages/plugin-api/src/ packages/llm-provider-api/src/ packages/llm-provider/src/ packages/llm-adapter-anthropic/src/ packages/llm-adapter-openai/src/ packages/harness-ui-helpers/src/ packages/harness-channel-sdk/src/ packages/harness-diagrams/src/ packages/harness-memory/src/ packages/harness-memory-postgres/src/ packages/harness-embeddings/src/ packages/harness-knowledge-graph-inmemory/src/ packages/harness-knowledge-graph-neon/src/ packages/harness-usage-telemetry/src/ packages/harness-orchestrator-extras/src/ packages/harness-verifier/src/ packages/harness-orchestrator/src/ packages/harness-plugin-web-search/src/ packages/harness-plugin-quality-guard/src/ packages/harness-plugin-privacy-guard/src/ packages/harness-plugin-office/src/ packages/omadia-ui-orchestrator/src/ packages/omadia-ui-channel/src/ packages/harness-plugin-plan-runner/src/ --fix", - "typecheck": "npm run typecheck -w @omadia/plugin-api && npm run typecheck -w @omadia/llm-provider-api && npm run typecheck -w @omadia/llm-provider && npm run typecheck -w @omadia/llm-adapter-anthropic && npm run typecheck -w @omadia/llm-adapter-openai && npm run typecheck -w @omadia/canvas-core && npm run typecheck -w @omadia/conductor-core && npm run typecheck -w @omadia/plugin-ui-helpers && npm run typecheck -w @omadia/channel-sdk && npm run typecheck -w @omadia/diagrams && npm run typecheck -w @omadia/memory && npm run typecheck -w @omadia/memory-postgres && npm run typecheck -w @omadia/embeddings && npm run typecheck -w @omadia/knowledge-graph-inmemory && npm run typecheck -w @omadia/knowledge-graph-neon && npm run typecheck -w @omadia/orchestrator-extras && npm run typecheck -w @omadia/verifier && npm run typecheck -w @omadia/orchestrator && npm run typecheck -w @omadia/ui-orchestrator && npm run typecheck -w @omadia/ui-channel && npm run typecheck -w @omadia/plugin-office && npm run typecheck -w @omadia/plugin-web-search && npm run typecheck -w @omadia/plugin-quality-guard && npm run typecheck -w @omadia/plugin-privacy-guard && npm run typecheck -w @omadia/agent-seo-analyst && npm run typecheck -w @omadia/agent-reference-maximum && npm run typecheck -w @omadia/plugin-plan-runner && tsc --noEmit", + "typecheck": "npm run typecheck -w @omadia/plugin-api && npm run typecheck -w @omadia/llm-provider-api && npm run typecheck -w @omadia/llm-provider && npm run typecheck -w @omadia/llm-adapter-anthropic && npm run typecheck -w @omadia/llm-adapter-openai && npm run typecheck -w @omadia/canvas-core && npm run typecheck -w @omadia/conductor-core && npm run typecheck -w @omadia/plugin-ui-helpers && npm run typecheck -w @omadia/channel-sdk && npm run typecheck -w @omadia/diagrams && npm run typecheck -w @omadia/memory && npm run typecheck -w @omadia/memory-postgres && npm run typecheck -w @omadia/embeddings && npm run typecheck -w @omadia/knowledge-graph-inmemory && npm run typecheck -w @omadia/knowledge-graph-neon && npm run typecheck -w @omadia/orchestrator-extras && npm run typecheck -w @omadia/verifier && npm run typecheck -w @omadia/plugin-privacy-guard && npm run typecheck -w @omadia/orchestrator && npm run typecheck -w @omadia/ui-orchestrator && npm run typecheck -w @omadia/ui-channel && npm run typecheck -w @omadia/plugin-office && npm run typecheck -w @omadia/plugin-web-search && npm run typecheck -w @omadia/plugin-quality-guard && npm run typecheck -w @omadia/agent-seo-analyst && npm run typecheck -w @omadia/agent-reference-maximum && npm run typecheck -w @omadia/plugin-plan-runner && tsc --noEmit", "format": "prettier --write \"src/**/*.ts\"", "format:check": "prettier --check \"src/**/*.ts\"", "smoke:entity-refs": "tsx scripts/smoke-entity-refs.ts", diff --git a/middleware/packages/harness-channel-sdk/src/chatAgent.ts b/middleware/packages/harness-channel-sdk/src/chatAgent.ts index 96cbc8d1..d7001120 100644 --- a/middleware/packages/harness-channel-sdk/src/chatAgent.ts +++ b/middleware/packages/harness-channel-sdk/src/chatAgent.ts @@ -1,4 +1,4 @@ -import type { PrivacyReceipt, RecalledContext } from '@omadia/plugin-api'; +import type { ChannelKind, PrivacyReceipt, RecalledContext } from '@omadia/plugin-api'; import type { AgentConsultation, DelegatedAnswer, @@ -223,6 +223,23 @@ export interface ChatTurnInput { * "only this user's history". Never reaches the model prompt. */ userId?: string; + /** + * #430 fixup — the turn's channel-native identity, when the dispatcher can + * map one. Populated ONLY for channel turns whose `ChannelUserRef.kind` + * maps to a {@link ChannelKind} the `KnowledgeGraph` ACL model understands + * (`createOrchestratorDispatcher` in `middleware/src/channels/ + * orchestratorDispatcher.ts` is the sole producer). When present, `userId` + * above is a RAW channel-native id (Teams AAD oid, …) — NOT the canonical + * `omadiaUserId` uuid the KG's ACL routes filter on — and any code that + * needs to write an `ownerOmadiaUserId` (dataset ingest, MK ACLs, …) must + * resolve it first via `KnowledgeGraph.resolveOrCreateChannelIdentity`. + * Absent for HTTP/CLI turns, where `userId` (resolved from + * `req.session.omadia_user_id` or a validated `x-user-id`) already IS the + * canonical uuid, and for channel kinds the KG model doesn't have a + * `ChannelKind` for yet (discord, whatsapp, canvas' `'custom'` userRef) — + * deliberately not guessed at. + */ + channelIdentity?: { channelKind: ChannelKind; channelUserId: string }; /** * Chronologically ordered previous turns of this chat (oldest first), as * maintained by an in-memory store outside the orchestrator. When present, diff --git a/middleware/packages/harness-knowledge-graph-inmemory/src/inMemoryKnowledgeGraph.ts b/middleware/packages/harness-knowledge-graph-inmemory/src/inMemoryKnowledgeGraph.ts index 61f29b46..25e38e09 100644 --- a/middleware/packages/harness-knowledge-graph-inmemory/src/inMemoryKnowledgeGraph.ts +++ b/middleware/packages/harness-knowledge-graph-inmemory/src/inMemoryKnowledgeGraph.ts @@ -19,6 +19,15 @@ import { userNodeId, type ChannelIdentityIngest, type CreateMergeCandidateInput, + type DatasetAggregate, + type DatasetColumnType, + type DatasetFilter, + type DatasetIngest, + type DatasetIngestResult, + type DatasetQueryOptions, + type DatasetQueryResult, + type DatasetSummary, + validateDatasetQueryOptions, type EntityRef, type EntityCapturedTurnsHit, type EntityCapturedTurnsOptions, @@ -111,6 +120,97 @@ function matchesAgentScopePrefix( return false; } +/** #430 — one `dataset_rows` row's evaluation of a single `DatasetFilter`. + * Mirrors the SQL semantics `NeonKnowledgeGraph` builds over the JSONB + * column: numeric comparisons coerce both sides to `Number`, `contains` + * is a case-insensitive substring match on strings only. */ +function matchesDatasetFilter( + value: unknown, + filter: DatasetFilter, + columnType: DatasetColumnType, +): boolean { + switch (filter.op) { + case 'eq': + case 'neq': { + // #430 fixup — coerce `filter.value` to the COLUMN's declared type + // before comparing, exactly mirroring `buildDatasetFilterClause` in + // `neonKnowledgeGraph.ts` (`(data->>col)::numeric = $1::numeric` for + // a `number` column, plain text equality otherwise). The + // `query_dataset` tool's Zod schema accepts a `string | number | + // boolean` filter value regardless of the target column's type or + // op, so `{ column: 'amount', op: 'eq', value: '250' }` against a + // `number` column storing `250` must still match — comparing + // `value === filter.value` with no coercion silently returned + // `totalMatched: 0` here while Neon matched correctly on the same + // input (#430 review finding). + const isNumeric = columnType === 'number'; + const a = isNumeric ? Number(value) : String(value); + const b = isNumeric ? Number(filter.value) : String(filter.value); + return filter.op === 'eq' ? a === b : a !== b; + } + case 'contains': + // #430 fixup — same reasoning as eq/neq: `filter.value` can arrive + // as a non-string even though `contains` only targets `string` + // columns (validated by `validateDatasetQueryOptions`, but that + // validation doesn't touch `filter.value`'s own JS type). Coerce it + // to a string before the substring check instead of requiring + // `typeof filter.value === 'string'` and returning `false` otherwise. + return typeof value === 'string' + ? value.toLowerCase().includes(String(filter.value).toLowerCase()) + : false; + case 'gt': + case 'gte': + case 'lt': + case 'lte': { + const a = + typeof value === 'number' + ? value + : typeof value === 'string' + ? Number(value) + : NaN; + const b = + typeof filter.value === 'number' ? filter.value : Number(filter.value); + if (Number.isNaN(a) || Number.isNaN(b)) return false; + if (filter.op === 'gt') return a > b; + if (filter.op === 'gte') return a >= b; + if (filter.op === 'lt') return a < b; + return a <= b; + } + default: + return false; + } +} + +/** #430 — non-numeric / missing values are dropped from the aggregate + * rather than throwing, matching how a SQL `::numeric` cast on a mixed + * JSONB column would need `WHERE value ~ numeric-pattern` to avoid an + * error; `null` (not `0`/`NaN`) signals "no numeric values in scope". */ +function computeDatasetAggregate( + rows: ReadonlyArray>, + aggregate: DatasetAggregate, +): number | null { + if (aggregate.fn === 'count') return rows.length; + const column = aggregate.column; + if (column === undefined) return null; + const values = rows + .map((r) => r[column]) + .map((v) => (typeof v === 'number' ? v : typeof v === 'string' ? Number(v) : NaN)) + .filter((v) => !Number.isNaN(v)); + if (values.length === 0) return null; + switch (aggregate.fn) { + case 'sum': + return values.reduce((a, b) => a + b, 0); + case 'avg': + return values.reduce((a, b) => a + b, 0) / values.length; + case 'min': + return Math.min(...values); + case 'max': + return Math.max(...values); + default: + return null; + } +} + /** * In-memory knowledge graph. Lives in the middleware process, lost on * restart — the session transcripts on disk remain the source of truth, a @@ -127,6 +227,24 @@ export class InMemoryKnowledgeGraph implements KnowledgeGraph { /** Slice 3 — per-memory append-only ACL audit log. */ private readonly aclAudit = new Map(); + /** #430 — one entry per imported dataset (CSV import). Rows live inline; + * the dataset itself is also mirrored as a single `PluginEntity` graph + * node (see `ingestDataset`) whose id is `graphNodeId` below. */ + private readonly datasets = new Map< + string, + { + id: string; + ownerOmadiaUserId: string; + name: string; + sourceFileName: string; + sourceStorageKey?: string; + columns: DatasetSummary['columns']; + rows: Array>; + graphNodeId: string; + createdAt: string; + } + >(); + /** Slice 7 — embedding cache keyed by node externalId. The InMemory * backend has no `embedding` column on its node bag, so we keep a * parallel map. Tests can seed entries directly via `setEmbedding` @@ -2762,6 +2880,162 @@ export class InMemoryKnowledgeGraph implements KnowledgeGraph { return { entityIds: ids, inserted, updated }; } + // ------------------------------------------------------------------- + // #430 — structured dataset ingestion. Parity implementation: rows are + // NEVER promoted to graph nodes (see interface doc), only the one + // Dataset PluginEntity node created via `ingestEntities` below. + // ------------------------------------------------------------------- + + async ingestDataset(input: DatasetIngest): Promise { + const datasetId = randomUUID(); + const { entityIds } = await this.ingestEntities([ + { + system: 'dataset', + model: 'dataset', + id: datasetId, + displayName: input.name, + extras: { + rowCount: input.rows.length, + columnNames: input.columns.map((c) => c.name), + }, + }, + ]); + const graphNodeId = entityIds[0]; + if (graphNodeId === undefined) { + throw new Error('dataset graph-node ingest returned no id'); + } + this.datasets.set(datasetId, { + id: datasetId, + ownerOmadiaUserId: input.ownerOmadiaUserId, + name: input.name, + sourceFileName: input.sourceFileName, + ...(input.sourceStorageKey + ? { sourceStorageKey: input.sourceStorageKey } + : {}), + columns: input.columns, + rows: input.rows.map((r) => ({ ...r })), + graphNodeId, + createdAt: new Date().toISOString(), + }); + return { datasetId, rowCount: input.rows.length, graphNodeId }; + } + + async listDatasets(opts: { + ownerOmadiaUserId: string; + limit?: number; + }): Promise { + const limit = Math.max(1, Math.min(opts.limit ?? 50, 200)); + return [...this.datasets.values()] + .filter((d) => d.ownerOmadiaUserId === opts.ownerOmadiaUserId) + .sort((a, b) => b.createdAt.localeCompare(a.createdAt)) + .slice(0, limit) + .map((d) => this.datasetToSummary(d)); + } + + async getDataset( + datasetId: string, + viewerOmadiaUserId: string, + ): Promise { + const d = this.datasets.get(datasetId); + if (!d || d.ownerOmadiaUserId !== viewerOmadiaUserId) return null; + return this.datasetToSummary(d); + } + + async queryDatasetRows( + datasetId: string, + viewerOmadiaUserId: string, + opts?: DatasetQueryOptions, + ): Promise { + const d = this.datasets.get(datasetId); + if (!d || d.ownerOmadiaUserId !== viewerOmadiaUserId) return null; + const normalized = validateDatasetQueryOptions(d.columns, opts); + const columnTypeByName = new Map(d.columns.map((c) => [c.name, c.type])); + const matched = d.rows.filter((row) => + normalized.filters.every((f) => + matchesDatasetFilter( + row[f.column], + f, + columnTypeByName.get(f.column) ?? 'string', + ), + ), + ); + if (!normalized.aggregate) { + const page = matched.slice( + normalized.offset, + normalized.offset + normalized.limit, + ); + return { + rows: page.map((r) => ({ ...r })), + totalMatched: matched.length, + }; + } + if (normalized.groupBy !== undefined) { + const groupKey = normalized.groupBy; + const buckets = new Map>>(); + for (const row of matched) { + const key = row[groupKey]; + const bucket = buckets.get(key); + if (bucket) bucket.push(row); + else buckets.set(key, [row]); + } + // #430 fixup — cap at 200 groups, same as the Neon backend's + // `LIMIT 200` (neonKnowledgeGraph.ts). Uncapped, a dataset with many + // unique group keys could blow the turn token budget through this + // backend even though Neon is safe. Sorted by value (desc, nulls + // last) BEFORE truncating — same order as Neon's + // `ORDER BY value DESC NULLS LAST LIMIT 200` — so which 200 survive is + // deterministic, not insertion-order-dependent. + const MAX_GROUPS = 200; + const groups = [...buckets.entries()] + .map(([key, rowsInGroup]) => ({ + key, + value: computeDatasetAggregate(rowsInGroup, normalized.aggregate!), + })) + .sort((a, b) => { + if (a.value === null) return b.value === null ? 0 : 1; + if (b.value === null) return -1; + return b.value - a.value; + }) + .slice(0, MAX_GROUPS); + return { groups, totalMatched: matched.length }; + } + return { + aggregateValue: computeDatasetAggregate(matched, normalized.aggregate), + totalMatched: matched.length, + }; + } + + async deleteDataset( + datasetId: string, + actor: AclMutationOptions, + ): Promise { + const d = this.datasets.get(datasetId); + if (!d || d.ownerOmadiaUserId !== actor.actorOmadiaUserId) return false; + this.nodes.delete(d.graphNodeId); + this.datasets.delete(datasetId); + return true; + } + + private datasetToSummary(d: { + id: string; + ownerOmadiaUserId: string; + name: string; + sourceFileName: string; + columns: DatasetSummary['columns']; + rows: Array>; + createdAt: string; + }): DatasetSummary { + return { + id: d.id, + name: d.name, + sourceFileName: d.sourceFileName, + ownerOmadiaUserId: d.ownerOmadiaUserId, + rowCount: d.rows.length, + columns: d.columns, + createdAt: d.createdAt, + }; + } + async findEntityCapturedTurns( opts: EntityCapturedTurnsOptions, ): Promise { diff --git a/middleware/packages/harness-knowledge-graph-neon/src/migrations/0029_datasets.sql b/middleware/packages/harness-knowledge-graph-neon/src/migrations/0029_datasets.sql new file mode 100644 index 00000000..4f60b897 --- /dev/null +++ b/middleware/packages/harness-knowledge-graph-neon/src/migrations/0029_datasets.sql @@ -0,0 +1,47 @@ +-- Issue #430 — structured dataset ingestion. Relational sidecar in the same +-- Neon pool (NOT graph-node explosion): `datasets` carries one row per +-- imported file (tenant/owner scoping, inferred column schema as JSONB); +-- `dataset_rows` carries one row per imported data row (JSONB payload). +-- Individual rows are NEVER promoted to graph nodes — only the parent +-- dataset gets a single `PluginEntity` (system='dataset') node for +-- recall/citation linking (see NeonKnowledgeGraph.ingestDataset), matching +-- the existing warning on `ingestEntities`/`ingestFacts` that node +-- properties are GIN-indexed and must stay small. +-- +-- Raw uploaded file bytes live in Tigris (precedent: migration +-- 0013_teams_attachments.sql); `source_storage_key` below is that +-- object's key, not the bytes themselves. + +CREATE TABLE IF NOT EXISTS datasets ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + tenant_id TEXT NOT NULL, + owner_omadia_user_id TEXT NOT NULL, + name TEXT NOT NULL, + source_file_name TEXT NOT NULL, + source_storage_key TEXT NULL, + row_count INTEGER NOT NULL DEFAULT 0, + columns JSONB NOT NULL DEFAULT '[]'::jsonb, + graph_node_external_id TEXT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE INDEX IF NOT EXISTS idx_datasets_tenant_owner + ON datasets (tenant_id, owner_omadia_user_id, created_at DESC); + +CREATE TABLE IF NOT EXISTS dataset_rows ( + id BIGSERIAL PRIMARY KEY, + dataset_id UUID NOT NULL REFERENCES datasets (id) ON DELETE CASCADE, + row_index INTEGER NOT NULL, + data JSONB NOT NULL +); + +CREATE INDEX IF NOT EXISTS idx_dataset_rows_dataset + ON dataset_rows (dataset_id, row_index); + +-- Query/aggregate hot path: `query_dataset` filters on arbitrary column +-- values inside `data`. A single GIN index over the whole JSONB column +-- covers containment (`@>`) and existence (`?`) lookups reasonably well +-- for CSV-scale datasets without needing a per-column index per import. +CREATE INDEX IF NOT EXISTS idx_dataset_rows_data_gin + ON dataset_rows USING GIN (data); diff --git a/middleware/packages/harness-knowledge-graph-neon/src/neonKnowledgeGraph.ts b/middleware/packages/harness-knowledge-graph-neon/src/neonKnowledgeGraph.ts index 20af4c2f..cafd3444 100644 --- a/middleware/packages/harness-knowledge-graph-neon/src/neonKnowledgeGraph.ts +++ b/middleware/packages/harness-knowledge-graph-neon/src/neonKnowledgeGraph.ts @@ -21,6 +21,15 @@ import { userNodeId, type ChannelIdentityIngest, type CreateMergeCandidateInput, + type DatasetAggregate, + type DatasetColumnSchema, + type DatasetFilter, + type DatasetIngest, + type DatasetIngestResult, + type DatasetQueryOptions, + type DatasetQueryResult, + type DatasetSummary, + validateDatasetQueryOptions, type EntityCapturedTurnsHit, type EntityCapturedTurnsOptions, type EntityIngest, @@ -250,6 +259,94 @@ export async function waitForPostgres( } } +/** #430 — raw `datasets` row shape (migration 0029). */ +interface DatasetRow { + id: string; + name: string; + source_file_name: string; + owner_omadia_user_id: string; + row_count: number; + columns: DatasetColumnSchema[]; + created_at: Date | string; +} + +/** + * #430 — one `DatasetFilter` as a parameterized SQL fragment against + * `dataset_rows.data` (JSONB). The column NAME is bound as a parameter too + * (`data->>$N`), not string-interpolated — `columns` in a dataset's schema + * are themselves data from an uploaded CSV's header row, not a trusted + * literal, even after `validateDatasetQueryOptions` confirms the name is + * one the schema actually has. `columnType` gates which SQL this can even + * build: `validateDatasetQueryOptions` already rejected op/type mismatches + * (numeric ops on non-number columns, `contains` on non-string columns), + * so every branch here is reachable only for a compatible pairing. + */ +function buildDatasetFilterClause( + filter: DatasetFilter, + columnType: DatasetColumnSchema['type'], + params: unknown[], +): string { + params.push(filter.column); + const colParamIdx = params.length; + const isNumeric = columnType === 'number'; + const lhs = isNumeric ? `(data->>$${String(colParamIdx)})::numeric` : `data->>$${String(colParamIdx)}`; + + switch (filter.op) { + case 'eq': + case 'neq': { + params.push(isNumeric ? Number(filter.value) : String(filter.value)); + const rhs = isNumeric ? `$${String(params.length)}::numeric` : `$${String(params.length)}::text`; + return `${lhs} ${filter.op === 'eq' ? '=' : '!='} ${rhs}`; + } + case 'gt': + case 'gte': + case 'lt': + case 'lte': { + params.push(Number(filter.value)); + const opSql = { gt: '>', gte: '>=', lt: '<', lte: '<=' }[filter.op]; + return `${lhs} ${opSql} $${String(params.length)}::numeric`; + } + case 'contains': { + // #430 fixup — a literal `%`/`_` (or backslash) in `filter.value` would + // otherwise be interpreted as a SQL wildcard/escape char instead of a + // literal character once wrapped for ILIKE, diverging from the + // in-memory backend's literal `.includes()` substring match (see + // `matchesDatasetFilter` in `inMemoryKnowledgeGraph.ts`). Escaping the + // escape character itself FIRST is required — otherwise a value + // containing a literal backslash would double-escape. + const escaped = String(filter.value).replace(/[\\%_]/g, '\\$&'); + params.push(`%${escaped}%`); + return `${lhs} ILIKE $${String(params.length)} ESCAPE '\\'`; + } + } +} + +/** + * #430 — SQL expression for a `DatasetAggregate`, appending its column (if + * any) to `params` as a bound parameter for the same "not a trusted + * literal" reason as {@link buildDatasetFilterClause}. + * `validateDatasetQueryOptions` already confirmed the aggregate column (when + * required) is a `number` column, so the `::numeric` cast is always valid. + */ +function buildAggregateSelectSql( + aggregate: DatasetAggregate, + params: unknown[], +): string { + if (aggregate.fn === 'count') return 'COUNT(*)'; + params.push(aggregate.column); + const expr = `(data->>$${String(params.length)})::numeric`; + switch (aggregate.fn) { + case 'sum': + return `SUM(${expr})`; + case 'avg': + return `AVG(${expr})`; + case 'min': + return `MIN(${expr})`; + case 'max': + return `MAX(${expr})`; + } +} + /** * Postgres-backed knowledge graph (Neon serverless). * @@ -427,6 +524,267 @@ export class NeonKnowledgeGraph implements KnowledgeGraph { return { entityIds, inserted, updated }; } + // ------------------------------------------------------------------- + // #430 — structured dataset ingestion. Relational sidecar (`datasets` + + // `dataset_rows`, migration 0029) in this same Neon pool — rows never + // become graph nodes; only one Dataset PluginEntity node is created per + // dataset via the existing `ingestEntities` path above. + // ------------------------------------------------------------------- + + /** Insert the `datasets` row + all `dataset_rows` in one transaction. + * Split out from {@link ingestDataset} so the returned `datasetId` is + * definitely-assigned by construction (`return` inside the `try`) + * rather than relying on control-flow analysis across a rethrow. */ + private async insertDatasetAndRows(input: DatasetIngest): Promise { + const client = await this.pool.connect(); + try { + await client.query('BEGIN'); + const insertResult = await client.query<{ id: string }>( + `INSERT INTO datasets + (tenant_id, owner_omadia_user_id, name, source_file_name, source_storage_key, row_count, columns) + VALUES ($1, $2, $3, $4, $5, $6, $7) + RETURNING id`, + [ + this.tenantId, + input.ownerOmadiaUserId, + input.name, + input.sourceFileName, + input.sourceStorageKey ?? null, + input.rows.length, + JSON.stringify(input.columns), + ], + ); + const datasetId = insertResult.rows[0]?.id; + if (datasetId === undefined) { + throw new Error('dataset insert returned no id'); + } + + // Batched multi-row INSERT (chunked so a very large CSV doesn't + // build one gigantic statement) — one round-trip per chunk instead + // of one per row. + const CHUNK_SIZE = 500; + for (let start = 0; start < input.rows.length; start += CHUNK_SIZE) { + const chunk = input.rows.slice(start, start + CHUNK_SIZE); + const values: string[] = []; + const params: unknown[] = []; + chunk.forEach((row, offset) => { + const base = params.length; + values.push(`($${String(base + 1)}, $${String(base + 2)}, $${String(base + 3)})`); + params.push(datasetId, start + offset, JSON.stringify(row)); + }); + await client.query( + `INSERT INTO dataset_rows (dataset_id, row_index, data) VALUES ${values.join(', ')}`, + params, + ); + } + + await client.query('COMMIT'); + return datasetId; + } catch (err) { + await client.query('ROLLBACK'); + throw err; + } finally { + client.release(); + } + } + + async ingestDataset(input: DatasetIngest): Promise { + const datasetId = await this.insertDatasetAndRows(input); + + // Dataset graph node — created after the row transaction commits, via + // the same path proactive entity-sync (Odoo/Confluence) uses. `system: + // 'dataset'` maps to the generic PluginEntity node type; `extras` stays + // small (row count + column NAMES only, never the data) per the + // GIN-index warning on `ingestEntities`. + const { entityIds } = await this.ingestEntities([ + { + system: 'dataset', + model: 'dataset', + id: datasetId, + displayName: input.name, + extras: { + rowCount: input.rows.length, + columnNames: input.columns.map((c) => c.name), + }, + }, + ]); + const graphNodeId = entityIds[0]; + if (graphNodeId === undefined) { + throw new Error('dataset graph-node ingest returned no id'); + } + await this.pool.query( + `UPDATE datasets SET graph_node_external_id = $1, updated_at = now() WHERE tenant_id = $2 AND id = $3`, + [graphNodeId, this.tenantId, datasetId], + ); + return { datasetId, rowCount: input.rows.length, graphNodeId }; + } + + private async loadDatasetRow(datasetId: string): Promise { + const result = await this.pool.query( + `SELECT id, name, source_file_name, owner_omadia_user_id, row_count, columns, created_at + FROM datasets WHERE tenant_id = $1 AND id = $2`, + [this.tenantId, datasetId], + ); + return result.rows[0] ?? null; + } + + private datasetRowToSummary(row: DatasetRow): DatasetSummary { + return { + id: row.id, + name: row.name, + sourceFileName: row.source_file_name, + ownerOmadiaUserId: row.owner_omadia_user_id, + rowCount: row.row_count, + columns: row.columns, + createdAt: + row.created_at instanceof Date + ? row.created_at.toISOString() + : String(row.created_at), + }; + } + + async listDatasets(opts: { + ownerOmadiaUserId: string; + limit?: number; + }): Promise { + const limit = Math.max(1, Math.min(opts.limit ?? 50, 200)); + const result = await this.pool.query( + `SELECT id, name, source_file_name, owner_omadia_user_id, row_count, columns, created_at + FROM datasets + WHERE tenant_id = $1 AND owner_omadia_user_id = $2 + ORDER BY created_at DESC + LIMIT $3`, + [this.tenantId, opts.ownerOmadiaUserId, limit], + ); + return result.rows.map((r) => this.datasetRowToSummary(r)); + } + + async getDataset( + datasetId: string, + viewerOmadiaUserId: string, + ): Promise { + const row = await this.loadDatasetRow(datasetId); + if (!row || row.owner_omadia_user_id !== viewerOmadiaUserId) return null; + return this.datasetRowToSummary(row); + } + + async queryDatasetRows( + datasetId: string, + viewerOmadiaUserId: string, + opts?: DatasetQueryOptions, + ): Promise { + const dataset = await this.loadDatasetRow(datasetId); + if (!dataset || dataset.owner_omadia_user_id !== viewerOmadiaUserId) { + return null; + } + const columns = dataset.columns; + const normalized = validateDatasetQueryOptions(columns, opts); + const columnTypeByName = new Map(columns.map((c) => [c.name, c.type])); + + const params: unknown[] = [datasetId]; + const whereClauses = normalized.filters.map((f) => { + const colType = columnTypeByName.get(f.column); + if (colType === undefined) { + // Unreachable — validateDatasetQueryOptions already rejected any + // filter naming a column outside `columns`. + throw new Error(`internal: validated column '${f.column}' has no type`); + } + return buildDatasetFilterClause(f, colType, params); + }); + const whereSql = ['dataset_id = $1', ...whereClauses].join(' AND '); + + const totalResult = await this.pool.query<{ count: string }>( + `SELECT COUNT(*)::text AS count FROM dataset_rows WHERE ${whereSql}`, + params, + ); + const totalMatched = Number(totalResult.rows[0]?.count ?? '0'); + + if (!normalized.aggregate) { + const pageParams = [...params, normalized.limit, normalized.offset]; + const limitIdx = pageParams.length - 1; + const offsetIdx = pageParams.length; + const rowsResult = await this.pool.query<{ data: Record }>( + `SELECT data FROM dataset_rows WHERE ${whereSql} + ORDER BY row_index ASC LIMIT $${String(limitIdx)} OFFSET $${String(offsetIdx)}`, + pageParams, + ); + return { rows: rowsResult.rows.map((r) => r.data), totalMatched }; + } + + if (normalized.groupBy !== undefined) { + const groupParams = [...params, normalized.groupBy]; + const groupIdx = groupParams.length; + const groupExpr = `data->>$${String(groupIdx)}`; + const aggSql = buildAggregateSelectSql(normalized.aggregate, groupParams); + const result = await this.pool.query<{ key: unknown; value: string | null }>( + `SELECT ${groupExpr} AS key, ${aggSql} AS value + FROM dataset_rows WHERE ${whereSql} + GROUP BY ${groupExpr} + ORDER BY value DESC NULLS LAST + LIMIT 200`, + groupParams, + ); + return { + groups: result.rows.map((r) => ({ + key: r.key, + value: r.value === null ? null : Number(r.value), + })), + totalMatched, + }; + } + + const aggParams = [...params]; + const aggSql = buildAggregateSelectSql(normalized.aggregate, aggParams); + const result = await this.pool.query<{ value: string | null }>( + `SELECT ${aggSql} AS value FROM dataset_rows WHERE ${whereSql}`, + aggParams, + ); + const value = result.rows[0]?.value; + return { + aggregateValue: value === undefined || value === null ? null : Number(value), + totalMatched, + }; + } + + async deleteDataset( + datasetId: string, + actor: AclMutationOptions, + ): Promise { + const client = await this.pool.connect(); + try { + await client.query('BEGIN'); + const result = await client.query<{ + id: string; + owner_omadia_user_id: string; + graph_node_external_id: string | null; + }>( + `SELECT id, owner_omadia_user_id, graph_node_external_id + FROM datasets WHERE tenant_id = $1 AND id = $2 FOR UPDATE`, + [this.tenantId, datasetId], + ); + const row = result.rows[0]; + if (!row || row.owner_omadia_user_id !== actor.actorOmadiaUserId) { + await client.query('ROLLBACK'); + return false; + } + // CASCADE (migration 0029) removes dataset_rows. + await client.query(`DELETE FROM datasets WHERE id = $1`, [row.id]); + if (row.graph_node_external_id) { + await client.query( + `DELETE FROM graph_nodes WHERE tenant_id = $1 AND external_id = $2`, + [this.tenantId, row.graph_node_external_id], + ); + } + await client.query('COMMIT'); + return true; + } catch (err) { + await client.query('ROLLBACK'); + throw err; + } finally { + client.release(); + } + } + async ingestFacts(facts: FactIngest[]): Promise { if (facts.length === 0) { return { factIds: [], inserted: 0, updated: 0 }; diff --git a/middleware/packages/harness-orchestrator-extras/src/captureFilteringKnowledgeGraph.ts b/middleware/packages/harness-orchestrator-extras/src/captureFilteringKnowledgeGraph.ts index 54ccc6d8..2fa2bee2 100644 --- a/middleware/packages/harness-orchestrator-extras/src/captureFilteringKnowledgeGraph.ts +++ b/middleware/packages/harness-orchestrator-extras/src/captureFilteringKnowledgeGraph.ts @@ -86,6 +86,11 @@ import type { MergeCandidateResolution, TopicNamingSource, TopicNode, + DatasetIngest, + DatasetIngestResult, + DatasetQueryOptions, + DatasetQueryResult, + DatasetSummary, } from '@omadia/plugin-api'; import { sessionNodeId, @@ -602,4 +607,35 @@ export class CaptureFilteringKnowledgeGraph implements KnowledgeGraph { markPalaiaExcerptMergeChecked(excerptExternalId: string): Promise { return this.inner.markPalaiaExcerptMergeChecked(excerptExternalId); } + + // #430 — structured dataset ingestion. Not turn-shaped, so the + // capture-filter has nothing to classify here; forwards verbatim. + ingestDataset(input: DatasetIngest): Promise { + return this.inner.ingestDataset(input); + } + listDatasets(opts: { + ownerOmadiaUserId: string; + limit?: number; + }): Promise { + return this.inner.listDatasets(opts); + } + getDataset( + datasetId: string, + viewerOmadiaUserId: string, + ): Promise { + return this.inner.getDataset(datasetId, viewerOmadiaUserId); + } + queryDatasetRows( + datasetId: string, + viewerOmadiaUserId: string, + opts?: DatasetQueryOptions, + ): Promise { + return this.inner.queryDatasetRows(datasetId, viewerOmadiaUserId, opts); + } + deleteDataset( + datasetId: string, + actor: AclMutationOptions, + ): Promise { + return this.inner.deleteDataset(datasetId, actor); + } } diff --git a/middleware/packages/harness-orchestrator-extras/src/inconsistencyTriggeringKnowledgeGraph.ts b/middleware/packages/harness-orchestrator-extras/src/inconsistencyTriggeringKnowledgeGraph.ts index f3f81a14..8e406840 100644 --- a/middleware/packages/harness-orchestrator-extras/src/inconsistencyTriggeringKnowledgeGraph.ts +++ b/middleware/packages/harness-orchestrator-extras/src/inconsistencyTriggeringKnowledgeGraph.ts @@ -77,6 +77,11 @@ import type { TurnIngest, TurnIngestResult, TurnSearchHit, + DatasetIngest, + DatasetIngestResult, + DatasetQueryOptions, + DatasetQueryResult, + DatasetSummary, } from '@omadia/plugin-api'; export interface InconsistencyTriggeringKnowledgeGraphOptions { @@ -528,4 +533,35 @@ export class InconsistencyTriggeringKnowledgeGraph implements KnowledgeGraph { markPalaiaExcerptMergeChecked(excerptExternalId: string): Promise { return this.inner.markPalaiaExcerptMergeChecked(excerptExternalId); } + + // #430 — structured dataset ingestion. Not turn-shaped and not something + // the inconsistency detector inspects; forwards verbatim. + ingestDataset(input: DatasetIngest): Promise { + return this.inner.ingestDataset(input); + } + listDatasets(opts: { + ownerOmadiaUserId: string; + limit?: number; + }): Promise { + return this.inner.listDatasets(opts); + } + getDataset( + datasetId: string, + viewerOmadiaUserId: string, + ): Promise { + return this.inner.getDataset(datasetId, viewerOmadiaUserId); + } + queryDatasetRows( + datasetId: string, + viewerOmadiaUserId: string, + opts?: DatasetQueryOptions, + ): Promise { + return this.inner.queryDatasetRows(datasetId, viewerOmadiaUserId, opts); + } + deleteDataset( + datasetId: string, + actor: AclMutationOptions, + ): Promise { + return this.inner.deleteDataset(datasetId, actor); + } } diff --git a/middleware/packages/harness-orchestrator-extras/src/mergeTriggeringKnowledgeGraph.ts b/middleware/packages/harness-orchestrator-extras/src/mergeTriggeringKnowledgeGraph.ts index 7e70f249..57218523 100644 --- a/middleware/packages/harness-orchestrator-extras/src/mergeTriggeringKnowledgeGraph.ts +++ b/middleware/packages/harness-orchestrator-extras/src/mergeTriggeringKnowledgeGraph.ts @@ -82,6 +82,11 @@ import type { TurnIngest, TurnIngestResult, TurnSearchHit, + DatasetIngest, + DatasetIngestResult, + DatasetQueryOptions, + DatasetQueryResult, + DatasetSummary, } from '@omadia/plugin-api'; export interface MergeTriggeringKnowledgeGraphOptions { @@ -583,4 +588,35 @@ export class MergeTriggeringKnowledgeGraph implements KnowledgeGraph { markPalaiaExcerptMergeChecked(excerptExternalId: string): Promise { return this.inner.markPalaiaExcerptMergeChecked(excerptExternalId); } + + // #430 — structured dataset ingestion. Not turn-shaped and not something + // the merge-candidate detector inspects; forwards verbatim. + ingestDataset(input: DatasetIngest): Promise { + return this.inner.ingestDataset(input); + } + listDatasets(opts: { + ownerOmadiaUserId: string; + limit?: number; + }): Promise { + return this.inner.listDatasets(opts); + } + getDataset( + datasetId: string, + viewerOmadiaUserId: string, + ): Promise { + return this.inner.getDataset(datasetId, viewerOmadiaUserId); + } + queryDatasetRows( + datasetId: string, + viewerOmadiaUserId: string, + opts?: DatasetQueryOptions, + ): Promise { + return this.inner.queryDatasetRows(datasetId, viewerOmadiaUserId, opts); + } + deleteDataset( + datasetId: string, + actor: AclMutationOptions, + ): Promise { + return this.inner.deleteDataset(datasetId, actor); + } } diff --git a/middleware/packages/harness-orchestrator/package.json b/middleware/packages/harness-orchestrator/package.json index e62befc8..08964d74 100644 --- a/middleware/packages/harness-orchestrator/package.json +++ b/middleware/packages/harness-orchestrator/package.json @@ -18,6 +18,7 @@ "@omadia/memory": "*", "@omadia/orchestrator-extras": "*", "@omadia/plugin-api": "*", + "@omadia/plugin-privacy-guard": "*", "@omadia/usage-telemetry": "*", "@omadia/verifier": "*", "pg": "^8.13.0", @@ -28,6 +29,7 @@ }, "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0", + "csv-parse": "^7.0.1", "mammoth": "^1.8.0", "pdf-parse": "^2.4.5" }, diff --git a/middleware/packages/harness-orchestrator/src/attachmentExtract.ts b/middleware/packages/harness-orchestrator/src/attachmentExtract.ts index 67e96029..5a5f2589 100644 --- a/middleware/packages/harness-orchestrator/src/attachmentExtract.ts +++ b/middleware/packages/harness-orchestrator/src/attachmentExtract.ts @@ -136,6 +136,27 @@ export function checkVisionEmbeddable( return { ok: true, mediaType: ct }; } +const CSV_TYPES = new Set(['text/csv']); +const CSV_EXTS = new Set(['csv']); + +/** + * #430 — true when an attachment should route through the structured + * `importCsvDataset` path (`datasetImport.ts`) instead of the plain-text + * extraction below. Checked BEFORE `extractAttachmentText` at both entry + * points (chat-attachment auto-ingest in `orchestrator.ts`, and the + * `POST /api/v1/datasets` route) so a CSV is never silently truncated at + * `MAX_TEXT_CHARS` — it gets a real schema + queryable rows instead. + */ +export function isCsvAttachment( + contentType: string | undefined, + fileName: string | undefined, +): boolean { + return ( + CSV_TYPES.has(normalizeContentType(contentType)) || + CSV_EXTS.has(extOf(fileName)) + ); +} + /** * Extract plain text from an attachment's bytes. Never throws — any failure * (unknown type, corrupt binary, missing extractor) resolves to diff --git a/middleware/packages/harness-orchestrator/src/datasetImport.ts b/middleware/packages/harness-orchestrator/src/datasetImport.ts new file mode 100644 index 00000000..df475a06 --- /dev/null +++ b/middleware/packages/harness-orchestrator/src/datasetImport.ts @@ -0,0 +1,301 @@ +/** + * CSV → structured dataset import (#430). Shared by both entry points: + * the `POST /api/v1/datasets` REST route and the chat-attachment + * auto-ingest branch in {@link ./attachmentExtract.js | attachmentExtract.ts} + * (via `orchestrator.ts`'s `ingestAttachments`). + * + * Pipeline: parse → infer column types → privacy-scan every row → hand the + * scrubbed rows to `KnowledgeGraph.ingestDataset`. The scan step is + * mandatory and cannot be bypassed by a caller — there is no "skip privacy" + * parameter, matching the maintainer-approved plan on issue #430 ("do not + * skip this or scan headers/sample-only"). + * + * Scanning uses the SAME `createBaselineDetector()` (C0 regex) pass that + * protects free-text user prompts today (`@omadia/plugin-privacy-guard`). + * Only `string`/`date`-typed columns are scanned: a `number`/`boolean` + * column is, by construction, a cell that parsed cleanly as a number/bool + * for EVERY row — there is no free-text surface left for the regex to + * match, and running it anyway risks corrupting legitimate numeric data on + * a false-positive hit (e.g. a 7-digit id that happens to start with a + * leading `0`, which the phone-number pattern would flag). This is a v1 + * scoping call, not a bypass: every ROW still goes through the pipeline, + * exactly as the issue requires — only cells the pipeline could not + * possibly find PII in are skipped. + * + * Cost note: this is O(rows × string-columns) baseline-detector calls, + * each a handful of regex passes over one cell's text — CPU-bound, not + * network-bound (`createBaselineDetector` never makes an HTTP call), so a + * few thousand rows costs single-digit milliseconds. If a future GLiNER + * (C1 transformer) sidecar is wired in for this path — it is NOT, in this + * change, only the C0 baseline is applied — that additional per-row HTTP + * hop is the piece worth budgeting for; see the PR description's + * follow-up-issue note. + */ + +import { parse as parseCsvSync } from 'csv-parse/sync'; + +import { + createBaselineDetector, + maskPrompt, +} from '@omadia/plugin-privacy-guard'; +import type { + DatasetColumnSchema, + DatasetColumnType, + DatasetIngestResult, + KnowledgeGraph, +} from '@omadia/plugin-api'; + +/** Hard cap on imported rows — protects `dataset_rows` + the per-row + * privacy scan from an unbounded upload. Mirrors the spirit of + * `MAX_TEXT_CHARS` in `attachmentExtract.ts`: a cap that degrades + * gracefully (truncate + report) rather than one that OOMs the process. */ +export const MAX_DATASET_ROWS = 50_000; +/** Per-cell char cap BEFORE the privacy scan — an absurdly long single CSV + * cell (e.g. a stray multi-KB blob in one field) would otherwise dominate + * the scan's cost for no import-quality benefit. */ +const MAX_CELL_CHARS = 4_000; + +/** #430 fixup — per-cell truncation stats. `MAX_CELL_CHARS` still caps every + * cell (protects the privacy scan + storage from an absurd single-cell + * blob), but silently cutting a 4000+-char cell with no signal contradicted + * the PR's "no more silent CSV truncation" claim — this makes the cut + * visible instead of removing it (removing it would let one pathological + * cell blow the scan/storage budget). */ +export interface CsvTruncationStats { + /** Total cells whose raw value exceeded `MAX_CELL_CHARS` and was cut. */ + truncatedCellCount: number; + /** Column names that had at least one truncated cell, in header order. */ + truncatedColumns: string[]; +} + +export type CsvParseResult = + | { + ok: true; + headers: string[]; + rows: Array>; + truncation: CsvTruncationStats; + } + | { ok: false; reason: string }; + +/** Parse CSV bytes into header-keyed string rows. Never throws — a + * malformed CSV (ragged rows, empty file, encoding garbage) resolves to + * `{ ok: false, reason }` so callers can surface a clean 4xx/tool-error + * instead of a 500 / unhandled rejection. */ +export function parseCsv(bytes: Buffer): CsvParseResult { + let records: unknown; + try { + records = parseCsvSync(bytes, { + columns: true, + skip_empty_lines: true, + trim: true, + bom: true, + }); + } catch (err) { + return { + ok: false, + reason: `invalid CSV — ${err instanceof Error ? err.message : String(err)}`, + }; + } + if (!Array.isArray(records) || records.length === 0) { + return { ok: false, reason: 'CSV has no data rows' }; + } + const first = records[0]; + if (typeof first !== 'object' || first === null) { + return { ok: false, reason: 'CSV did not parse into row objects' }; + } + const headers = Object.keys(first as Record); + if (headers.length === 0) { + return { ok: false, reason: 'CSV header row is empty' }; + } + if (records.length > MAX_DATASET_ROWS) { + return { + ok: false, + reason: `CSV has ${String(records.length)} rows, exceeding the ${String(MAX_DATASET_ROWS)}-row import cap`, + }; + } + let truncatedCellCount = 0; + const truncatedColumnSet = new Set(); + const rows = (records as Array>).map((record) => { + const row: Record = {}; + for (const h of headers) { + const v = record[h]; + const full = v === undefined || v === null ? '' : String(v); + if (full.length > MAX_CELL_CHARS) { + truncatedCellCount += 1; + truncatedColumnSet.add(h); + } + row[h] = full.slice(0, MAX_CELL_CHARS); + } + return row; + }); + return { + ok: true, + headers, + rows, + truncation: { + truncatedCellCount, + truncatedColumns: headers.filter((h) => truncatedColumnSet.has(h)), + }, + }; +} + +const NUMBER_RE = /^-?\d+(?:\.\d+)?$/; +const BOOLEAN_RE = /^(?:true|false)$/i; +const DATE_RE = /^\d{4}-\d{2}-\d{2}(?:[t ]\d{2}:\d{2}(?::\d{2})?)?$|^\d{1,2}[./]\d{1,2}[./]\d{2,4}$/i; +/** A pure-digit value with a leading zero (`'0301234567'`, `'01234'`), or its + * negative counterpart (`'-0123'`), is a zero-padded identifier — phone + * number, postal code, account number — not a number. `Number()` silently + * drops the leading zero, corrupting the + * value, and a column typed `'number'` skips the mandatory privacy scan + * (see module doc), so such a column must NOT be inferred as `'number'`. + * A bare `'0'` or a `'0.x'` decimal is still a legitimate number and is + * intentionally excluded from this pattern (with or without a leading + * minus sign). */ +const LEADING_ZERO_RE = /^-?0\d/; + +/** Infer one column's type from every non-empty value across all rows — + * ALL values must agree for a type to win; a single non-conforming cell + * falls the column back to `'string'` (the safe default that never + * mis-parses). Empty-only columns default to `'string'`. A column that + * otherwise looks numeric but contains any zero-padded value (leading + * zero) is also forced to `'string'` — see `LEADING_ZERO_RE`. */ +function inferColumnType(values: readonly string[]): DatasetColumnType { + const nonEmpty = values.map((v) => v.trim()).filter((v) => v.length > 0); + if (nonEmpty.length === 0) return 'string'; + if ( + nonEmpty.every((v) => NUMBER_RE.test(v)) && + !nonEmpty.some((v) => LEADING_ZERO_RE.test(v)) + ) { + return 'number'; + } + if (nonEmpty.every((v) => BOOLEAN_RE.test(v))) return 'boolean'; + if (nonEmpty.every((v) => DATE_RE.test(v))) return 'date'; + return 'string'; +} + +export interface PrivacyScanStats { + /** Total cells (across every row) that were passed through the baseline + * detector — string/date-typed columns only, see module doc. */ + scannedCells: number; + /** Cells where at least one span was masked. */ + maskedCells: number; +} + +/** + * Full pipeline: parse → infer schema → privacy-scan every row → + * type-coerce. Returns the scrubbed rows ready for + * `KnowledgeGraph.ingestDataset` plus the inferred column schema. The + * privacy scan runs unconditionally — there is no flag to skip it. + */ +export async function buildDatasetFromCsv(bytes: Buffer): Promise< + | { + ok: true; + columns: DatasetColumnSchema[]; + rows: Array>; + privacyScan: PrivacyScanStats; + truncation: CsvTruncationStats; + } + | { ok: false; reason: string } +> { + const parsed = parseCsv(bytes); + if (!parsed.ok) return parsed; + + const columnTypes = new Map(); + for (const header of parsed.headers) { + columnTypes.set( + header, + inferColumnType(parsed.rows.map((r) => r[header] ?? '')), + ); + } + + const detectors = [createBaselineDetector()]; + let scannedCells = 0; + let maskedCells = 0; + + const scrubbedRows: Array> = []; + for (const rawRow of parsed.rows) { + const outRow: Record = {}; + for (const header of parsed.headers) { + const type = columnTypes.get(header) ?? 'string'; + const raw = rawRow[header] ?? ''; + if (type === 'number') { + outRow[header] = raw.trim() === '' ? null : Number(raw); + continue; + } + if (type === 'boolean') { + outRow[header] = raw.trim() === '' ? null : /^true$/i.test(raw.trim()); + continue; + } + // 'string' | 'date' — the only cells that can carry free text, so the + // only ones that go through the privacy scan (see module doc). + scannedCells += 1; + if (raw.length === 0) { + outRow[header] = raw; + continue; + } + const scanned = await maskPrompt(raw, detectors); + if (scanned.maskedText !== raw) maskedCells += 1; + outRow[header] = scanned.maskedText; + } + scrubbedRows.push(outRow); + } + + const columns: DatasetColumnSchema[] = parsed.headers.map((name) => { + const type = columnTypes.get(name) ?? 'string'; + const firstRow = scrubbedRows[0]; + const sampleValue = firstRow ? firstRow[name] : undefined; + const sample = + sampleValue === null || sampleValue === undefined + ? undefined + : String(sampleValue).slice(0, 200); + return { name, type, ...(sample !== undefined ? { sample } : {}) }; + }); + + return { + ok: true, + columns, + rows: scrubbedRows, + privacyScan: { scannedCells, maskedCells }, + truncation: parsed.truncation, + }; +} + +export interface ImportCsvDatasetInput { + graph: KnowledgeGraph; + bytes: Buffer; + datasetName: string; + sourceFileName: string; + ownerOmadiaUserId: string; + sourceStorageKey?: string; +} + +export type ImportCsvDatasetResult = + | { + ok: true; + result: DatasetIngestResult; + privacyScan: PrivacyScanStats; + truncation: CsvTruncationStats; + } + | { ok: false; reason: string }; + +/** End-to-end: CSV bytes → privacy-scrubbed rows → persisted dataset. The + * single function both entry points (REST route, chat-attachment + * auto-ingest) call, so the pipeline can never be invoked with the scan + * step skipped from one of the two paths but not the other. */ +export async function importCsvDataset( + input: ImportCsvDatasetInput, +): Promise { + const built = await buildDatasetFromCsv(input.bytes); + if (!built.ok) return built; + const result = await input.graph.ingestDataset({ + ownerOmadiaUserId: input.ownerOmadiaUserId, + name: input.datasetName, + sourceFileName: input.sourceFileName, + ...(input.sourceStorageKey + ? { sourceStorageKey: input.sourceStorageKey } + : {}), + columns: built.columns, + rows: built.rows, + }); + return { ok: true, result, privacyScan: built.privacyScan, truncation: built.truncation }; +} diff --git a/middleware/packages/harness-orchestrator/src/index.ts b/middleware/packages/harness-orchestrator/src/index.ts index cfd685de..d49f089e 100644 --- a/middleware/packages/harness-orchestrator/src/index.ts +++ b/middleware/packages/harness-orchestrator/src/index.ts @@ -344,6 +344,24 @@ export { export type { AttachmentReader } from './tools/readAttachmentTool.js'; export { createAttachmentReader } from './attachmentReaderFactory.js'; export type { AttachmentByteStore } from './attachmentReaderFactory.js'; +export { + QUERY_DATASET_TOOL_NAME, + QueryDatasetTool, + queryDatasetToolSpec, +} from './tools/queryDatasetTool.js'; +export { isCsvAttachment } from './attachmentExtract.js'; +export { + buildDatasetFromCsv, + importCsvDataset, + parseCsv, + MAX_DATASET_ROWS, +} from './datasetImport.js'; +export type { + CsvParseResult, + ImportCsvDatasetInput, + ImportCsvDatasetResult, + PrivacyScanStats, +} from './datasetImport.js'; export { FindFreeSlotsTool, FIND_FREE_SLOTS_TOOL_NAME, diff --git a/middleware/packages/harness-orchestrator/src/orchestrator.ts b/middleware/packages/harness-orchestrator/src/orchestrator.ts index 5efde495..26e592ef 100644 --- a/middleware/packages/harness-orchestrator/src/orchestrator.ts +++ b/middleware/packages/harness-orchestrator/src/orchestrator.ts @@ -74,8 +74,18 @@ import { ReadAttachmentTool, readAttachmentToolSpec, } from './tools/readAttachmentTool.js'; +import { + QUERY_DATASET_TOOL_NAME, + QueryDatasetTool, + queryDatasetToolSpec, +} from './tools/queryDatasetTool.js'; import { parseAttachmentsInfo } from './attachmentsInfo.js'; -import { checkVisionEmbeddable, extractAttachmentText } from './attachmentExtract.js'; +import { + checkVisionEmbeddable, + extractAttachmentText, + isCsvAttachment, +} from './attachmentExtract.js'; +import { importCsvDataset } from './datasetImport.js'; import type { EntityRefBus, KnowledgeGraph, @@ -136,6 +146,7 @@ import type { import { streamMessageEvents } from './streaming.js'; import { steeringBus } from './steeringBus.js'; import { buildDateHeader, today, turnContext } from './turnContext.js'; +import { resolveTurnOwnerIdentity } from './resolveTurnOwnerIdentity.js'; import { isMcpServerPrivacyBypassed } from './mcpPrivacyBypass.js'; import { isMcpServerKgIngest } from './mcpKgIngest.js'; @@ -1303,6 +1314,8 @@ export class Orchestrator { private readonly attachmentReader: AttachmentReader | undefined; /** #268 — lazily built `read_attachment` handler (only when reader present). */ private readonly readAttachmentTool: ReadAttachmentTool | undefined; + /** #430 — `query_dataset` native tool; only needs the KnowledgeGraph handle. */ + private readonly queryDatasetTool: QueryDatasetTool | undefined; private readonly chatParticipantsTool: ChatParticipantsTool | undefined; private readonly findFreeSlotsTool: FindFreeSlotsTool | undefined; private readonly bookMeetingTool: BookMeetingTool | undefined; @@ -1368,6 +1381,9 @@ export class Orchestrator { this.knowledgeGraphTool = options.knowledgeGraph ? new KnowledgeGraphTool(options.knowledgeGraph, options.embeddingClient) : undefined; + this.queryDatasetTool = options.knowledgeGraph + ? new QueryDatasetTool(options.knowledgeGraph) + : undefined; this.factExtractor = options.factExtractor; this.chatParticipantsTool = options.chatParticipantsTool; this.askUserChoiceTool = options.askUserChoiceTool; @@ -2181,6 +2197,15 @@ export class Orchestrator { const privacyHandle = privacyService ? this.buildPrivacyHandle(privacyService, sessionId, turnId) : undefined; + // #430 fixup (reviewer round 5) — resolve the canonical omadiaUserId ONCE + // for the whole turn; see `resolveTurnOwnerIdentity` for the fallback + // rules. Read by `QueryDatasetTool` and `ingestAttachments` via + // `turnContext.current()?.resolvedOmadiaUserId` instead of each + // re-deriving it independently. + const resolvedOmadiaUserId = await resolveTurnOwnerIdentity( + this.knowledgeGraph, + input, + ); return turnContext.run( { @@ -2192,6 +2217,7 @@ export class Orchestrator { // Human user id — dispatch-time consumers (MCP→KG ingestion) attribute // per-user data with it. ...(input.userId ? { userId: input.userId } : {}), + ...(resolvedOmadiaUserId ? { resolvedOmadiaUserId } : {}), ...(parent?.chatParticipants ? { chatParticipants: parent.chatParticipants } : {}), @@ -3420,12 +3446,22 @@ export class Orchestrator { const privacyHandle = privacyService ? this.buildPrivacyHandle(privacyService, sessionId, turnId) : undefined; + // #430 fixup (reviewer round 5) — same per-turn resolution as `runTurn` + // above. This streaming entry point is what channel adapters (Teams/ + // Slack/Telegram, via `createOrchestratorDispatcher`) actually call, so + // without this the resolved identity would never reach a channel turn's + // tool dispatch at all — see `resolveTurnOwnerIdentity`. + const resolvedOmadiaUserId = await resolveTurnOwnerIdentity( + this.knowledgeGraph, + input, + ); turnContext.enter({ turnId, turnDate: today(), // Per-orchestrator isolation: see the matching `turnContext.run` above. agentSlug: this.agentId, + ...(resolvedOmadiaUserId ? { resolvedOmadiaUserId } : {}), ...(parent?.chatParticipants ? { chatParticipants: parent.chatParticipants } : {}), @@ -4891,6 +4927,9 @@ export class Orchestrator { if (name === KNOWLEDGE_GRAPH_TOOL_NAME && this.knowledgeGraphTool) { return this.knowledgeGraphTool.handle(input); } + if (name === QUERY_DATASET_TOOL_NAME && this.queryDatasetTool) { + return this.queryDatasetTool.handle(input); + } if (name === CHAT_PARTICIPANTS_TOOL_NAME && this.chatParticipantsTool) { return this.chatParticipantsTool.handle(); } @@ -5275,13 +5314,72 @@ export class Orchestrator { 'fileName' in fetched ? (fetched as { fileName?: string }).fileName : undefined; + const attachmentFileName = fetchedFileName ?? c.fileName; + const label = c.fileName ?? c.storageKey ?? c.url ?? 'attachment'; + // #430 — CSV attachments become a queryable dataset instead of a + // truncated `[attachment-content]` text blob, whenever a + // KnowledgeGraph AND a resolved user identity are both available. + // Falls back to the plain-text path otherwise, so CSV ingest + // degrades instead of silently failing on a channel without + // either. + // + // #430 fixup (reviewer round 2) — dataset ownership needs the + // canonical `omadiaUserId` uuid, NOT the raw channel-native id + // `input.userId` carries for channel turns (Teams AAD oid, …; see + // `ChatTurnInput.userId`'s doc comment). + // + // #430 fixup (reviewer round 5) — this used to re-resolve + // `input.channelIdentity` independently, right here, on every CSV + // attachment. That was the ONLY place the canonical id got + // computed — `QueryDatasetTool` had no way to read it and fell + // back to the raw `turnContext.current()?.userId`, so a dataset a + // channel user just imported could never be found again by that + // same user. Now reads the ONE per-turn resolution both the + // import and query paths share — see + // `resolveTurnOwnerIdentity`/`TurnContextValue.resolvedOmadiaUserId` + // for the resolution + fallback rules (still idempotent, still + // degrades to the plain-text path below when unresolved). + if (isCsvAttachment(contentType, attachmentFileName) && this.knowledgeGraph) { + const ownerOmadiaUserId = turnContext.current()?.resolvedOmadiaUserId; + if (ownerOmadiaUserId) { + const imported = await importCsvDataset({ + graph: this.knowledgeGraph, + bytes: fetched.bytes, + datasetName: attachmentFileName ?? label, + sourceFileName: attachmentFileName ?? label, + ownerOmadiaUserId, + ...(c.storageKey ? { sourceStorageKey: c.storageKey } : {}), + }); + if (imported.ok) { + // #430 fixup — per-cell truncation (MAX_CELL_CHARS) still + // happens (see datasetImport.ts module doc); only tell the + // model "not truncated" when that's actually true this time, + // rather than making a blanket claim the PR no longer backs. + const { truncatedCellCount, truncatedColumns } = imported.truncation; + const truncationNote = + truncatedCellCount > 0 + ? `Note: ${String(truncatedCellCount)} cell(s) in column(s) [${truncatedColumns.join(', ')}] exceeded the per-cell length cap and were truncated on import.` + : 'No cells were truncated on import.'; + textBlocks.push( + `\n\n[dataset-imported: ${label}]\ndataset_id=${imported.result.datasetId}, rows=${String(imported.result.rowCount)}. ` + + `Use the \`${QUERY_DATASET_TOOL_NAME}\` tool with this dataset_id to filter/aggregate this data — do not ask the user to re-paste it. ${truncationNote}\n[/dataset-imported]`, + ); + continue; + } + console.warn( + `[harness-orchestrator] ingestAttachments: CSV dataset import failed for ${label} — ${imported.reason}`, + ); + // Fall through to the plain-text path below so the CSV's raw + // text (even if capped) still reaches the model rather than + // vanishing silently. + } + } const result = await extractAttachmentText( fetched.bytes, contentType, - fetchedFileName ?? c.fileName, + attachmentFileName, ); if (!result.ok) continue; - const label = c.fileName ?? c.storageKey ?? c.url ?? 'attachment'; textBlocks.push( `\n\n[attachment-content: ${label}]\n${result.text}\n[/attachment-content]`, ); @@ -5336,6 +5434,7 @@ export class Orchestrator { tools.push({ type: MEMORY_TOOL_TYPE, name: MEMORY_TOOL_NAME }); } if (this.knowledgeGraphTool) tools.push(knowledgeGraphToolSpec); + if (this.queryDatasetTool) tools.push(queryDatasetToolSpec); // Diagrams + enrich_company tool specs come from nativeTools registry (plugin-contributed). if (this.chatParticipantsTool) tools.push(chatParticipantsToolSpec); if (this.askUserChoiceTool) tools.push(askUserChoiceToolSpec); diff --git a/middleware/packages/harness-orchestrator/src/resolveTurnOwnerIdentity.ts b/middleware/packages/harness-orchestrator/src/resolveTurnOwnerIdentity.ts new file mode 100644 index 00000000..84e713f4 --- /dev/null +++ b/middleware/packages/harness-orchestrator/src/resolveTurnOwnerIdentity.ts @@ -0,0 +1,48 @@ +import type { ChatTurnInput } from '@omadia/channel-sdk'; +import type { KnowledgeGraph } from '@omadia/plugin-api'; + +/** + * #430 fixup (reviewer round 5) — resolves the ONE canonical `omadiaUserId` + * for a turn, once, so every turn-scoped consumer that needs the caller's + * identity for a KnowledgeGraph ACL (dataset ownership on import, dataset + * ownership on query, …) reads the SAME value instead of re-deriving it + * independently. Before this, `ingestAttachments` resolved + * `input.channelIdentity` into a canonical id for the IMPORT path only; + * `QueryDatasetTool` read the raw `turnContext.current()?.userId` for the + * QUERY path — for a channel turn (Teams/Slack/Telegram) that raw id is the + * channel-native id (Teams AAD oid, …), never the canonical uuid, so a + * dataset a channel user just imported could never be found again by that + * same user in that same channel. + * + * Mirrors the exact fallback `ingestAttachments` already implemented: + * `input.channelIdentity` present → resolve via + * `KnowledgeGraph.resolveOrCreateChannelIdentity` (idempotent — re-resolving + * the same `(channelKind, channelUserId)` pair is safe and returns the same + * id); absent → `input.userId` already IS the canonical uuid (HTTP/CLI turns, + * and channel kinds the KG model doesn't cover yet) so it's used as-is. + */ +export async function resolveTurnOwnerIdentity( + knowledgeGraph: KnowledgeGraph | undefined, + input: Pick, +): Promise { + if (!input.channelIdentity) return input.userId; + // No KnowledgeGraph wired up ⇒ no way to resolve a channel identity into a + // canonical uuid. Deliberately returns undefined rather than guessing with + // the raw channel-native id — callers (dataset ACL checks) must degrade to + // "no identity available" rather than silently using the wrong id. + if (!knowledgeGraph) return undefined; + try { + const { omadiaUserId } = await knowledgeGraph.resolveOrCreateChannelIdentity({ + channelKind: input.channelIdentity.channelKind, + channelUserId: input.channelIdentity.channelUserId, + }); + return omadiaUserId; + } catch (err) { + console.warn( + `[harness-orchestrator] resolveTurnOwnerIdentity: channel identity resolution failed — ${ + err instanceof Error ? err.message : String(err) + }`, + ); + return undefined; + } +} diff --git a/middleware/packages/harness-orchestrator/src/tools/queryDatasetTool.ts b/middleware/packages/harness-orchestrator/src/tools/queryDatasetTool.ts new file mode 100644 index 00000000..1d2d37a2 --- /dev/null +++ b/middleware/packages/harness-orchestrator/src/tools/queryDatasetTool.ts @@ -0,0 +1,193 @@ +import { z } from 'zod'; +import { + DatasetQueryValidationError, + type KnowledgeGraph, +} from '@omadia/plugin-api'; + +import { turnContext } from '../turnContext.js'; + +/** + * #430 — native tool over `KnowledgeGraph.{listDatasets,getDataset, + * queryDatasetRows}`. Mirrors `KnowledgeGraphTool`'s multi-query-shape + * pattern (one tool, a `query` discriminator picks the operation) rather + * than three separate tool specs — keeps the tool list short. + * + * ACL: every operation resolves the caller's CANONICAL `omadiaUserId` from + * `turnContext.current()?.resolvedOmadiaUserId` — the same per-turn value + * `ingestAttachments` uses to set `ownerOmadiaUserId` on CSV import (see + * `resolveTurnOwnerIdentity`). This is deliberately NOT `turnContext.current() + * ?.userId`: for a channel turn (Teams/Slack/Telegram) that field is the RAW + * channel-native id (Teams AAD oid, …), which never matches the canonical + * uuid a dataset was actually stored under — reading it here would make + * every channel-native user's own just-imported datasets permanently + * unfindable (#430 fixup, reviewer round 5). There is no anonymous dataset + * access, and a dataset the caller doesn't own is indistinguishable from a + * missing one (`not_found_or_not_owned`), matching the `/api/v1/memory` + * ACL convention of never leaking existence to non-owners. + * + * `query_rows` never returns more than `filters`/`limit` allow — the + * `KnowledgeGraph` implementation pages/aggregates server-side (see + * `DatasetQueryOptions`), so this tool can't accidentally dump a whole + * dataset into turn context even if the model asks it to. + */ + +const FilterSchema = z.object({ + column: z.string().min(1).max(200), + op: z.enum(['eq', 'neq', 'gt', 'gte', 'lt', 'lte', 'contains']), + value: z.union([z.string(), z.number(), z.boolean()]), +}); + +const AggregateSchema = z.object({ + fn: z.enum(['count', 'sum', 'avg', 'min', 'max']), + /** Required for every `fn` except `count`. */ + column: z.string().min(1).max(200).optional(), +}); + +const QueryDatasetInputSchema = z.object({ + query: z.enum(['list_datasets', 'get_schema', 'query_rows']), + /** Required for `get_schema` and `query_rows`. */ + dataset_id: z.string().min(1).max(200).optional(), + /** `query_rows` only. Every `column` MUST be one of `get_schema`'s + * returned column names — unknown columns are rejected. */ + filters: z.array(FilterSchema).max(10).optional(), + group_by: z.string().min(1).max(200).optional(), + aggregate: AggregateSchema.optional(), + /** Row cap for `query_rows` without `aggregate`. Clamped server-side to + * [1, 200]. */ + limit: z.number().int().min(1).max(200).optional(), + offset: z.number().int().min(0).optional(), +}); + +export const QUERY_DATASET_TOOL_NAME = 'query_dataset'; + +export const queryDatasetToolSpec = { + name: QUERY_DATASET_TOOL_NAME, + description: + 'Query structured datasets (CSV imports) the current user has uploaded — tables of rows with typed columns, as opposed to free-text documents. ' + + 'Three operations via `query`:\n' + + '- `list_datasets`: list the caller\'s datasets (id, name, row count, column names+types). Call this FIRST when you don\'t already know the `dataset_id`.\n' + + '- `get_schema`: full column schema (name, inferred type, sample value) for one dataset — pass `dataset_id`.\n' + + '- `query_rows`: filter/aggregate over a dataset\'s rows — pass `dataset_id` plus any of `filters` (column/op/value, `op` one of eq/neq/gt/gte/lt/lte/contains — gt/gte/lt/lte only on number columns, contains only on string columns), `group_by` (a column name), `aggregate` ({fn: count/sum/avg/min/max, column?}), `limit`, `offset`. ' + + 'NEVER invent column names — call `get_schema` first if unsure. Results are always paged/aggregated server-side; the response includes `totalMatched` so you can tell the user when there is more than what was returned.', + input_schema: { + type: 'object' as const, + properties: { + query: { + type: 'string', + enum: ['list_datasets', 'get_schema', 'query_rows'], + }, + dataset_id: { type: 'string' }, + filters: { + type: 'array', + items: { + type: 'object', + properties: { + column: { type: 'string' }, + op: { + type: 'string', + enum: ['eq', 'neq', 'gt', 'gte', 'lt', 'lte', 'contains'], + }, + value: {}, + }, + required: ['column', 'op', 'value'], + }, + }, + group_by: { type: 'string' }, + aggregate: { + type: 'object', + properties: { + fn: { type: 'string', enum: ['count', 'sum', 'avg', 'min', 'max'] }, + column: { type: 'string' }, + }, + required: ['fn'], + }, + limit: { type: 'integer' }, + offset: { type: 'integer' }, + }, + required: ['query'], + }, +}; + +export class QueryDatasetTool { + constructor(private readonly graph: KnowledgeGraph) {} + + async handle(input: unknown): Promise { + const parsed = QueryDatasetInputSchema.safeParse(input); + if (!parsed.success) { + return `Error: invalid query_dataset input — ${parsed.error.issues + .map((i) => `${i.path.join('.')}: ${i.message}`) + .join('; ')}`; + } + const args = parsed.data; + const viewerOmadiaUserId = turnContext.current()?.resolvedOmadiaUserId; + if (!viewerOmadiaUserId) { + return 'Error: query_dataset requires a resolved user identity — not available for this channel/turn.'; + } + + switch (args.query) { + case 'list_datasets': { + const datasets = await this.graph.listDatasets({ + ownerOmadiaUserId: viewerOmadiaUserId, + ...(args.limit !== undefined ? { limit: args.limit } : {}), + }); + return JSON.stringify({ + datasets: datasets.map((d) => ({ + id: d.id, + name: d.name, + sourceFileName: d.sourceFileName, + rowCount: d.rowCount, + columns: d.columns.map((c) => ({ name: c.name, type: c.type })), + createdAt: d.createdAt, + })), + }); + } + + case 'get_schema': { + if (!args.dataset_id) { + return 'Error: get_schema requires `dataset_id`.'; + } + const dataset = await this.graph.getDataset( + args.dataset_id, + viewerOmadiaUserId, + ); + if (!dataset) { + return JSON.stringify({ error: 'not_found_or_not_owned' }); + } + return JSON.stringify({ + id: dataset.id, + name: dataset.name, + rowCount: dataset.rowCount, + columns: dataset.columns, + }); + } + + case 'query_rows': { + if (!args.dataset_id) { + return 'Error: query_rows requires `dataset_id`.'; + } + try { + const result = await this.graph.queryDatasetRows( + args.dataset_id, + viewerOmadiaUserId, + { + ...(args.filters ? { filters: args.filters } : {}), + ...(args.group_by ? { groupBy: args.group_by } : {}), + ...(args.aggregate ? { aggregate: args.aggregate } : {}), + ...(args.limit !== undefined ? { limit: args.limit } : {}), + ...(args.offset !== undefined ? { offset: args.offset } : {}), + }, + ); + if (!result) { + return JSON.stringify({ error: 'not_found_or_not_owned' }); + } + return JSON.stringify(result); + } catch (err) { + if (err instanceof DatasetQueryValidationError) { + return `Error: ${err.code} — ${err.message}. Call \`get_schema\` to see the real column names/types.`; + } + return `Error: query_dataset failed — ${err instanceof Error ? err.message : String(err)}`; + } + } + } + } +} diff --git a/middleware/packages/harness-orchestrator/src/turnContext.ts b/middleware/packages/harness-orchestrator/src/turnContext.ts index 0752d071..30781bff 100644 --- a/middleware/packages/harness-orchestrator/src/turnContext.ts +++ b/middleware/packages/harness-orchestrator/src/turnContext.ts @@ -52,6 +52,22 @@ export interface TurnContextValue { * owner. Undefined for system/ad-hoc turns. */ userId?: string; + /** + * #430 fixup (reviewer round 5) — the turn caller's CANONICAL `omadiaUserId` + * uuid, resolved ONCE by `resolveTurnOwnerIdentity` at turn start (see + * `orchestrator.ts`'s `runTurn`/`chatStream`) and reused by every + * turn-scoped consumer that needs it for a KnowledgeGraph dataset ACL + * check — currently `QueryDatasetTool` (viewer/owner filtering) and + * `ingestAttachments` (dataset ownership on CSV import). + * + * Unlike `userId` above (which is the RAW turn input — a Teams AAD oid for + * a channel turn, already-canonical for HTTP/CLI turns), this field is + * ALWAYS the canonical uuid when set. Undefined when resolution wasn't + * possible (no `KnowledgeGraph` wired up for a channel turn, or resolution + * failed) — callers must treat that as "no identity available", never fall + * back to the raw `userId` for an ACL decision. + */ + resolvedOmadiaUserId?: string; chatParticipants?: ChatParticipantsProvider; /** * Privacy-Proxy Slice 2.1: per-turn privacy handle threaded through the diff --git a/middleware/packages/harness-plugin-privacy-guard/src/index.ts b/middleware/packages/harness-plugin-privacy-guard/src/index.ts index 794930a3..60282947 100644 --- a/middleware/packages/harness-plugin-privacy-guard/src/index.ts +++ b/middleware/packages/harness-plugin-privacy-guard/src/index.ts @@ -18,3 +18,10 @@ export { createPrivacyGuardService } from './service.js'; // tests and for hosts that wire the seam manually. export { createC1HttpDetector, C1_DETECTOR_ID } from './c1Detector.js'; export type { C1HttpDetectorOptions } from './c1Detector.js'; + +// #361 — the C0 regex baseline + substitution pass, re-exported so other +// ingestion paths that need the SAME PII-masking pipeline free-text prompts +// get (not just the turn-scoped `maskUserPrompt` service) can call it +// directly. #430 (dataset import) is the first such caller. +export { createBaselineDetector, maskPrompt, dedupSpans } from './promptMask.js'; +export type { MaskPromptResult, ResolvedSpan } from './promptMask.js'; diff --git a/middleware/packages/plugin-api/src/knowledgeGraph.ts b/middleware/packages/plugin-api/src/knowledgeGraph.ts index 3a89c4fa..1a6e934e 100644 --- a/middleware/packages/plugin-api/src/knowledgeGraph.ts +++ b/middleware/packages/plugin-api/src/knowledgeGraph.ts @@ -691,6 +691,55 @@ export interface KnowledgeGraph { rootExternalIds: string[], opts?: { maxHops?: number; maxNodes?: number }, ): Promise<{ nodes: KgWalkNode[]; edges: KgWalkEdge[] }>; + + /** + * #430 — persist a structured dataset (CSV import) as a relational + * sidecar: one `datasets` row + one `dataset_rows` row per record, plus + * exactly one `Dataset` graph node (`PluginEntity`, `system='dataset'`) + * for recall/citation linking. Individual rows are NEVER promoted to + * graph nodes — node properties are GIN-indexed and exploding rows into + * the graph would defeat that index (see {@link ingestEntities}). `rows` + * MUST already be privacy-scanned by the caller: mirrors the existing + * convention for `ingestTurn`, where masking happens BEFORE the + * KnowledgeGraph boundary, never inside it. + */ + ingestDataset(input: DatasetIngest): Promise; + /** #430 — list datasets owned by the caller, most-recent first. */ + listDatasets(opts: { + ownerOmadiaUserId: string; + limit?: number; + }): Promise; + /** + * #430 — read one dataset's metadata + inferred schema. Null when + * missing or the viewer doesn't own it (ACL mirrors `/api/v1/memory`: + * owner-only for v1, no team/public visibility tier yet). + */ + getDataset( + datasetId: string, + viewerOmadiaUserId: string, + ): Promise; + /** + * #430 — filter/aggregate over a dataset's rows via the constrained + * {@link DatasetQueryOptions} DSL — never raw SQL from the caller. + * Powers the `query_dataset` native tool. Returns `null` under the same + * ACL rule as {@link getDataset}. Always paginates/aggregates server-side + * (`limit` clamped) so a caller can't accidentally dump a whole table + * into turn context. + */ + queryDatasetRows( + datasetId: string, + viewerOmadiaUserId: string, + opts?: DatasetQueryOptions, + ): Promise; + /** + * #430 — hard-delete a dataset, its rows (cascade), and its graph node. + * Owner-only. Returns `false` (no-op) when the dataset is missing or not + * owned by `actor`. + */ + deleteDataset( + datasetId: string, + actor: AclMutationOptions, + ): Promise; } // --------------------------------------------------------------------------- @@ -2003,3 +2052,242 @@ export function factNodeId( s.toLowerCase().replace(/[:\s]+/g, '_').slice(0, 80); return `fact:${sourceTurnId}:${clean(subject)}:${clean(predicate)}:${clean(object)}`; } + +// --------------------------------------------------------------------------- +// Issue #430 — structured dataset ingestion and handling. Relational +// sidecar in the same store (`datasets` + `dataset_rows`), NOT a graph-node +// explosion: rows never become graph nodes, only the parent dataset gets a +// single `PluginEntity` (system='dataset') node. See `ingestDataset`. +// --------------------------------------------------------------------------- + +/** Inferred column type from CSV import (#430). No `unknown` catch-all — + * a column with no confidently-typed values falls back to `'string'`. */ +export type DatasetColumnType = 'string' | 'number' | 'boolean' | 'date'; + +export interface DatasetColumnSchema { + name: string; + type: DatasetColumnType; + /** First non-empty value seen for this column — admin-UI schema preview + * only, never used for query filtering. */ + sample?: string; +} + +export interface DatasetIngest { + /** + * Cluster-root uuid of the importing user. Sole initial ACL owner — + * mirrors the `/api/v1/memory` ACL model (an owners array seeded from + * the session/turn identity, checked on every read/write). + */ + ownerOmadiaUserId: string; + name: string; + sourceFileName: string; + /** + * Tigris object key for the raw uploaded file. Omitted when the dataset + * was auto-ingested from a chat attachment whose raw bytes aren't + * separately persisted to Tigris (see `attachmentExtract.ts`'s CSV + * branch in `@omadia/orchestrator`). + */ + sourceStorageKey?: string; + columns: DatasetColumnSchema[]; + /** + * Already privacy-scanned rows. MUST be scrubbed by the caller before + * this call — see {@link KnowledgeGraph.ingestDataset}. + */ + rows: ReadonlyArray>; +} + +export interface DatasetIngestResult { + datasetId: string; + rowCount: number; + /** External id of the single Dataset `PluginEntity` graph node. */ + graphNodeId: string; +} + +export interface DatasetSummary { + id: string; + name: string; + sourceFileName: string; + ownerOmadiaUserId: string; + rowCount: number; + columns: DatasetColumnSchema[]; + createdAt: string; +} + +export type DatasetFilterOp = + | 'eq' + | 'neq' + | 'gt' + | 'gte' + | 'lt' + | 'lte' + | 'contains'; + +export interface DatasetFilter { + column: string; + op: DatasetFilterOp; + value: string | number | boolean; +} + +export type DatasetAggregateFn = 'count' | 'sum' | 'avg' | 'min' | 'max'; + +export interface DatasetAggregate { + fn: DatasetAggregateFn; + /** Required for every `fn` except `'count'`. */ + column?: string; +} + +/** + * Constrained query DSL for the `query_dataset` native tool (#430) — the + * model never writes raw SQL. Every `column` referenced (in `filters`, + * `groupBy`, or `aggregate`) MUST name a column from the dataset's own + * inferred schema; backends reject unknown columns instead of passing them + * through to a query string. + */ +export interface DatasetQueryOptions { + filters?: DatasetFilter[]; + /** Group the aggregate by this column instead of collapsing to one row. + * Ignored when `aggregate` is omitted. */ + groupBy?: string; + aggregate?: DatasetAggregate; + /** Row cap when no `aggregate` is requested. Clamped to [1, 200] server-side. + * Default 50. */ + limit?: number; + offset?: number; +} + +export interface DatasetQueryResult { + /** Raw matching rows — populated only when `aggregate` was omitted. */ + rows?: Array>; + /** One entry per group — populated only when `aggregate` + `groupBy` were both set. */ + groups?: Array<{ key: unknown; value: number | null }>; + /** Single scalar — populated only when `aggregate` was set without `groupBy`. */ + aggregateValue?: number | null; + /** + * Total rows matching `filters`, before `limit`/`offset` — lets a caller + * (the `query_dataset` tool's response text) surface "showing 50 of 4,213" + * instead of silently truncating. + */ + totalMatched: number; +} + +/** Thrown by {@link validateDatasetQueryOptions} when a `query_dataset` + * request references a column the dataset's inferred schema doesn't have, + * or an aggregate is missing its required column. Caller-input error, not + * an ACL failure — distinct from the `null` (not found / not owned) return + * of {@link KnowledgeGraph.queryDatasetRows}. */ +export class DatasetQueryValidationError extends Error { + constructor( + public readonly code: string, + message: string, + ) { + super(message); + this.name = 'DatasetQueryValidationError'; + } +} + +export interface NormalizedDatasetQuery { + filters: DatasetFilter[]; + groupBy?: string; + aggregate?: DatasetAggregate; + limit: number; + offset: number; +} + +const DATASET_QUERY_LIMIT_MIN = 1; +const DATASET_QUERY_LIMIT_MAX = 200; +const DATASET_QUERY_LIMIT_DEFAULT = 50; + +/** + * Shared validation for the `query_dataset` constrained DSL (#430) — both + * `NeonKnowledgeGraph` and `InMemoryKnowledgeGraph` call this FIRST so the + * two backends reject the same malformed requests identically. Every + * `column` reference is checked against the dataset's own inferred schema + * (`columns`); `limit`/`offset` are clamped rather than rejected, since an + * over-large limit is a harmless caller mistake, not a shape violation. + */ +const NUMERIC_COMPARISON_OPS: ReadonlySet = new Set([ + 'gt', + 'gte', + 'lt', + 'lte', +]); + +export function validateDatasetQueryOptions( + columns: readonly DatasetColumnSchema[], + opts: DatasetQueryOptions | undefined, +): NormalizedDatasetQuery { + const columnsByName = new Map(columns.map((c) => [c.name, c])); + const filters = opts?.filters ?? []; + for (const f of filters) { + const column = columnsByName.get(f.column); + if (!column) { + throw new DatasetQueryValidationError( + 'unknown_filter_column', + `filter references unknown column '${f.column}'`, + ); + } + if (NUMERIC_COMPARISON_OPS.has(f.op) && column.type !== 'number') { + // v1 scope: only numeric comparisons — 'date' columns are stored as + // their original string representation (no canonical parse format + // assumed), so gt/gte/lt/lte on them isn't well-defined yet. A range + // filter on dates is a reasonable follow-up once dates are normalized + // to ISO at import time. + throw new DatasetQueryValidationError( + 'op_type_mismatch', + `filter op '${f.op}' on column '${f.column}' needs a number column (got '${column.type}')`, + ); + } + if (f.op === 'contains' && column.type !== 'string') { + throw new DatasetQueryValidationError( + 'op_type_mismatch', + `filter op 'contains' on column '${f.column}' needs a string column (got '${column.type}')`, + ); + } + } + if (opts?.groupBy !== undefined && !columnsByName.has(opts.groupBy)) { + throw new DatasetQueryValidationError( + 'unknown_group_by_column', + `groupBy references unknown column '${opts.groupBy}'`, + ); + } + if (opts?.aggregate) { + if (opts.aggregate.fn !== 'count') { + if (opts.aggregate.column === undefined) { + throw new DatasetQueryValidationError( + 'aggregate_column_required', + `aggregate '${opts.aggregate.fn}' requires a column`, + ); + } + const aggColumn = columnsByName.get(opts.aggregate.column); + if (!aggColumn) { + throw new DatasetQueryValidationError( + 'unknown_aggregate_column', + `aggregate references unknown column '${opts.aggregate.column}'`, + ); + } + if (aggColumn.type !== 'number') { + throw new DatasetQueryValidationError( + 'aggregate_type_mismatch', + `aggregate '${opts.aggregate.fn}' needs a number column (got '${aggColumn.type}')`, + ); + } + } + } + const rawLimit = opts?.limit ?? DATASET_QUERY_LIMIT_DEFAULT; + const limit = Math.min( + DATASET_QUERY_LIMIT_MAX, + Math.max( + DATASET_QUERY_LIMIT_MIN, + rawLimit === 0 ? 1 : Math.trunc(rawLimit) || DATASET_QUERY_LIMIT_DEFAULT, + ), + ); + const rawOffset = opts?.offset ?? 0; + const offset = Math.max(0, Math.trunc(rawOffset)); + return { + filters, + ...(opts?.groupBy !== undefined ? { groupBy: opts.groupBy } : {}), + ...(opts?.aggregate !== undefined ? { aggregate: opts.aggregate } : {}), + limit, + offset, + }; +} diff --git a/middleware/src/channels/orchestratorDispatcher.ts b/middleware/src/channels/orchestratorDispatcher.ts index a0da2f01..8eeb5d00 100644 --- a/middleware/src/channels/orchestratorDispatcher.ts +++ b/middleware/src/channels/orchestratorDispatcher.ts @@ -1,5 +1,29 @@ import { CHAT_AGENT_SERVICE } from '@omadia/channel-sdk'; -import type { ChatAgent, ChatAgentBundle } from '@omadia/channel-sdk'; +import type { ChatAgent, ChatAgentBundle, ChannelUserKind } from '@omadia/channel-sdk'; +import type { ChannelKind } from '@omadia/plugin-api'; + +/** + * #430 fixup — map the channel-plugin-facing {@link ChannelUserKind} + * namespace to the KG-facing {@link ChannelKind} the ACL/identity model + * understands. Deliberately partial: `discord-user` / `whatsapp-phone` have + * no `ChannelKind` counterpart yet, and `custom` (the canvas/Omadia-UI + * channel's own namespace, which carries its already-resolved + * `omadiaUserId` via a different path — `metadata.omadiaUserId`) is not a + * single channel at all. Callers must treat `undefined` as "cannot safely + * resolve an identity for this turn", not fall back to guessing. + */ +function toChannelKind(kind: ChannelUserKind): ChannelKind | undefined { + switch (kind) { + case 'teams-aad': + return 'teams'; + case 'slack-user': + return 'slack'; + case 'telegram-chat': + return 'telegram'; + default: + return undefined; + } +} import type { ChannelManifestBlock } from '../api/admin-v1.js'; import type { TurnDispatcher } from './coreApi.js'; @@ -127,10 +151,20 @@ export function createOrchestratorDispatcher( typeof (rawState as { basedOnRevision?: unknown }).basedOnRevision === 'string' ? (rawState as { basedOnRevision: string; currentTree: unknown }) : undefined; + // #430 fixup — the ONLY place a `ChatTurnInput.channelIdentity` is + // produced. `userId` above stays the raw channel-native id (unchanged, + // documented behaviour); `channelIdentity` gives downstream code + // (dataset ingest ACL) a typed, resolvable channel kind when one + // exists, without guessing for kinds the KG model doesn't cover. + const channelKind = toChannelKind(input.userRef.kind); + const channelIdentity = channelKind + ? { channelKind, channelUserId: input.userRef.id } + : undefined; yield* agent.chatStream({ userMessage: input.text, sessionScope: input.scope, userId: input.userRef.id, + ...(channelIdentity ? { channelIdentity } : {}), ...(canvasSessionId ? { canvasSessionId } : {}), ...(action ? { action } : {}), ...(canvasRefresh ? { canvasRefresh } : {}), diff --git a/middleware/src/index.ts b/middleware/src/index.ts index d1f5b7f7..5534c4b5 100644 --- a/middleware/src/index.ts +++ b/middleware/src/index.ts @@ -47,6 +47,7 @@ import type { OrchestratorRegistry as MultiOrchestratorRegistry, } from '@omadia/orchestrator'; import { createMemoryRouter } from './routes/memory.js'; +import { createDatasetsRouter } from './routes/datasets.js'; import { createBulkPromotionRouter } from './routes/bulkPromotion.js'; import { createInconsistenciesRouter } from './routes/inconsistencies.js'; import { createDuplicatesRouter } from './routes/duplicates.js'; @@ -2221,6 +2222,15 @@ async function main(): Promise { ); console.log('[middleware] memory endpoint ready at /api/v1/memory (auth-gated)'); + // #430 — structured dataset ingestion (CSV import) REST surface. Same + // requireAuth + per-route session-user ACL pattern as /api/v1/memory. + app.use( + '/api/v1/datasets', + requireAuth, + createDatasetsRouter({ graph: knowledgeGraph }), + ); + console.log('[middleware] datasets endpoint ready at /api/v1/datasets (auth-gated)'); + // Slice 8 — bulk score + promote admin endpoint. Mounted only when // the orchestrator-extras plugin published the bulkPromotion service // (which requires a graphPool capability — i.e. the Neon backend). diff --git a/middleware/src/routes/datasets.ts b/middleware/src/routes/datasets.ts new file mode 100644 index 00000000..1786d909 --- /dev/null +++ b/middleware/src/routes/datasets.ts @@ -0,0 +1,212 @@ +import { Router } from 'express'; +import type { Request, Response } from 'express'; +import multer from 'multer'; +import { z } from 'zod'; + +import type { KnowledgeGraph } from '@omadia/plugin-api'; +import { DatasetQueryValidationError } from '@omadia/plugin-api'; +import { importCsvDataset, isCsvAttachment } from '@omadia/orchestrator'; + +/** + * #430 — REST surface for structured dataset ingestion (CSV import). + * + * Mounted under `/api/v1/datasets`, ACL pattern mirrors `/api/v1/memory` + * (`memory.ts`): `req.session.omadia_user_id` is the sole owner/viewer + * identity, no anonymous access. Upload pattern mirrors the package-zip + * upload route (`packages.ts`): `multer` memory storage, one file per + * request, mapped error codes on 4xx. + * + * Every uploaded row runs through the SAME privacy-scan pipeline as the + * chat-attachment auto-ingest path — both call `importCsvDataset` from + * `@omadia/orchestrator`, so there is exactly one place the scan could be + * skipped, and this route isn't it. + */ + +const MAX_UPLOAD_BYTES = 25 * 1024 * 1024; // mirrors TEAMS_ATTACHMENT_MAX_BYTES default + +const RowsQuerySchema = z.object({ + limit: z.coerce.number().int().min(1).max(200).optional(), + offset: z.coerce.number().int().min(0).optional(), +}); + +function requireSessionUserId(req: Request, res: Response): string | null { + const id = req.session?.omadia_user_id; + if (!id) { + res.status(401).json({ code: 'auth.required', message: 'login required' }); + return null; + } + return id; +} + +function mapErrorToHttp(err: unknown): { status: number; code: string; message: string } { + if (err instanceof DatasetQueryValidationError) { + return { status: 400, code: `dataset.${err.code}`, message: err.message }; + } + const message = err instanceof Error ? err.message : String(err); + return { status: 500, code: 'dataset.internal_error', message }; +} + +export function createDatasetsRouter(deps: { graph: KnowledgeGraph }): Router { + const router = Router(); + + const upload = multer({ + storage: multer.memoryStorage(), + limits: { fileSize: MAX_UPLOAD_BYTES, files: 1, fields: 4 }, + }); + + // ── POST / — multipart CSV upload ─────────────────────────────────────── + router.post( + '/', + (req, res, next) => { + upload.single('file')(req, res, (err: unknown) => { + if (!err) { + next(); + return; + } + const isMulterError = + err instanceof multer.MulterError || + (typeof err === 'object' && err !== null && 'code' in err); + const code = isMulterError + ? String((err as { code?: string }).code ?? 'upload.multipart') + : 'upload.multipart'; + const message = err instanceof Error ? err.message : String(err); + const status = code === 'LIMIT_FILE_SIZE' ? 413 : 400; + res.status(status).json({ code: `dataset.${code.toLowerCase()}`, message }); + }); + }, + async (req: Request, res: Response) => { + const sessionUserId = requireSessionUserId(req, res); + if (!sessionUserId) return; + const file = (req as Request & { file?: Express.Multer.File }).file; + if (!file) { + res + .status(400) + .json({ code: 'dataset.no_file', message: "multipart field 'file' fehlt." }); + return; + } + if (!isCsvAttachment(file.mimetype, file.originalname)) { + res.status(422).json({ + code: 'dataset.unsupported_type', + message: 'Nur CSV-Dateien werden aktuell unterstützt (v1 scope, siehe #430).', + }); + return; + } + const nameField = req.body?.['name']; + const datasetName = + typeof nameField === 'string' && nameField.trim().length > 0 + ? nameField.trim() + : file.originalname; + + try { + const imported = await importCsvDataset({ + graph: deps.graph, + bytes: file.buffer, + datasetName, + sourceFileName: file.originalname, + ownerOmadiaUserId: sessionUserId, + }); + if (!imported.ok) { + res.status(422).json({ code: 'dataset.import_failed', message: imported.reason }); + return; + } + res.status(201).json({ + dataset: imported.result, + privacyScan: imported.privacyScan, + // #430 fixup — cells over MAX_CELL_CHARS are still cut (protects the + // scan + storage from one pathological cell), but the cut is no + // longer silent: callers can see it happened and which columns. + truncation: imported.truncation, + }); + } catch (err) { + // #430 fixup — an unexpected THROWN error (e.g. a transient Postgres + // failure inside NeonKnowledgeGraph.ingestDataset) must still return + // the same {code, message} JSON envelope as the other four dataset + // handlers below, not Express's default HTML error page. The + // structured `{ok: false, reason}` not-ok case above is unaffected — + // this only guards against a thrown exception. + const { status, code, message } = mapErrorToHttp(err); + res.status(status).json({ code, message }); + } + }, + ); + + // ── GET / — list current user's datasets ──────────────────────────────── + router.get('/', async (req: Request, res: Response) => { + const sessionUserId = requireSessionUserId(req, res); + if (!sessionUserId) return; + try { + const items = await deps.graph.listDatasets({ ownerOmadiaUserId: sessionUserId }); + res.json({ items }); + } catch (err) { + const { status, code, message } = mapErrorToHttp(err); + res.status(status).json({ code, message }); + } + }); + + // ── GET /:id — one dataset's schema ────────────────────────────────────── + router.get('/:id', async (req: Request, res: Response) => { + const sessionUserId = requireSessionUserId(req, res); + if (!sessionUserId) return; + try { + const dataset = await deps.graph.getDataset(String(req.params['id'] ?? ''), sessionUserId); + if (!dataset) { + res.status(404).json({ code: 'dataset.not_found' }); + return; + } + res.json(dataset); + } catch (err) { + const { status, code, message } = mapErrorToHttp(err); + res.status(status).json({ code, message }); + } + }); + + // ── GET /:id/rows — paginated row preview (admin UI table) ───────────── + router.get('/:id/rows', async (req: Request, res: Response) => { + const sessionUserId = requireSessionUserId(req, res); + if (!sessionUserId) return; + const parsed = RowsQuerySchema.safeParse(req.query); + if (!parsed.success) { + res.status(400).json({ code: 'dataset.invalid_query', issues: parsed.error.issues }); + return; + } + try { + const result = await deps.graph.queryDatasetRows( + String(req.params['id'] ?? ''), + sessionUserId, + { + ...(parsed.data.limit !== undefined ? { limit: parsed.data.limit } : {}), + ...(parsed.data.offset !== undefined ? { offset: parsed.data.offset } : {}), + }, + ); + if (!result) { + res.status(404).json({ code: 'dataset.not_found' }); + return; + } + res.json(result); + } catch (err) { + const { status, code, message } = mapErrorToHttp(err); + res.status(status).json({ code, message }); + } + }); + + // ── DELETE /:id — hard delete ──────────────────────────────────────────── + router.delete('/:id', async (req: Request, res: Response) => { + const sessionUserId = requireSessionUserId(req, res); + if (!sessionUserId) return; + try { + const deleted = await deps.graph.deleteDataset(String(req.params['id'] ?? ''), { + actorOmadiaUserId: sessionUserId, + }); + if (!deleted) { + res.status(404).json({ code: 'dataset.not_found' }); + return; + } + res.status(204).end(); + } catch (err) { + const { status, code, message } = mapErrorToHttp(err); + res.status(status).json({ code, message }); + } + }); + + return router; +} diff --git a/middleware/test/datasetImport.test.ts b/middleware/test/datasetImport.test.ts new file mode 100644 index 00000000..e9364f88 --- /dev/null +++ b/middleware/test/datasetImport.test.ts @@ -0,0 +1,264 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { InMemoryKnowledgeGraph } from '@omadia/knowledge-graph-inmemory'; + +import { + buildDatasetFromCsv, + importCsvDataset, + parseCsv, + MAX_DATASET_ROWS, +} from '../packages/harness-orchestrator/src/datasetImport.js'; + +describe('parseCsv', () => { + it('parses header + rows into header-keyed string records', () => { + const csv = 'name,age\nAda,36\nGrace,85\n'; + const result = parseCsv(Buffer.from(csv, 'utf8')); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.deepEqual(result.headers, ['name', 'age']); + assert.deepEqual(result.rows, [ + { name: 'Ada', age: '36' }, + { name: 'Grace', age: '85' }, + ]); + }); + + it('rejects a CSV with no data rows', () => { + const result = parseCsv(Buffer.from('name,age\n', 'utf8')); + assert.equal(result.ok, false); + }); + + it('rejects a CSV over the row cap', () => { + const header = 'v\n'; + const rows = Array.from({ length: MAX_DATASET_ROWS + 1 }, (_, i) => String(i)).join('\n'); + const result = parseCsv(Buffer.from(header + rows, 'utf8')); + assert.equal(result.ok, false); + if (result.ok) return; + assert.match(result.reason, /row/i); + }); + + it('reports zero truncation for a CSV with no over-limit cells', () => { + const result = parseCsv(Buffer.from('name,age\nAda,36\n', 'utf8')); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.truncation.truncatedCellCount, 0); + assert.deepEqual(result.truncation.truncatedColumns, []); + }); + + it('#430 fixup — surfaces truncatedCellCount + truncatedColumns instead of silently cutting an over-limit cell', () => { + const longValue = 'x'.repeat(5000); + const csv = `name,notes\nAda,${longValue}\nGrace,short\n`; + const result = parseCsv(Buffer.from(csv, 'utf8')); + assert.equal(result.ok, true); + if (!result.ok) return; + assert.equal(result.truncation.truncatedCellCount, 1); + assert.deepEqual(result.truncation.truncatedColumns, ['notes']); + // The cell is still cut (protects storage/scan) — just no longer silent. + assert.equal(result.rows[0]?.['notes']?.length, 4000); + }); +}); + +describe('buildDatasetFromCsv — column type inference', () => { + it('infers number, boolean, and string columns', async () => { + const csv = 'name,age,active\nAda,36,true\nGrace,85,false\n'; + const built = await buildDatasetFromCsv(Buffer.from(csv, 'utf8')); + assert.equal(built.ok, true); + if (!built.ok) return; + const byName = new Map(built.columns.map((c) => [c.name, c])); + assert.equal(byName.get('name')?.type, 'string'); + assert.equal(byName.get('age')?.type, 'number'); + assert.equal(byName.get('active')?.type, 'boolean'); + assert.equal(built.rows[0]?.['age'], 36); + assert.equal(built.rows[0]?.['active'], true); + }); + + it('falls back to string when a column has one non-conforming value', async () => { + const csv = 'code\n123\nAB12\n'; + const built = await buildDatasetFromCsv(Buffer.from(csv, 'utf8')); + assert.equal(built.ok, true); + if (!built.ok) return; + assert.equal(built.columns[0]?.type, 'string'); + assert.equal(built.rows[0]?.['code'], '123'); + }); +}); + +describe('buildDatasetFromCsv — privacy scan', () => { + it('masks an email found in a string column before storage', async () => { + const csv = 'name,contact\nAda,ada@example.com\n'; + const built = await buildDatasetFromCsv(Buffer.from(csv, 'utf8')); + assert.equal(built.ok, true); + if (!built.ok) return; + const contact = String(built.rows[0]?.['contact'] ?? ''); + assert.ok(!contact.includes('ada@example.com'), 'raw email must not survive the scan'); + assert.equal(built.privacyScan.maskedCells, 1); + }); + + it('does not scan (and cannot corrupt) a number-typed column', async () => { + const csv = 'id,amount\n1,1000\n2,2000\n'; + const built = await buildDatasetFromCsv(Buffer.from(csv, 'utf8')); + assert.equal(built.ok, true); + if (!built.ok) return; + assert.equal(built.rows[0]?.['amount'], 1000); + assert.equal(built.rows[1]?.['amount'], 2000); + }); + + it('types a zero-padded, digit-only column as string — not number — so a short zero-padded code round-trips without corruption (#430 fixup)', async () => { + // '012'/'019' are short enough that the baseline PII detector does not + // treat them as phone-number-shaped, so this case isolates the + // type-inference fix in the clear: no privacy-masking noise, just proof + // that the leading zero is no longer silently dropped by `Number()`. + const csv = 'name,code\nAda,012\nGrace,019\n'; + const built = await buildDatasetFromCsv(Buffer.from(csv, 'utf8')); + assert.equal(built.ok, true); + if (!built.ok) return; + + const byName = new Map(built.columns.map((c) => [c.name, c])); + assert.equal( + byName.get('code')?.type, + 'string', + 'a leading-zero digit string must not be typed number', + ); + // Value round-trips with its leading zero intact — Number() would have + // silently dropped it (e.g. '012' -> 12). + assert.equal(built.rows[0]?.['code'], '012'); + assert.equal(built.rows[1]?.['code'], '019'); + // The column is string-typed, so the mandatory privacy scan actually + // runs over it (2 cells) instead of being bypassed the way a + // number-typed column is (see the sibling test above). + assert.ok( + built.privacyScan.scannedCells >= 2, + `expected the leading-zero column to be scanned, got scannedCells=${String(built.privacyScan.scannedCells)}`, + ); + + // A bare '0'/'0.x' decimal is still a legitimate number column. + const decimalCsv = 'ratio\n0\n0.5\n'; + const decimalBuilt = await buildDatasetFromCsv(Buffer.from(decimalCsv, 'utf8')); + assert.equal(decimalBuilt.ok, true); + if (!decimalBuilt.ok) return; + assert.equal(decimalBuilt.columns[0]?.type, 'number'); + }); + + it('types a signed, zero-padded column as string — not number — so a negative zero-padded code round-trips without corruption (#430 fixup round 6)', async () => { + // Before this fix, LEADING_ZERO_RE only matched an unsigned leading + // zero ('0123'), so a signed zero-padded value like '-012' still + // passed NUMBER_RE (which allows an optional leading '-') without + // tripping the leading-zero guard. That silently mistyped the column + // as 'number' (Number('-012') === -12, dropping the leading zero) and + // skipped the mandatory privacy scan. '012'/'019' (as opposed to a + // longer digit run) are short enough that the baseline PII detector's + // phone pattern does not also fire, isolating the type-inference fix. + const csv = 'name,code\nAda,-012\nGrace,-019\n'; + const built = await buildDatasetFromCsv(Buffer.from(csv, 'utf8')); + assert.equal(built.ok, true); + if (!built.ok) return; + + const byName = new Map(built.columns.map((c) => [c.name, c])); + assert.equal( + byName.get('code')?.type, + 'string', + 'a signed leading-zero digit string must not be typed number', + ); + // Value round-trips with its sign and leading zero intact — Number() + // would have silently dropped the zero (e.g. '-012' -> -12). + assert.equal(built.rows[0]?.['code'], '-012'); + assert.equal(built.rows[1]?.['code'], '-019'); + // The column is string-typed, so the mandatory privacy scan actually + // runs over it instead of being bypassed the way a number-typed + // column is. + assert.ok( + built.privacyScan.scannedCells >= 2, + `expected the signed leading-zero column to be scanned, got scannedCells=${String(built.privacyScan.scannedCells)}`, + ); + + // A bare '0'/'-0.x' decimal is still a legitimate number column. + const decimalCsv = 'ratio\n-0\n-0.5\n'; + const decimalBuilt = await buildDatasetFromCsv(Buffer.from(decimalCsv, 'utf8')); + assert.equal(decimalBuilt.ok, true); + if (!decimalBuilt.ok) return; + assert.equal(decimalBuilt.columns[0]?.type, 'number'); + }); + + it('a zero-padded phone number no longer bypasses the mandatory privacy scan (#430 fixup — the exact scenario from the reviewer report)', async () => { + // Before the fix, '0301234567' was inferred as type 'number': the raw + // digits were stored via `Number()` (silently dropping the leading + // zero, corrupting the value to 301234567) AND the column was excluded + // from the privacy scan entirely — a real German phone number would + // have been persisted un-redacted. After the fix the column is typed + // 'string', so it goes through the same mandatory C0 scan as any other + // free-text column and gets masked like the shipped phone-number + // pattern is meant to. + const csv = 'name,phone\nAda,0301234567\nGrace,0301234568\n'; + const built = await buildDatasetFromCsv(Buffer.from(csv, 'utf8')); + assert.equal(built.ok, true); + if (!built.ok) return; + + const byName = new Map(built.columns.map((c) => [c.name, c])); + assert.equal(byName.get('phone')?.type, 'string'); + + const stored = String(built.rows[0]?.['phone'] ?? ''); + // Never the leading-zero-dropped, `Number()`-corrupted value. + assert.notEqual(stored, '301234567'); + // Never the raw, un-redacted phone number either — the scan must have + // masked it, proving the bypass is closed. + assert.notEqual(stored, '0301234567'); + assert.ok( + built.privacyScan.scannedCells >= 2, + `expected the phone column to be scanned, got scannedCells=${String(built.privacyScan.scannedCells)}`, + ); + assert.ok( + built.privacyScan.maskedCells >= 1, + 'expected the phone number to be masked by the baseline detector', + ); + }); +}); + +describe('importCsvDataset', () => { + it('persists a dataset + rows via KnowledgeGraph.ingestDataset', async () => { + const graph = new InMemoryKnowledgeGraph(); + const csv = 'name,age\nAda,36\nGrace,85\n'; + const imported = await importCsvDataset({ + graph, + bytes: Buffer.from(csv, 'utf8'), + datasetName: 'People', + sourceFileName: 'people.csv', + ownerOmadiaUserId: 'user-1', + }); + assert.equal(imported.ok, true); + if (!imported.ok) return; + assert.equal(imported.result.rowCount, 2); + + const dataset = await graph.getDataset(imported.result.datasetId, 'user-1'); + assert.ok(dataset); + assert.equal(dataset?.name, 'People'); + assert.equal(dataset?.rowCount, 2); + assert.equal(imported.truncation.truncatedCellCount, 0); + }); + + it('#430 fixup — threads truncation stats through to the top-level import result', async () => { + const graph = new InMemoryKnowledgeGraph(); + const longValue = 'y'.repeat(4500); + const imported = await importCsvDataset({ + graph, + bytes: Buffer.from(`name,bio\nAda,${longValue}\n`, 'utf8'), + datasetName: 'Bios', + sourceFileName: 'bios.csv', + ownerOmadiaUserId: 'user-1', + }); + assert.equal(imported.ok, true); + if (!imported.ok) return; + assert.equal(imported.truncation.truncatedCellCount, 1); + assert.deepEqual(imported.truncation.truncatedColumns, ['bio']); + }); + + it('surfaces a clean failure reason for a malformed CSV instead of throwing', async () => { + const graph = new InMemoryKnowledgeGraph(); + const imported = await importCsvDataset({ + graph, + bytes: Buffer.from('', 'utf8'), + datasetName: 'Empty', + sourceFileName: 'empty.csv', + ownerOmadiaUserId: 'user-1', + }); + assert.equal(imported.ok, false); + }); +}); diff --git a/middleware/test/datasetsRoute.test.ts b/middleware/test/datasetsRoute.test.ts new file mode 100644 index 00000000..98710a65 --- /dev/null +++ b/middleware/test/datasetsRoute.test.ts @@ -0,0 +1,155 @@ +import { strict as assert } from 'node:assert'; +import type { Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { after, before, describe, it } from 'node:test'; + +import express from 'express'; +import type { NextFunction, Request, Response } from 'express'; + +import { InMemoryKnowledgeGraph } from '@omadia/knowledge-graph-inmemory'; + +import { createDatasetsRouter } from '../src/routes/datasets.js'; + +/** + * HTTP integration test for the #430 datasets REST surface, mirroring the + * `memoryPurgeRoute.test.ts` pattern: a real express `listen(0)` server over + * a real `InMemoryKnowledgeGraph`. `requireAuth` is applied at MOUNT time in + * prod, not inside the router, so the test injects `req.session` directly + * via a tiny fixture middleware instead of exercising real auth. + */ + +const MOUNT = '/api/v1/datasets'; + +interface Harness { + baseUrl: string; + graph: InMemoryKnowledgeGraph; + close: () => Promise; +} + +function withSession(userId: string | undefined) { + return (req: Request, _res: Response, next: NextFunction): void => { + (req as Request & { session?: { omadia_user_id?: string } }).session = userId + ? { omadia_user_id: userId } + : {}; + next(); + }; +} + +// No default value here on purpose — a default would silently win over an +// explicit `undefined` argument (JS default-parameter semantics), which is +// exactly the "anonymous caller" case this harness needs to express. +async function makeHarness( + userId: string | undefined, + graph: InMemoryKnowledgeGraph = new InMemoryKnowledgeGraph(), +): Promise { + const app = express(); + app.use(express.json()); + app.use(MOUNT, withSession(userId), createDatasetsRouter({ graph })); + const server: Server = app.listen(0); + await new Promise((resolve) => server.once('listening', resolve)); + const { port } = server.address() as AddressInfo; + return { + baseUrl: `http://127.0.0.1:${String(port)}${MOUNT}`, + graph, + close: async () => { + await new Promise((resolve) => server.close(() => resolve())); + }, + }; +} + +const CSV = 'name,age\nAda,36\nGrace,85\n'; + +/** #430 fixup regression harness — a graph whose `ingestDataset` throws, + * simulating an unexpected failure (e.g. a transient Postgres error inside + * `NeonKnowledgeGraph.ingestDataset`) rather than returning the structured + * `{ok: false, reason}` not-ok result `importCsvDataset` handles already. */ +class ThrowingIngestKnowledgeGraph extends InMemoryKnowledgeGraph { + override async ingestDataset(): Promise { + throw new Error('simulated ingest failure'); + } +} + +describe('POST /api/v1/datasets', () => { + let h: Harness; + before(async () => { + h = await makeHarness('user-1'); + }); + after(() => h.close()); + + it('401s without a session', async () => { + const anon = await makeHarness(undefined); + const res = await fetch(anon.baseUrl, { method: 'GET' }); + assert.equal(res.status, 401); + await anon.close(); + }); + + it('rejects a non-CSV upload with 422', async () => { + const form = new FormData(); + form.append('file', new Blob(['hello'], { type: 'text/plain' }), 'notes.txt'); + const res = await fetch(h.baseUrl, { method: 'POST', body: form }); + assert.equal(res.status, 422); + }); + + it('uploads a CSV, lists it, reads its schema + rows, then deletes it', async () => { + const form = new FormData(); + form.append('file', new Blob([CSV], { type: 'text/csv' }), 'people.csv'); + form.append('name', 'People'); + const uploadRes = await fetch(h.baseUrl, { method: 'POST', body: form }); + assert.equal(uploadRes.status, 201); + const uploadBody = (await uploadRes.json()) as { + dataset: { datasetId: string; rowCount: number }; + }; + assert.equal(uploadBody.dataset.rowCount, 2); + const { datasetId } = uploadBody.dataset; + + const listRes = await fetch(h.baseUrl); + const listBody = (await listRes.json()) as { items: Array<{ id: string; name: string }> }; + assert.equal(listBody.items.length, 1); + assert.equal(listBody.items[0]?.name, 'People'); + + const schemaRes = await fetch(`${h.baseUrl}/${datasetId}`); + const schemaBody = (await schemaRes.json()) as { columns: Array<{ name: string }> }; + assert.deepEqual( + schemaBody.columns.map((c) => c.name), + ['name', 'age'], + ); + + const rowsRes = await fetch(`${h.baseUrl}/${datasetId}/rows`); + const rowsBody = (await rowsRes.json()) as { rows: Array> }; + assert.equal(rowsBody.rows.length, 2); + + const deleteRes = await fetch(`${h.baseUrl}/${datasetId}`, { method: 'DELETE' }); + assert.equal(deleteRes.status, 204); + const afterDeleteRes = await fetch(`${h.baseUrl}/${datasetId}`); + assert.equal(afterDeleteRes.status, 404); + }); + + it('returns a JSON {code, message} body — not an unhandled rejection / Express default page — when importCsvDataset throws', async () => { + const throwing = await makeHarness('user-1', new ThrowingIngestKnowledgeGraph()); + const form = new FormData(); + form.append('file', new Blob([CSV], { type: 'text/csv' }), 'people.csv'); + const res = await fetch(throwing.baseUrl, { method: 'POST', body: form }); + assert.equal(res.status, 500); + assert.equal(res.headers.get('content-type')?.includes('application/json'), true); + const body = (await res.json()) as { code: string; message: string }; + assert.equal(body.code, 'dataset.internal_error'); + assert.equal(body.message, 'simulated ingest failure'); + await throwing.close(); + }); + + it('scopes datasets per owner — a different session cannot see or delete them', async () => { + const form = new FormData(); + form.append('file', new Blob([CSV], { type: 'text/csv' }), 'people.csv'); + const uploadRes = await fetch(h.baseUrl, { method: 'POST', body: form }); + const { dataset } = (await uploadRes.json()) as { dataset: { datasetId: string } }; + + const other = await makeHarness('user-2'); + const getRes = await fetch(`${other.baseUrl}/${dataset.datasetId}`); + assert.equal(getRes.status, 404); + const deleteRes = await fetch(`${other.baseUrl}/${dataset.datasetId}`, { + method: 'DELETE', + }); + assert.equal(deleteRes.status, 404); + await other.close(); + }); +}); diff --git a/middleware/test/inMemoryKnowledgeGraph.test.ts b/middleware/test/inMemoryKnowledgeGraph.test.ts index f723d583..0f0cd2ca 100644 --- a/middleware/test/inMemoryKnowledgeGraph.test.ts +++ b/middleware/test/inMemoryKnowledgeGraph.test.ts @@ -140,3 +140,263 @@ describe('InMemoryKnowledgeGraph.ingestTurn', () => { assert.equal(await g.getSession('nonexistent'), null); }); }); + +// #430 — structured dataset ingestion. +describe('InMemoryKnowledgeGraph — datasets (#430)', () => { + it('ingests a dataset, creates exactly one Dataset graph node, and is listable/gettable by owner', async () => { + const g = new InMemoryKnowledgeGraph(); + const result = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'People', + sourceFileName: 'people.csv', + columns: [ + { name: 'name', type: 'string' }, + { name: 'age', type: 'number' }, + ], + rows: [ + { name: 'Ada', age: 36 }, + { name: 'Grace', age: 85 }, + ], + }); + assert.equal(result.rowCount, 2); + + const stats = await g.stats(); + assert.equal(stats.byNodeType.PluginEntity, 1); + + const listed = await g.listDatasets({ ownerOmadiaUserId: 'user-1' }); + assert.equal(listed.length, 1); + assert.equal(listed[0]?.id, result.datasetId); + + const fetched = await g.getDataset(result.datasetId, 'user-1'); + assert.ok(fetched); + assert.equal(fetched?.rowCount, 2); + }); + + it('hides a dataset from a non-owner (getDataset/listDatasets/queryDatasetRows all return null/empty)', async () => { + const g = new InMemoryKnowledgeGraph(); + const result = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Secret', + sourceFileName: 's.csv', + columns: [{ name: 'v', type: 'number' }], + rows: [{ v: 1 }], + }); + assert.equal(await g.getDataset(result.datasetId, 'user-2'), null); + assert.deepEqual(await g.listDatasets({ ownerOmadiaUserId: 'user-2' }), []); + assert.equal(await g.queryDatasetRows(result.datasetId, 'user-2'), null); + }); + + it('filters rows via the constrained DSL (eq / contains / numeric comparisons)', async () => { + const g = new InMemoryKnowledgeGraph(); + const { datasetId } = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Sales', + sourceFileName: 'sales.csv', + columns: [ + { name: 'region', type: 'string' }, + { name: 'amount', type: 'number' }, + ], + rows: [ + { region: 'North', amount: 100 }, + { region: 'South', amount: 250 }, + { region: 'North', amount: 400 }, + ], + }); + + const north = await g.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'region', op: 'eq', value: 'North' }], + }); + assert.equal(north?.totalMatched, 2); + assert.equal(north?.rows?.length, 2); + + const big = await g.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'amount', op: 'gt', value: 200 }], + }); + assert.equal(big?.totalMatched, 2); + + const contains = await g.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'region', op: 'contains', value: 'orth' }], + }); + assert.equal(contains?.totalMatched, 2); + }); + + // #430 review fixup — `matchesDatasetFilter`'s `eq`/`neq`/`contains` cases + // must coerce `filter.value` to the column's declared type BEFORE + // comparing, exactly like `buildDatasetFilterClause` does for the Neon + // backend (`(data->>col)::numeric = $1::numeric` for a `number` column). + // Without that coercion, a `number` column storing a JS `number` row value + // silently failed to match a filter value that arrived as a JSON string + // (the tool's Zod schema allows `string | number | boolean` regardless of + // the target column's type or op) — the exact same logical query matched + // on the Neon backend but returned `totalMatched: 0` here. + it('coerces a string filter value against a number column for eq (backend parity, #430 fixup)', async () => { + const g = new InMemoryKnowledgeGraph(); + const { datasetId } = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Sales', + sourceFileName: 'sales.csv', + columns: [ + { name: 'region', type: 'string' }, + { name: 'amount', type: 'number' }, + { name: 'label', type: 'string' }, + ], + rows: [ + { region: 'North', amount: 100, label: 'Order-100' }, + { region: 'South', amount: 250, label: 'Order-250' }, + { region: 'North', amount: 400, label: 'Order-400' }, + ], + }); + + // `amount` is a `number` column storing `250` as a JS number; the filter + // value arrives as the string `'250'` — must still match. + const eqCoerced = await g.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'amount', op: 'eq', value: '250' }], + }); + assert.equal(eqCoerced?.totalMatched, 1); + assert.equal(eqCoerced?.rows?.[0]?.['region'], 'South'); + + // Mirror case for `neq`: everything except the coerced match. + const neqCoerced = await g.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'amount', op: 'neq', value: '250' }], + }); + assert.equal(neqCoerced?.totalMatched, 2); + + // Mirror case for `contains`: `filter.value` arrives as a number even + // though the target column (`label`) is `string` — must be coerced to + // a string before the substring check instead of being rejected + // outright (the old code required `typeof filter.value === 'string'`). + const containsCoerced = await g.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'label', op: 'contains', value: 400 as unknown as string }], + }); + assert.equal(containsCoerced?.totalMatched, 1); + assert.equal(containsCoerced?.rows?.[0]?.['label'], 'Order-400'); + }); + + it('clamps an explicit limit:0 to 1 row instead of silently falling back to the default (#430 fixup)', async () => { + const g = new InMemoryKnowledgeGraph(); + const { datasetId } = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Sales', + sourceFileName: 'sales.csv', + columns: [{ name: 'region', type: 'string' }], + rows: [{ region: 'North' }, { region: 'South' }], + }); + + const zeroLimit = await g.queryDatasetRows(datasetId, 'user-1', { limit: 0 }); + assert.equal(zeroLimit?.rows?.length, 1, 'limit:0 must clamp to 1, not fall back to the default'); + assert.equal(zeroLimit?.totalMatched, 2); + }); + + it('aggregates with and without groupBy', async () => { + const g = new InMemoryKnowledgeGraph(); + const { datasetId } = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Sales', + sourceFileName: 'sales.csv', + columns: [ + { name: 'region', type: 'string' }, + { name: 'amount', type: 'number' }, + ], + rows: [ + { region: 'North', amount: 100 }, + { region: 'South', amount: 250 }, + { region: 'North', amount: 400 }, + ], + }); + + const total = await g.queryDatasetRows(datasetId, 'user-1', { + aggregate: { fn: 'sum', column: 'amount' }, + }); + assert.equal(total?.aggregateValue, 750); + + const byRegion = await g.queryDatasetRows(datasetId, 'user-1', { + groupBy: 'region', + aggregate: { fn: 'sum', column: 'amount' }, + }); + const asMap = new Map(byRegion?.groups?.map((gr) => [gr.key, gr.value])); + assert.equal(asMap.get('North'), 500); + assert.equal(asMap.get('South'), 250); + + const count = await g.queryDatasetRows(datasetId, 'user-1', { + aggregate: { fn: 'count' }, + }); + assert.equal(count?.aggregateValue, 3); + }); + + it('#430 fixup — caps grouped results at 200, matching the Neon backend LIMIT, sorted by value descending', async () => { + const g = new InMemoryKnowledgeGraph(); + const rows = Array.from({ length: 250 }, (_, i) => ({ + key: `k${String(i)}`, + amount: i, // distinct value per group so the sort order is unambiguous + })); + const { datasetId } = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'ManyGroups', + sourceFileName: 'many.csv', + columns: [ + { name: 'key', type: 'string' }, + { name: 'amount', type: 'number' }, + ], + rows, + }); + + const result = await g.queryDatasetRows(datasetId, 'user-1', { + groupBy: 'key', + aggregate: { fn: 'sum', column: 'amount' }, + }); + assert.equal(result?.groups?.length, 200, 'must cap at 200 groups even though 250 unique keys exist'); + // `totalMatched` still reflects every row, not just the returned groups. + assert.equal(result?.totalMatched, 250); + // Deterministic: sorted by value descending, so the 200 HIGHEST-amount + // groups survive (k249..k50), not the first 200 inserted (k0..k199). + const values = (result?.groups ?? []).map((gr) => gr.value); + assert.deepEqual(values, [...values].sort((a, b) => (b ?? 0) - (a ?? 0))); + assert.equal(result?.groups?.[0]?.key, 'k249'); + assert.equal( + result?.groups?.some((gr) => gr.key === 'k0'), + false, + 'the lowest-value group must be truncated away, not the last-inserted one', + ); + }); + + it('rejects an unknown filter column and an aggregate on a non-number column', async () => { + const g = new InMemoryKnowledgeGraph(); + const { datasetId } = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'D', + sourceFileName: 'd.csv', + columns: [{ name: 'name', type: 'string' }], + rows: [{ name: 'Ada' }], + }); + await assert.rejects( + g.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'nope', op: 'eq', value: 1 }], + }), + ); + await assert.rejects( + g.queryDatasetRows(datasetId, 'user-1', { + aggregate: { fn: 'sum', column: 'name' }, + }), + ); + }); + + it('deletes a dataset (owner-only) and drops its graph node', async () => { + const g = new InMemoryKnowledgeGraph(); + const { datasetId } = await g.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'D', + sourceFileName: 'd.csv', + columns: [{ name: 'v', type: 'number' }], + rows: [{ v: 1 }], + }); + assert.equal( + await g.deleteDataset(datasetId, { actorOmadiaUserId: 'user-2' }), + false, + 'non-owner delete is a no-op', + ); + assert.equal(await g.deleteDataset(datasetId, { actorOmadiaUserId: 'user-1' }), true); + assert.equal(await g.getDataset(datasetId, 'user-1'), null); + const stats = await g.stats(); + assert.equal(stats.byNodeType.PluginEntity ?? 0, 0); + }); +}); diff --git a/middleware/test/neonDatasetFilterEscaping.test.ts b/middleware/test/neonDatasetFilterEscaping.test.ts new file mode 100644 index 00000000..59f45702 --- /dev/null +++ b/middleware/test/neonDatasetFilterEscaping.test.ts @@ -0,0 +1,60 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { + NeonKnowledgeGraph, + createNeonPool, +} from '@omadia/knowledge-graph-neon'; + +/** + * Live-Neon integration test for the #430 fixup — round-2 review finding 3 + * (`contains` filter's ILIKE wildcard escaping). Gated on `DATABASE_URL`, + * same convention as `palaiaHybridRetrievalNeon.test.ts`, so a default + * `npm test` without env stays fully hermetic; requires migration + * `0029_datasets.sql` to already be applied on the target DB. + * + * Self-contained (unlike the palaia test, doesn't rely on pre-existing dev-DB + * fixtures): ingests its own tiny dataset per test run via `ingestDataset`. + */ + +const DSN = process.env['DATABASE_URL']; +const ENABLED = typeof DSN === 'string' && DSN.length > 0; + +const describeIf = ENABLED ? describe : describe.skip; + +describeIf('NeonKnowledgeGraph — dataset contains-filter wildcard escaping (#430 fixup)', () => { + it('matches a literal % / _ in the filter value as a literal substring, not a SQL wildcard', async () => { + const pool = createNeonPool(DSN as string); + const graph = new NeonKnowledgeGraph({ pool, tenantId: `test-${String(Date.now())}` }); + try { + const { datasetId } = await graph.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Discounts', + sourceFileName: 'discounts.csv', + columns: [{ name: 'label', type: 'string' }], + rows: [ + { label: '10% off' }, + { label: '10x off' }, // would ALSO match unescaped `%` as a wildcard + { label: 'no_discount' }, + { label: 'noXdiscount' }, // would ALSO match unescaped `_` as a wildcard + ], + }); + + const percent = await graph.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'label', op: 'contains', value: '10%' }], + }); + assert.equal(percent?.totalMatched, 1, 'literal "10%" must match only "10% off", not "10x off"'); + + const underscore = await graph.queryDatasetRows(datasetId, 'user-1', { + filters: [{ column: 'label', op: 'contains', value: 'no_discount' }], + }); + assert.equal( + underscore?.totalMatched, + 1, + 'literal "no_discount" must match only itself, not "noXdiscount"', + ); + } finally { + await pool.end(); + } + }); +}); diff --git a/middleware/test/orchestratorCsvDatasetIdentity.test.ts b/middleware/test/orchestratorCsvDatasetIdentity.test.ts new file mode 100644 index 00000000..ec5c2236 --- /dev/null +++ b/middleware/test/orchestratorCsvDatasetIdentity.test.ts @@ -0,0 +1,151 @@ +/** + * #430 fixup (reviewer round 2, finding 1) — orchestrator-level coverage for + * the chat-attachment CSV auto-ingest path's dataset ownership. Before the + * fix, `ingestAttachments` wrote `ownerOmadiaUserId: input.userId` directly — + * for a channel turn that's the RAW channel-native id (Teams AAD oid, …), + * NOT the canonical `omadiaUserId` uuid the KG's ACL routes filter on. This + * verifies the resolved identity (via `input.channelIdentity` + + * `KnowledgeGraph.resolveOrCreateChannelIdentity`) is what actually gets + * used as the dataset owner. + */ + +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import type { LlmProvider, LlmRequest, LlmResponse, LlmStreamEvent } from '@omadia/llm-provider'; +import { + type AttachmentReader, + NativeToolRegistry, + Orchestrator, +} from '@omadia/orchestrator'; +import { InMemoryKnowledgeGraph } from '@omadia/knowledge-graph-inmemory'; + +const providerCapabilities = { + tools: true, + vision: true, + streaming: true, + promptCaching: true, + forcedToolChoice: true, + parallelToolCalls: true, +} as const; + +function textResponse(text: string): LlmResponse { + return { + content: [{ type: 'text', text }], + finishReason: 'stop', + providerFinishReason: 'end_turn', + model: 'test', + usage: { inputTokens: 10, outputTokens: 1, cacheReadTokens: 0, cacheWriteTokens: 0 }, + }; +} + +/** Same fake-provider pattern as `orchestratorVisionAttachmentIngest.test.ts`. */ +function recordingProvider(requests: LlmRequest[]): LlmProvider { + const provider = { + id: 'anthropic', + capabilities: providerCapabilities, + complete: async (req: LlmRequest): Promise => { + requests.push(req); + return textResponse('ok'); + }, + stream: (): AsyncIterable => { + throw new Error('recordingProvider: stream() not scripted'); + }, + classifyError: () => ({ retryable: false, kind: 'other' as const }), + }; + return provider as unknown as LlmProvider; +} + +function fakeAttachmentReader(byStorageKey: Record): AttachmentReader { + return { + readByStorageKey: async (storageKey: string) => byStorageKey[storageKey], + readByUrl: async () => undefined, + }; +} + +const CSV_BYTES = Buffer.from('name,age\nAda,36\nGrace,85\n', 'utf8'); + +const CSV_MANIFEST = + 'Hier ist die Datei.\n\n' + + '[attachments-info] 1 Datei(en) in diesem Turn hochgeladen + persistiert:\n' + + '- data.csv (text/csv, 1 KB) · storage_key=tigris:csv-1'; + +type OrchestratorOptions = ConstructorParameters[0]; + +function options( + requests: LlmRequest[], + knowledgeGraph: InMemoryKnowledgeGraph, +): OrchestratorOptions { + return { + provider: recordingProvider(requests), + model: 'test', + maxTokens: 1024, + maxToolIterations: 3, + domainTools: [], + nativeToolRegistry: new NativeToolRegistry(), + attachmentReader: fakeAttachmentReader({ + 'tigris:csv-1': { bytes: CSV_BYTES, contentType: 'text/csv', fileName: 'data.csv' }, + }), + knowledgeGraph, + } as OrchestratorOptions; +} + +describe('#430 fixup — CSV dataset-import ACL identity resolution', () => { + it('resolves a channel turn (channelIdentity present) to the canonical omadiaUserId, NOT the raw channel-native id', async () => { + const requests: LlmRequest[] = []; + const graph = new InMemoryKnowledgeGraph(); + const orch = new Orchestrator(options(requests, graph)); + + // Pre-resolve what the canonical id WILL be, so we can assert against it + // without depending on internal id-generation details. + const expected = await graph.resolveOrCreateChannelIdentity({ + channelKind: 'teams', + channelUserId: 'aad-oid-123', + }); + + await orch.runTurn({ + userMessage: CSV_MANIFEST, + sessionScope: 'sess-1', + userId: 'aad-oid-123', // raw channel-native id, as the real dispatcher sets it + channelIdentity: { channelKind: 'teams', channelUserId: 'aad-oid-123' }, + }); + + const owned = await graph.listDatasets({ ownerOmadiaUserId: expected.omadiaUserId }); + assert.equal(owned.length, 1, 'the dataset must be owned by the RESOLVED omadiaUserId'); + + const wronglyOwned = await graph.listDatasets({ ownerOmadiaUserId: 'aad-oid-123' }); + assert.equal( + wronglyOwned.length, + 0, + 'the raw channel-native id must NOT own the dataset (the bug this fixup closes)', + ); + }); + + it('uses input.userId as-is for an HTTP/CLI turn (no channelIdentity — userId already IS canonical)', async () => { + const requests: LlmRequest[] = []; + const graph = new InMemoryKnowledgeGraph(); + const orch = new Orchestrator(options(requests, graph)); + + await orch.runTurn({ + userMessage: CSV_MANIFEST, + sessionScope: 'sess-1', + userId: 'a1b2c3d4-0000-0000-0000-000000000001', // already-canonical uuid, e.g. from req.session.omadia_user_id + }); + + const owned = await graph.listDatasets({ + ownerOmadiaUserId: 'a1b2c3d4-0000-0000-0000-000000000001', + }); + assert.equal(owned.length, 1); + }); + + it('does not import as a dataset (falls through to plain text) when channelIdentity is absent AND userId is absent', async () => { + const requests: LlmRequest[] = []; + const graph = new InMemoryKnowledgeGraph(); + const orch = new Orchestrator(options(requests, graph)); + + await orch.runTurn({ userMessage: CSV_MANIFEST, sessionScope: 'sess-1' }); + + const stats = await graph.stats(); + assert.equal(stats.byNodeType['PluginEntity'] ?? 0, 0, 'no Dataset node should have been created'); + }); +}); diff --git a/middleware/test/orchestratorDispatcher.test.ts b/middleware/test/orchestratorDispatcher.test.ts index a95ce261..3be05e64 100644 --- a/middleware/test/orchestratorDispatcher.test.ts +++ b/middleware/test/orchestratorDispatcher.test.ts @@ -254,4 +254,63 @@ describe('createOrchestratorDispatcher', () => { assert.equal(bindingConsulted, false); assert.deepEqual(asked, ['chatAgent']); }); + + // ── #430 fixup — channelIdentity threading ───────────────────────────── + + it('threads a resolvable channelIdentity for a teams-aad userRef', async () => { + const seen: unknown[] = []; + const dispatcher = createOrchestratorDispatcher({ + getChannelBlock: () => undefined, + getAgentBundle: () => ({ + agent: { + chat: () => Promise.resolve({ text: '' }), + async *chatStream(input) { + seen.push(input); + await Promise.resolve(); + yield { type: 'done', answer: 'ok', toolCalls: 0, iterations: 1 } as ChatStreamEvent; + }, + }, + }), + }); + await collect( + dispatcher.streamTurn({ + ...turn, + userRef: { kind: 'teams-aad', id: 'aad-oid-123' }, + channelId: 'de.byte5.channel.teams', + }), + ); + const input = seen[0] as { + userId?: string; + channelIdentity?: { channelKind: string; channelUserId: string }; + }; + // `userId` stays the raw channel-native id (unchanged, documented + // behaviour) — `channelIdentity` is the NEW, typed, resolvable signal. + assert.equal(input.userId, 'aad-oid-123'); + assert.deepEqual(input.channelIdentity, { + channelKind: 'teams', + channelUserId: 'aad-oid-123', + }); + }); + + it('omits channelIdentity for a userRef kind the KG ChannelKind model has no mapping for (custom)', async () => { + const seen: unknown[] = []; + const dispatcher = createOrchestratorDispatcher({ + getChannelBlock: () => undefined, + getAgentBundle: () => ({ + agent: { + chat: () => Promise.resolve({ text: '' }), + async *chatStream(input) { + seen.push(input); + await Promise.resolve(); + yield { type: 'done', answer: 'ok', toolCalls: 0, iterations: 1 } as ChatStreamEvent; + }, + }, + }), + }); + // The shared `turn` fixture uses `kind: 'custom'` (e.g. the canvas + // channel) — no ChannelKind counterpart, so no identity is guessed. + await collect(dispatcher.streamTurn({ ...turn, channelId: 'de.byte5.channel.omadia-ui' })); + const input = seen[0] as { channelIdentity?: unknown }; + assert.equal(input.channelIdentity, undefined); + }); }); diff --git a/middleware/test/queryDatasetTool.test.ts b/middleware/test/queryDatasetTool.test.ts new file mode 100644 index 00000000..b6e5116d --- /dev/null +++ b/middleware/test/queryDatasetTool.test.ts @@ -0,0 +1,210 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { InMemoryKnowledgeGraph } from '@omadia/knowledge-graph-inmemory'; + +// Imported from the SAME relative source path `queryDatasetTool.ts` itself +// uses (not the built `@omadia/orchestrator` package) — tsx loads test files +// straight from source, so importing the compiled package here would create +// a SECOND module instance with its own AsyncLocalStorage, and turnContext +// set in the test would never be visible inside the tool. +import { turnContext } from '../packages/harness-orchestrator/src/turnContext.js'; +import { resolveTurnOwnerIdentity } from '../packages/harness-orchestrator/src/resolveTurnOwnerIdentity.js'; +import { QueryDatasetTool } from '../packages/harness-orchestrator/src/tools/queryDatasetTool.js'; + +// #430 fixup (reviewer round 5) — `QueryDatasetTool` now reads +// `resolvedOmadiaUserId`, not the raw `userId`. These existing tests treat +// the two as equal (HTTP/CLI-turn shape: no `channelIdentity`, so +// `resolvedOmadiaUserId` === `userId` by `resolveTurnOwnerIdentity`'s +// fallback rule) — see the dedicated channel-turn test below for the case +// where they diverge. +function asUser(userId: string, fn: () => Promise): Promise { + return turnContext.run( + { turnId: 't', turnDate: '2026-01-01', userId, resolvedOmadiaUserId: userId }, + fn, + ); +} + +describe('QueryDatasetTool', () => { + it('returns an error string (not a throw) when no user identity is resolved', async () => { + const graph = new InMemoryKnowledgeGraph(); + const tool = new QueryDatasetTool(graph); + const out = await tool.handle({ query: 'list_datasets' }); + assert.match(out, /Error:.*user identity/); + }); + + it('list_datasets, get_schema, and query_rows round-trip for the owning user', async () => { + const graph = new InMemoryKnowledgeGraph(); + const { datasetId } = await graph.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Sales', + sourceFileName: 'sales.csv', + columns: [ + { name: 'region', type: 'string' }, + { name: 'amount', type: 'number' }, + ], + rows: [ + { region: 'North', amount: 100 }, + { region: 'South', amount: 250 }, + ], + }); + const tool = new QueryDatasetTool(graph); + + const listed = await asUser('user-1', () => tool.handle({ query: 'list_datasets' })); + const listedJson = JSON.parse(listed) as { datasets: Array<{ id: string }> }; + assert.equal(listedJson.datasets.length, 1); + assert.equal(listedJson.datasets[0]?.id, datasetId); + + const schema = await asUser('user-1', () => + tool.handle({ query: 'get_schema', dataset_id: datasetId }), + ); + const schemaJson = JSON.parse(schema) as { columns: Array<{ name: string }> }; + assert.deepEqual( + schemaJson.columns.map((c) => c.name), + ['region', 'amount'], + ); + + const rows = await asUser('user-1', () => + tool.handle({ + query: 'query_rows', + dataset_id: datasetId, + filters: [{ column: 'region', op: 'eq', value: 'North' }], + }), + ); + const rowsJson = JSON.parse(rows) as { totalMatched: number }; + assert.equal(rowsJson.totalMatched, 1); + }); + + it('never leaks existence of another user\'s dataset', async () => { + const graph = new InMemoryKnowledgeGraph(); + const { datasetId } = await graph.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'Secret', + sourceFileName: 's.csv', + columns: [{ name: 'v', type: 'number' }], + rows: [{ v: 1 }], + }); + const tool = new QueryDatasetTool(graph); + const out = await asUser('user-2', () => + tool.handle({ query: 'get_schema', dataset_id: datasetId }), + ); + assert.deepEqual(JSON.parse(out), { error: 'not_found_or_not_owned' }); + }); + + // #430 fixup (reviewer round 5) — the bug this closes: a channel turn + // (Teams/Slack/Telegram) imports a dataset under the CANONICAL + // `omadiaUserId`, but `query_dataset` used to read the RAW channel-native + // id from `turnContext.current()?.userId` — those never match, so the + // exact user/channel that just imported a dataset could never find it + // again. Uses the SAME production resolution helper + // (`resolveTurnOwnerIdentity`) the orchestrator now calls once per turn, + // and builds the turnContext the same shape a real channel turn gets + // (`userId` = raw channel-native id, `resolvedOmadiaUserId` = the + // resolved canonical uuid) — not a hand-picked value that would pass even + // if the real wiring were broken. Imports directly via + // `KnowledgeGraph.ingestDataset` (full CSV-attachment wiring is covered + // by `orchestratorCsvDatasetIdentity.test.ts`) since `resolveOrCreate + // ChannelIdentity` is documented idempotent — a real import turn through + // `ingestAttachments` would resolve to the exact same `omadiaUserId`. + it('finds a dataset imported by a channel turn when queried by the SAME channel turn', async () => { + const graph = new InMemoryKnowledgeGraph(); + const rawChannelUserId = 'aad-oid-channel-1'; + const channelIdentity = { channelKind: 'teams' as const, channelUserId: rawChannelUserId }; + + // What the orchestrator's per-turn resolution computes at turn start — + // this is the identical call `runTurn`/`chatStream` make. + const resolvedOmadiaUserId = await resolveTurnOwnerIdentity(graph, { + userId: rawChannelUserId, + channelIdentity, + }); + assert.ok(resolvedOmadiaUserId, 'channel identity must resolve to a canonical id'); + assert.notEqual( + resolvedOmadiaUserId, + rawChannelUserId, + 'the resolved id must NOT be the raw channel-native id', + ); + + // The import path: writes ownership under the CANONICAL id (mirrors + // `ingestAttachments` after the #430 fixup). + const { datasetId } = await graph.ingestDataset({ + ownerOmadiaUserId: resolvedOmadiaUserId as string, + name: 'Channel import', + sourceFileName: 'data.csv', + columns: [{ name: 'v', type: 'number' }], + rows: [{ v: 1 }], + }); + + const tool = new QueryDatasetTool(graph); + + // The query path, from the SAME channel turn: turnContext carries both + // the raw `userId` AND the resolved `resolvedOmadiaUserId`, exactly as + // `runTurn`/`chatStream` now populate it. + const listed = await turnContext.run( + { + turnId: 't-channel', + turnDate: '2026-01-01', + userId: rawChannelUserId, + resolvedOmadiaUserId, + }, + () => tool.handle({ query: 'list_datasets' }), + ); + const listedJson = JSON.parse(listed) as { datasets: Array<{ id: string }> }; + assert.equal( + listedJson.datasets.length, + 1, + 'the channel user must find the dataset THEY just imported', + ); + assert.equal(listedJson.datasets[0]?.id, datasetId); + + const schema = await turnContext.run( + { + turnId: 't-channel', + turnDate: '2026-01-01', + userId: rawChannelUserId, + resolvedOmadiaUserId, + }, + () => tool.handle({ query: 'get_schema', dataset_id: datasetId }), + ); + assert.notDeepEqual(JSON.parse(schema), { error: 'not_found_or_not_owned' }); + + // Regression guard for the exact bug this closes: if the tool were + // still reading the raw `userId` (pre-fixup behaviour), it would list + // the dataset under the WRONG (raw) id. Prove ownership is keyed to the + // canonical id only — the raw id owns nothing in the graph directly. + const ownedByRawId = await graph.listDatasets({ ownerOmadiaUserId: rawChannelUserId }); + assert.equal( + ownedByRawId.length, + 0, + 'the raw channel-native id must never itself own the dataset', + ); + + // And a turn that only has the raw id (no `resolvedOmadiaUserId` — + // resolution failed/unavailable) is correctly treated as "no identity", + // not silently allowed through with the wrong id. + const noResolvedId = await turnContext.run( + { turnId: 't-raw', turnDate: '2026-01-01', userId: rawChannelUserId }, + () => tool.handle({ query: 'list_datasets' }), + ); + assert.match(noResolvedId, /Error:.*user identity/); + }); + + it('surfaces a validation error for an unknown column without throwing', async () => { + const graph = new InMemoryKnowledgeGraph(); + const { datasetId } = await graph.ingestDataset({ + ownerOmadiaUserId: 'user-1', + name: 'D', + sourceFileName: 'd.csv', + columns: [{ name: 'v', type: 'number' }], + rows: [{ v: 1 }], + }); + const tool = new QueryDatasetTool(graph); + const out = await asUser('user-1', () => + tool.handle({ + query: 'query_rows', + dataset_id: datasetId, + filters: [{ column: 'nope', op: 'eq', value: 1 }], + }), + ); + assert.match(out, /Error:.*unknown_filter_column/); + }); +});