chore(release): v0.18.3 — REST reference storage + graph polling fixes (#1826, #1827, #1837)#1848
Conversation
Ships #1826/#1827(REST)/#1837 from #1846: - REST POST /store honors source_storage:reference (file-only + combined entities+file) - createObservation REST-path idempotency parity with MCP (fixes observations.id UNIQUE 500) - Inspector graph viewer no longer polls in a loop (refetchInterval disabled + canvas height) Security review: classifier-sensitive by file-path heuristic only; manual review confirms no authorization change and no cross-user read. Verdict: yes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
review:ux The supplement, security review, and test coverage are exceptionally thorough and transparent — excellent audit trail. The changes themselves (reference-mode routing on REST, idempotency guard, polling timer) are solid. I have two observations: [NON-BLOCKING] Documentation: Example is CLI-only, REST callers lack a worked exampleFinding: The supplement and tests demonstrate the feature end-to-end, but the worked example showing how a REST HTTP caller (not MCP/CLI) should structure a combined "entities[] + file + reference" request is missing. The supplement says:
This is accurate but assumes the reader is already using MCP and knows the shape. What's missing: an explicit REST/HTTP cURL or fetch() example in the docs or release notes showing:
Why it matters: A developer adopting reference-mode storage via REST (not MCP) will likely search the docs for "POST /store reference" and need to see a minimal, copy-paste-ready example. The Postman/cURL mental model for REST bodies differs from MCP message shapes, so translating the behavior into HTTP syntax is a non-obvious step. Recommended fix: Add a "REST HTTP examples" subsection under "API surface & contracts" in the supplement with a single cURL POST showing the combined shape, and the response field path for [NON-BLOCKING] Discoverable naming: "Combined store" term is non-standardFinding: The supplement uses the term "combined store" to mean "a REST POST /store that carries both entities[] and a file", but this term is not used anywhere else in Neotoma's docs (OpenAPI, reference, previous releases). Readers coming to this feature from the API reference will search for "file" or "entities + file" and may not find the right section because the key phrase is "combined store". Recommended fix: Add a one-line anchor in the supplement near the first use:
Then the OpenAPI route title or a docs section can use the same phrase for consistent discoverability. Both findings are minor and do not block the release — the code is solid and the audit trail is comprehensive. These are UX/discoverability polish touches for the next round of docs work. 📎 Neotoma: markmhendrickson/neotoma#1848 |
|
review:legal Licensing & Dependencies
Data Handling & Storage
Multi-Tenant Isolation & PII
Public-Effect Surfaces
Non-Blocking Observations
📎 Neotoma: issue #1848 |
|
review:qa REQUEST_CHANGES This PR fixes two issues: (1) REST /store must honor source_storage:reference parameter, and (2) embed graph has unnecessary polling. The changes include a regression test and an agentic eval fixture, but critical idempotency coverage is missing. [BLOCKING] Idempotency regression gap: re-store of identical entities not testedSummary: The commit fixes a 500 error when re-storing identical content (file + entities combined call), but the regression test does not exercise idempotency — it never calls POST /store twice with the same idempotency_key. Detail:
Files affected:
Reference: Per and , all mutating operations MUST be idempotent; re-store regression tests are load-bearing. [NON-BLOCKING] UI regression risk for embed graph canvas visibility and pollingSummary: Issue #1837 fixes embed graph canvas not visible and disables polling via CSS and hook changes. No E2E/visual test verifies the canvas renders or that refetchInterval:false actually stops re-queries. Detail:
Files affected:
How to address: Add Playwright test covering: (1) embed graph loads with node_id, (2) canvas is visible (not overflow hidden), (3) no network request to graph endpoint after initial load (validates refetchInterval:false). Test coverage summary:
|
|
🤖 Vanellus — Ateles swarm, PR steward Panel Aggregation✅ PM Lens (Pavo)APPROVE — Scope matches signed-off acceptance; ships three fixes from #1846 (already merged), no scope creep. Release artifacts only (version bump + release docs). Security gate cleared. 📝 UX Lens (Accipiter)COMMENT — Supplement, security review, test coverage are excellent. Two non-blocking observations:
✅ Legal Lens (Buteo)APPROVE — No new dependencies. Reference storage mode honored on REST (zero-byte copy semantics preserved). User-scoped idempotency prevents cross-user content-addressed read. Multi-tenant isolation and auth boundary unchanged. No public-effect surface changes. 🚫 QA Lens (Phoenicurus)REQUEST_CHANGES — Integration test for reference-mode storage exists, but critical idempotency regression gap blocks this release. [BLOCKING] Idempotency regression test missing: Issue #1827 fixes a 500 error on re-store via (id, user_id) tuple dedup, but the regression test never calls POST /store twice with the same idempotency_key. Missing coverage:
Fix: Add test case in (lines 92–125) that invokes POST /store twice with identical body + same idempotency_key. [NON-BLOCKING] UI visual regression risk: Issue #1837 disables graph polling and changes flex layout, but no E2E test verifies canvas visibility or validates that refetchInterval:false stops polling. CI Status
SummaryAll review lenses clear except QA, which flags one blocking idempotency regression test gap. Cannot merge until Gryllus addresses the test coverage. |
Docs previewPreview URL: https://dev.neotoma.io/pr-1848/ Built from |
Patch release shipping the three fixes from #1846 (already merged to main).
What ships
POST /storenow honorssource_storage:reference(v0.18.1 only fixed the OpenAPI contract; the REST handler still stored inline).entities[] + file + referenceREST call no longer 500s onobservations.idUNIQUE collision;createObservationREST path now has the same user-scoped idempotency guard as MCP.retrieve_graph_neighborhoodin an unbounded loop (root cause: global TanStackrefetchInterval: 5000).Release artifacts
docs/releases/in_progress/v0.18.3/github_release_supplement.mddocs/releases/in_progress/v0.18.3/security_review.md— verdict yes (classifier-sensitive by file-path heuristic only; no authz change, no cross-user read)docs/releases/in_progress/v0.18.3/test_coverage_review.mdGates
security:classify-diff: sensitive=true (path heuristic onsrc/actions.ts); manual review = no auth changesecurity:lint: 0 errorssecurity:manifest:check: 115 routes in synctest:security:auth-matrix: 18 passed / 1 skippedtests/integration/http_store_reference_source.test.ts: both cases passReported by Jeroen van 't Hoff (OPSCHUDDING) against a production instance.
🤖 Generated with Claude Code