feat(nodes,ai): RocketRide cloud DB nodes — sql, vector, graph - #1713
feat(nodes,ai): RocketRide cloud DB nodes — sql, vector, graph#1713dylan-savage wants to merge 15 commits into
Conversation
…rocketride_vector Add the first two RocketRide-branded cloud database nodes. Both connect only to the RocketRide cloud data-core and take no connection config: instead of host/user/password they resolve a ready per-tenant DSN from the account layer, keyed by the authenticated client_id. - Account.resolve_db_dsn(client_id) contract on AccountBase; OSS stub raises 'RocketRide cloud DB nodes require signing into RocketRide cloud'. Real SaaS resolver is out of scope (extension.saas). - ai.common.rocketride_db: the shared seam all RR DB nodes use — client_id from ROCKETRIDE_CLIENT_ID (tool_filesystem precedent), asyncio.run bridge from sync node lifecycle, DSN normalisation/parsing. Tests inject a fake resolver via the ai.account singleton; node code is resolver-agnostic. - rocketride_sql: mirrors db_postgres on DatabaseGlobalBase/InstanceBase; overrides only connection resolution. Structured query surface, EXPLAIN validation, and the raw EXECUTE path (allow_execute, default off) inherited. - rocketride_vector: mirrors vectordb_postgres (node-local Store on DocumentStoreBase); adds the missing default index — HNSW created at first write with operator class derived from the similarity config (cosine/l2/inner_product -> vector_cosine_ops/vector_l2_ops/vector_ip_ops), m=16 / ef_construction=64 (config-overridable), skipped with a warning when vector_size > 2000. No execute path by design. - Co-located READMEs with generated-params markers; unit tests (25) run in the nodes:test gate, integration tests (test_rocketride_db_full.py, 9) run against a local PG16+pgvector container and verify end-to-end connect, gated execute, and that semantic search plans an HNSW index scan. Gate: ./builder nodes:test — 1690 passed, 48 skipped, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The integrations/neo4j page still linked /nodes/db_neo4j, which broke docs:build (Docusaurus broken-link check) after the node was renamed to graph_neo4j. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…+ rocketride_graph
Apache AGE cannot run bare Cypher, so ship the translation layer the design
mandates at ai/common/graph/age/ and the third cloud DB node on top of it.
Translation layer (pure transform, no DB access):
- analysis: openCypher ANTLR parse (vendored M23 grammar, parser generated
at ANTLR 4.13.2 and committed — no Java needed by developers). Extracts
RETURN projection/aliases, write clauses as typed contexts, $params,
variable-length depth bounds, invoked functions.
- firewall: resource caps on BOTH paths (query length, var-length depth cap,
unbounded-* rejection, per-txn statement_timeout); semantic read-only rules
(no writes / no CALL) on the safe path only.
- capabilities: dialect table keyed by AGE version. Verified 1.5.0 REJECT
cells with actionable messages: datetime(), RETURN *, ORDER BY <alias>
('could not find rte'). Unverified cells (merge_on_set, where_label_check,
multi_label, shortest_path) ship as TBD and pass through per the design.
- emit: cypher() envelope with collision-proof dollar-quoting, synthesized
'AS (c0 agtype, …)' column list, SET LOCAL search_path/statement_timeout
preamble (transaction-pooler safe), and PREPARE/EXECUTE/DEALLOCATE binding
(AGE rejects inline ::agtype literals as the params argument — verified).
- decode: agtype -> plain Python via the vendored apache/age driver parser
(upstream master @ 5a254d68, Apache-2.0; regenerated at 4.13.2; strict
error listener instead of upstream's silent error recovery). The stale
PyPI apache-age-python (0.0.7, hard-pins antlr 4.11.1) is deliberately
NOT a dependency.
- errors: AgeTranslationError / AgeUnsupportedFeature / AgeFirewallRejected,
all fail loud into the LLM repair loop.
- deps: antlr4-python3-runtime>=4.13.2,<4.14 via ai/common requirements;
vendored dirs excluded from ruff (force-exclude so lefthook's explicit
staged-file invocation honors it).
rocketride_graph node:
- GraphGlobalBase/InstanceBase subclass mirroring graph_neo4j; connection is
the shared resolve_db_dsn seam (no connection fields). Safe reads run in a
SET TRANSACTION READ ONLY transaction (server-side write block, verified);
_validate_query EXPLAINs the translated envelope so Cypher syntax errors
surface without executing; schema reflection = ag_catalog labels + bounded
property/edge sampling; raw EXECUTE stays behind allow_execute and still
obeys resource caps. Fails fast when the configured graph does not exist —
create_graph ownership is an open question and the node does not decide it.
Tests: 38 layer unit tests (offline) + 15 node integration tests against a
container on the live pin (PG16 + AGE 1.5.0 + pgvector 0.8.0), covering
params round-trip, read-only enforcement, limit+1 truncation contract,
EXECUTE gating, EXPLAIN validation, and schema reflection.
Gate: ./builder nodes:test — 1730 passed, 0 failed. builder docs:build green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r + env injection
Implements our half of the data-core contract locked with cloud/ops
(2026-07-23): POST /provision {tenant_id} -> {db_name, dsn}, idempotent,
password derived server-side, graph pre-created.
- AccountBase.resolve_db_dsn default implementation: env-gated call to the
data-core provisioner (ROCKETRIDE_DB_BROKER_URL + ROCKETRIDE_DB_BROKER_TOKEN,
Bearer auth, client_id passed verbatim as tenant_id, stdlib urllib in a
worker thread — no new dependency). Unset env raises the same cloud-sign-in
error as before, so OSS behavior is unchanged and the OSS subclass override
is removed as redundant. Consequence: the extension.saas overlay needs NO
code for these nodes — a cloud deployment sets two env vars.
- task_engine: resolve the DSN server-side at task start (the only process
with SaaS account context — --saas never reaches node subprocesses) and
inject ROCKETRIDE_DB_DSN into the node env alongside ROCKETRIDE_CLIENT_ID.
Scoped to pipelines containing rocketride_sql/vector/graph so unrelated
tasks never trigger provisioning; resolution failure is non-fatal (the node
surfaces its own clear error).
- rocketride_db seam: read the injected ROCKETRIDE_DB_DSN first; account
fallback unchanged. Stateless by design — the broker is idempotent with
derived passwords, so no caching, persistence, or rotation-invalidation
logic exists on this side; a rotated password flows through on the next
task start.
Tests: 10 broker tests against a real localhost fake /provision (Bearer,
verbatim tenant_id, idempotent recall, 4xx/timeout/missing-dsn); 3 pipeline-
scoping tests; seam env-first units; and an integration test running
rocketride_sql end-to-end against the live container via an injected env DSN
with the account poisoned — the exact production delivery path.
Gate: ./builder nodes:test — 1732 passed, 0 failed. ai tests (account + task
engine): 65 passed under the engine runtime.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Open-source builds can now use the three RocketRide cloud DB nodes by pasting a personal rr_ API key into the new cloud_api_key config field (password widget, all three nodes). The seam resolves credentials with a strict precedence — injected ROCKETRIDE_DB_DSN env, then the ambient account/broker path, then the config key exchanged at the cloud door (POST api.rocketride.ai/db/dsn, Bearer key, empty body: the door derives the tenant server-side), else a sign-in error naming the field. Ambient platform identity deliberately outranks the pasted key so a stray key in a hosted pipeline can never retarget writes to another tenant; broker failures propagate untouched rather than being masked by the key path. validateConfig surfaces the same guidance as a non-fatal warning (save time runs pre-injection); enforcement stays at task start. Door URL is env-overridable via ROCKETRIDE_DB_DOOR_URL for staging/tests. 11 new unit tests against a live localhost fake door (precedence, Bearer passing, 401/500/missing-dsn/unreachable); gate 1743 passed / 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lets the sql/vector integration suite run against a provisioned tenant DSN (t_<slug>_<hash> through a pooler) as well as the default local container. Verified against both: 10/10 on the default :55432 container and 10/10 against a real #381-provisioner tenant DSN via transaction-mode PgBouncer with TLS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#381 shape The provisioner shipped {database, role, dsn, created} (draft said db_name). Resolver behavior is unchanged — only dsn is ever read; this updates the two docstrings and the fake-broker/fake-door payloads to mirror the real response. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed on 1.5.0) Tested empirically against the exact pin container (PG 16.14 + AGE 1.5.0): MERGE...ON CREATE/MATCH SET, WHERE label predicates (both forms), multi-labels (n:A:B), and shortestPath() are all syntax-level failures, while plain MERGE on the same graph succeeds (baseline control). Capability behavior is a property of the AGE version, not the deployment, so the pin container is valid evidence. Each cell now rejects pre-flight with an actionable alternative instead of surfacing AGE's raw syntax error. Zero TBD cells remain in the 1.5.0 table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The client half (cloud_api_key field + door exchange) was built and tested, but the server half — the door route on api.rocketride.ai and external pooler exposure — has no owner yet, so shipping the field now would give OSS users a credential input that cannot work. Deferred until the server side is scheduled; the full implementation remains in branch history (8cf951b) to restore. Hosted-path behavior is unchanged: env-injected DSN, else broker via the signed-in account, else the sign-in error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
docs
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds tenant-scoped RocketRide SQL, AGE graph, and pgvector nodes. It introduces broker-backed DSN resolution, a Cypher-to-AGE translation layer, PostgreSQL-backed storage and execution, service registrations, documentation, CI database services, and unit/integration tests. ChangesRocketRide cloud database integrations
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/rocketride_graph/IGlobal.py`:
- Around line 290-304: Update _sample_properties to build the SELECT statement
with psycopg2.sql.SQL and Identifier rather than interpolating
self._qualified(label) into an f-string passed to execute(). Preserve the
existing properties column, LIMIT 1 behavior, cursor/transaction handling, and
result processing.
In `@nodes/src/nodes/rocketride_sql/__init__.py`:
- Around line 27-32: Update the dependency declaration used by the rocketride
SQL node so requirements.txt includes a pinned SQLAlchemy dependency alongside
psycopg2-binary==2.9.12, keeping the install comment and depends(requirements)
flow in __init__.py accurate.
In `@nodes/src/nodes/rocketride_sql/requirements.txt`:
- Line 1: Add sqlalchemy>=2.0 to the rocketride_sql requirements alongside
psycopg2-binary, ensuring DatabaseGlobalBase can import and use create_engine
during configuration validation and runtime queries.
In `@nodes/src/nodes/rocketride_vector/rocketride_vector.py`:
- Around line 157-166: Update the runtime connection in the initialization flow
around resolve_rocketride_dsn and psycopg2.connect to include a 3-second connect
timeout, matching IGlobal.validateConfig. Preserve the existing DSN and
connection setup behavior while ensuring unreachable poolers do not wait for the
OS TCP timeout.
- Around line 557-588: Update render() to size the text buffer from the fetched
results rather than renderChunkSize, avoiding allocation of a massive list for
sparse batches. Preserve chunkId-to-index placement and callback assembly, while
ensuring the buffer covers the highest fetched index needed by lastIndex.
- Around line 143-151: The HNSW configuration handling in rocketride_vector.py
must enforce pgvector’s valid bounds: clamp hnsw_m to a minimum of 2 and
hnsw_ef_construction to its supported minimum while preserving invalid-value
fallbacks. In nodes/src/nodes/rocketride_vector/services.json:117-124, raise
rrvector.hnsw_m.minimum to 2.
In `@packages/ai/src/ai/common/graph/age/_agtype/builder.py`:
- Around line 31-45: Update parseAgeValue to avoid reusing the module-global
resultHandler across calls; instantiate an independent Antlr4ResultHandler for
each parse so mutable lexer, parser, and token-stream state remains isolated
between threads. Preserve the existing None handling and AGTypeError wrapping,
and leave newResultHandler unchanged unless needed to remove the shared
instance.
- Around line 108-117: The _stripStringDelimiters helper currently removes
quotes without decoding Agtype/JSON-style escapes. Update
_stripStringDelimiters, used by visitStringValue, to unescape sequences such as
escaped quotes, newlines, carriage returns, and Unicode escapes while preserving
the existing removal of only the surrounding delimiters; ensure visitStringValue
returns the decoded plain Python string.
In `@packages/ai/src/ai/common/graph/age/emit.py`:
- Around line 116-118: Update the parameter-binding logic in the surrounding
Cypher emission flow: validate that every name in facts.param_names has a
supplied value, including when params is None or empty, and select the
prepared-statement cypher() path whenever facts.param_names is non-empty rather
than based on bool(params). Preserve the existing error for supplied parameters
when the query references no parameters.
In `@packages/ai/src/ai/common/rocketride_db.py`:
- Around line 150-159: Update the DSN parsing flow around the parsed field
accesses to preserve best-effort behavior for malformed authorities: safely
retrieve hostname, port, username, and password, treating any urllib.parse
validation errors as empty or None cosmetic values instead of allowing
exceptions to escape. Keep valid DSN parsing unchanged and ensure node
construction continues for invalid ports or malformed authority data.
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 1569-1587: The task subprocess environment currently inherits
broker credentials through subprocess_env. Update the RocketRide DB setup around
_pipeline_uses_rocketride_db and account.resolve_db_dsn so the parent resolves
and injects only ROCKETRIDE_DB_DSN, while explicitly removing
ROCKETRIDE_DB_BROKER_TOKEN and the broker URL variable from subprocess_env
before launching any child pipeline; preserve the existing non-fatal resolution
handling.
In `@packages/docs/content-static/integrations/neo4j.md`:
- Line 8: Update the Neo4j documentation examples to use the renamed graph_neo4j
node: change the JSON sample’s provider value to graph_neo4j and replace the old
provider/tool prefix in the tool examples with graph_neo4j., preserving the
surrounding example structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a72dcceb-8609-4add-9af6-990b10d3eb0c
⛔ Files ignored due to path filters (13)
nodes/src/nodes/rocketride_graph/rocketride_graph.svgis excluded by!**/*.svgnodes/src/nodes/rocketride_sql/rocketride_sql.svgis excluded by!**/*.svgnodes/src/nodes/rocketride_vector/rocketride_vector.svgis excluded by!**/*.svgpackages/ai/src/ai/common/graph/age/_agtype/gen/AgtypeLexer.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_agtype/gen/AgtypeListener.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_agtype/gen/AgtypeParser.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_agtype/gen/AgtypeVisitor.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_agtype/gen/__init__.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_cypher/gen/CypherLexer.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_cypher/gen/CypherListener.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_cypher/gen/CypherParser.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_cypher/gen/CypherVisitor.pyis excluded by!**/gen/**packages/ai/src/ai/common/graph/age/_cypher/gen/__init__.pyis excluded by!**/gen/**
📒 Files selected for processing (49)
nodes/src/nodes/rocketride_graph/IGlobal.pynodes/src/nodes/rocketride_graph/IInstance.pynodes/src/nodes/rocketride_graph/README.mdnodes/src/nodes/rocketride_graph/__init__.pynodes/src/nodes/rocketride_graph/requirements.txtnodes/src/nodes/rocketride_graph/services.jsonnodes/src/nodes/rocketride_sql/IGlobal.pynodes/src/nodes/rocketride_sql/IInstance.pynodes/src/nodes/rocketride_sql/README.mdnodes/src/nodes/rocketride_sql/__init__.pynodes/src/nodes/rocketride_sql/requirements.txtnodes/src/nodes/rocketride_sql/services.jsonnodes/src/nodes/rocketride_vector/IEndpoint.pynodes/src/nodes/rocketride_vector/IGlobal.pynodes/src/nodes/rocketride_vector/IInstance.pynodes/src/nodes/rocketride_vector/README.mdnodes/src/nodes/rocketride_vector/__init__.pynodes/src/nodes/rocketride_vector/requirements.txtnodes/src/nodes/rocketride_vector/rocketride_vector.pynodes/src/nodes/rocketride_vector/services.jsonnodes/test/test_age_translate.pynodes/test/test_rocketride_db.pynodes/test/test_rocketride_db_full.pynodes/test/test_rocketride_graph_full.pypackages/ai/src/ai/account/base.pypackages/ai/src/ai/account/oss/__init__.pypackages/ai/src/ai/common/graph/age/README.mdpackages/ai/src/ai/common/graph/age/__init__.pypackages/ai/src/ai/common/graph/age/_agtype/Agtype.g4packages/ai/src/ai/common/graph/age/_agtype/__init__.pypackages/ai/src/ai/common/graph/age/_agtype/builder.pypackages/ai/src/ai/common/graph/age/_agtype/exceptions.pypackages/ai/src/ai/common/graph/age/_agtype/models.pypackages/ai/src/ai/common/graph/age/_cypher/Cypher.g4packages/ai/src/ai/common/graph/age/_cypher/__init__.pypackages/ai/src/ai/common/graph/age/analysis.pypackages/ai/src/ai/common/graph/age/capabilities.pypackages/ai/src/ai/common/graph/age/decode.pypackages/ai/src/ai/common/graph/age/emit.pypackages/ai/src/ai/common/graph/age/errors.pypackages/ai/src/ai/common/graph/age/firewall.pypackages/ai/src/ai/common/graph/age/translate.pypackages/ai/src/ai/common/requirements.txtpackages/ai/src/ai/common/rocketride_db.pypackages/ai/src/ai/modules/task/task_engine.pypackages/ai/tests/ai/account/test_resolve_db_dsn.pypackages/ai/tests/ai/modules/task/test_task_engine.pypackages/docs/content-static/integrations/neo4j.mdpyproject.toml
- graph: build the label-sample SELECT with psycopg2.sql.Identifier (drop hand-rolled _qualified); bound runtime connect at 3s - vector: 3s connect_timeout on the runtime connection; size render() buffer from fetched rows, not the chunk window; clamp HNSW params to pgvector bounds (m in [2,100], ef_construction in [4,1000], >= 2*m) and raise services.json minimums to match - agtype decoder: decode JSON escapes in STRING tokens via json.loads (escaped quotes/newlines/unicode previously survived as literal backslash sequences); deviation itemized in the vendoring banner - emit: pre-flight error when the query references $parameters with no supplied values; key the PREPARE/EXECUTE branch on facts.param_names - rocketride_db: parse_dsn_fields degrades per-field on malformed authorities (invalid port, bad IPv6 bracket) instead of raising - task engine: scrub ROCKETRIDE_DB_BROKER_URL/_TOKEN from the node subprocess env — children only ever get the resolved DSN - sql node: correct the stale SQLAlchemy comment (dep comes from ai.common's own requirements, same as the other DB nodes) - docs: fix stale neo4jdb provider / tool prefix in neo4j integration Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-nodes # Conflicts: # packages/ai/tests/ai/modules/task/test_task_engine.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai/src/ai/modules/task/task_engine.py (1)
1569-1594: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winClear inherited
ROCKETRIDE_DB_DSNbefore launching the child.Removing only the broker variables is insufficient:
subprocess_envstarts fromos.environ, so every child can still inherit a parent-level DSN—including unrelated pipelines or the stale DSN when tenant resolution fails. RemoveROCKETRIDE_DB_DSNbefore the conditional and set it only after successful tenant-scoped resolution.Proposed fix
subprocess_env = os.environ.copy() subprocess_env['ROCKETRIDE_CLIENT_ID'] = self.client_id +subprocess_env.pop('ROCKETRIDE_DB_DSN', None) subprocess_env.pop('ROCKETRIDE_DB_BROKER_URL', None) subprocess_env.pop('ROCKETRIDE_DB_BROKER_TOKEN', None)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/src/ai/modules/task/task_engine.py` around lines 1569 - 1594, Clear ROCKETRIDE_DB_DSN from subprocess_env before the _pipeline_uses_rocketride_db() conditional, alongside the broker credential removals. Leave it absent when the pipeline does not use RocketRide DB nodes or tenant-scoped resolve_db_dsn fails, and set it only after successful resolution.
♻️ Duplicate comments (1)
nodes/src/nodes/rocketride_vector/rocketride_vector.py (1)
574-586: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winAvoid allocating by the highest sparse chunk ID.
Line 582 can still allocate ~33.5 million entries when one fetched row has
chunkIdnear the window end. Store fetched content by index and join sorted values instead.Proposed fix
- lastIndex = max(index for index, _ in rows) - text = [''] * (lastIndex + 1) - for index, content in rows: - text[index] = content - - fullText = ''.join(text) + content_by_index = {index: content for index, content in rows} + fullText = ''.join( + content_by_index[index] for index in sorted(content_by_index) + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/rocketride_vector/rocketride_vector.py` around lines 574 - 586, Update the row assembly in the retrieval flow around rows, lastIndex, and fullText so it does not allocate a list sized by the highest sparse index. Store fetched content keyed by its nonnegative index, order entries by index, and join the sorted content values while preserving the existing empty-results break behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/docs/content-static/integrations/neo4j.md`:
- Around line 103-104: Update the tool-prefix example in the connected-agent
documentation to use the configured node ID, such as `graph_1.get_data` or
`<node-id>.get_data`, instead of the provider ID `graph_neo4j.get_data`; clarify
that bare tool methods are automatically namespaced by the node ID.
---
Outside diff comments:
In `@packages/ai/src/ai/modules/task/task_engine.py`:
- Around line 1569-1594: Clear ROCKETRIDE_DB_DSN from subprocess_env before the
_pipeline_uses_rocketride_db() conditional, alongside the broker credential
removals. Leave it absent when the pipeline does not use RocketRide DB nodes or
tenant-scoped resolve_db_dsn fails, and set it only after successful resolution.
---
Duplicate comments:
In `@nodes/src/nodes/rocketride_vector/rocketride_vector.py`:
- Around line 574-586: Update the row assembly in the retrieval flow around
rows, lastIndex, and fullText so it does not allocate a list sized by the
highest sparse index. Store fetched content keyed by its nonnegative index,
order entries by index, and join the sorted content values while preserving the
existing empty-results break behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b7414c78-ae00-493c-91b6-979cb8b02676
📒 Files selected for processing (12)
nodes/src/nodes/rocketride_graph/IGlobal.pynodes/src/nodes/rocketride_sql/__init__.pynodes/src/nodes/rocketride_vector/rocketride_vector.pynodes/src/nodes/rocketride_vector/services.jsonnodes/test/test_age_translate.pynodes/test/test_rocketride_db.pypackages/ai/src/ai/common/graph/age/_agtype/__init__.pypackages/ai/src/ai/common/graph/age/_agtype/builder.pypackages/ai/src/ai/common/graph/age/emit.pypackages/ai/src/ai/common/rocketride_db.pypackages/ai/src/ai/modules/task/task_engine.pypackages/docs/content-static/integrations/neo4j.md
asclearuc
left a comment
asclearuc
left a comment
There was a problem hiding this comment.
Thanks @dylan-savage — this is careful, well-documented work, and the review guide in the description made a 21k-line diff genuinely reviewable. The DSN seam is small and in the right place, the broker tests run a real HTTP server instead of a mock, and vendoring the openCypher parser with full provenance rather than depending on the stale apache-age-python is the right call. The capability table with every 1.5.0 cell empirically verified is above the bar.
I am requesting changes on five points.
1. RecursionError escapes the layer's error taxonomy. Running this branch, RETURN with 100 nested parentheses — a 208-character query, 2% of the 10,000-char cap — raises a bare RecursionError. _validate_query catches only AgeTranslationError, so it propagates out of the node and the repair loop never sees an actionable message.
2. max_query_length is enforced after the parse it is meant to bound. translate() calls analyze() first, so the cap does not protect the parser. Measured: 80 KB -> 2.2 s, 320 KB -> 9.4 s, 800 KB -> 22.6 s of CPU before rejection, all before the database is contacted. The execute agent tool passes its raw argument straight through. Moving the len() check ahead of analyze() fixes it.
3. rocketride_vector/IGlobal.validateConfig dropped the depends() call that vectordb_postgres and 60+ other nodes make before import psycopg2. In CONFIG mode nothing else installs it, so save-time validation degrades to a ModuleNotFoundError warning and the real checks silently never run.
4. Test gaps. render() was rewritten in 06ab8c5 with no test exercising it. The broker-credential scrubbing in task_engine — a security control — has no test; only _pipeline_uses_rocketride_db is covered. And all 25 integration tests skip in CI because no PG+AGE+pgvector service is started; the MinIO (_build.yaml:156) and Azurite (_build.yaml:190) blocks are the precedent for wiring one up. The suite you wrote is good — CI should be running it.
5. I agree with CodeRabbit's outside-diff Major on task_engine.py:1944: ROCKETRIDE_DB_DSN is never cleared from the inherited environment, so a parent-level value survives into unrelated pipelines and into the broker-failure path.
One CodeRabbit finding I think you should push back on: the _agtype/builder.py thread-safety Major does not apply — parseAgeValue is only re-exported, never called, since decode.py constructs a fresh ResultVisitor per call. Keeping the vendored file verbatim is correct.
Non-blocking, but worth a look: rocketride_vector.py is 622 lines of which only 125 differ from vectordb_postgres.py, and IInstance.py / IEndpoint.py are byte-identical copies. rocketride_sql shows the better shape at 88 lines. The smaller items are inline below.
| NotImplementedError: broker environment not configured. | ||
| RuntimeError: the broker rejected the request or returned no DSN. | ||
| """ | ||
| broker_url = os.environ.get('ROCKETRIDE_DB_BROKER_URL', '').strip() |
There was a problem hiding this comment.
Nothing constrains the scheme of ROCKETRIDE_DB_BROKER_URL: it goes straight to urllib.request.urlopen, which happily accepts http://, and also file:// and ftp://. Over plain HTTP, the Bearer token — which by design can resolve any tenant's DSN, as the scrubbing comment in task_engine states — and the returned DSN with the tenant's password both travel in the clear; with a file:// value the call silently turns into a local file read.
This is server-side config, so the realistic scenario is a deployment mistake rather than an attacker, but the PR already treats that token as the most sensitive secret in the system (it's scrubbed out of every node subprocess), and the guard is one line: require https:// (with an explicit escape hatch for local dev if you need one) and reject anything else before the request is built.
There was a problem hiding this comment.
Fixed in 6d0737c: _check_broker_url() requires https before the request is built; plain http is allowed only for localhost/127.0.0.1 (the local rig). file://, ftp://, and remote http all raise. Tested.
| except Exception as e: | ||
| raise RuntimeError(f'DB broker unreachable: {e}') from e | ||
|
|
||
| dsn = body.get('dsn') if isinstance(body, dict) else None |
There was a problem hiding this comment.
The DSN is accepted on isinstance(str) alone and then handed straight to psycopg2.connect / create_engine. The example in the docstring above carries ?sslmode=require, but nothing checks that the DSN actually does — so if the broker ever omits it, every connection to the tenant database goes out without TLS, carrying the tenant's user and password, and nothing in the pipeline surfaces that it happened.
Worth validating the scheme is postgres:///postgresql:// and that sslmode is present (or appending sslmode=require when it isn't)? It's the same class of cheap defence as the connect_timeout the nodes already set, and it keeps a broker-side regression from silently downgrading transport security for every tenant.
There was a problem hiding this comment.
Fixed in 6d0737c: _pin_dsn_tls() validates the scheme is postgres/postgresql and appends sslmode=require when the query string lacks it — a broker regression can no longer silently downgrade transport. Tested (append, passthrough, bad-scheme).
| try: | ||
| with self.client.cursor() as cur: | ||
| if plan.read_only: | ||
| cur.execute('SET TRANSACTION READ ONLY') |
There was a problem hiding this comment.
Both server-side controls on this path rest on an assumption nothing checks. SET TRANSACTION READ ONLY only applies when it is the first statement of an open transaction, and the SET LOCAL statement_timeout that emit() prepends only applies inside one. Today both hold because the connection is left with autocommit = False, so psycopg2 opens the transaction implicitly on this very execute.
The failure mode is what worries me: PostgreSQL accepts both statements outside a transaction without an error — SET TRANSACTION READ ONLY becomes a no-op and SET LOCAL only emits a warning. So if that connection ever becomes autocommit (a later change here, or a pooler handing it back that way), the write protection and the timeout cap both vanish silently, and the suite wouldn't notice: test_write_rejected_before_reaching_db asserts the rejection in the translation layer, not at the database. Two of the firewall's three caps hang off an unverified precondition.
Cheap ways to close it, whichever you prefer: set readonly at the session level (self.client.set_session(readonly=True)) so it doesn't depend on statement ordering, assert self.client.autocommit is False on entry here, or add a test that reads back SHOW transaction_read_only inside a plan so a regression fails loudly.
There was a problem hiding this comment.
Fixed in 6d0737c: _execute_plan now asserts autocommit is False up front (loud failure instead of the silent no-op), and a new integration test runs SHOW transaction_read_only through a safe plan and asserts on — executing in CI as of this PR.
| return False | ||
|
|
||
|
|
||
| pytestmark = pytest.mark.skipif(not _db_reachable(), reason=f'RocketRide AGE test database not reachable at {TEST_DSN}') |
There was a problem hiding this comment.
This gate means the integration suites never run in CI. RR_TEST_AGE_DSN appears in no workflow and no task in packages/server/scripts/tasks.js, and the PR adds no service container, so nothing is listening on localhost:55433 on a runner — both test_rocketride_graph_full.py and test_rocketride_db_full.py (886 lines between them) skip wholesale, and the lane goes green.
What gets skipped is the part I'd most want a runner to prove: test_write_rejected_before_reaching_db, test_execute_gated_by_allow_execute, test_execute_still_enforces_resource_caps, the limit contract, the parameter round-trip, the agtype decode. The unit suites that do run cover translation against fakes, so as things stand no safety control of the graph node is exercised against a real AGE instance before merge.
Could the PR bring its own runner coverage — a services: block with the pinned PG16+AGE 1.5.0 image (the one your module docstring documents) plus RR_TEST_AGE_DSN in the job env, or a separate job if you'd rather keep it off the main lane? The suites are already written and pass locally; it's the wiring that's missing, and without it a regression in the firewall or the READ ONLY path merges silently.
There was a problem hiding this comment.
Fixed in 6d0737c: the Ubuntu build job starts pgvector/pgvector:pg16 + apache/age:release_PG16_1.5.0 (MinIO pattern), exports both DSNs, and sets RR_REQUIRE_DB_TESTS=1, which turns any skip in the two suites into a hard failure. Verified in this PR's own CI run: both extensions created, zero unreachable-skips, all 26 tests executed.
…ne, TLS guards, CI coverage Review round 2 (asclearuc, dsapandora) on #1713: AGE layer: - length + nesting caps now run BEFORE the parse (check_pre_parse): an oversize query is rejected in an O(n) scan instead of after seconds of ANTLR CPU; new max_nesting_depth cap (50) keeps deep expressions from ever nearing the recursion limit - RecursionError backstop in analyze(): the layer's contract (every failure is an AgeTranslationError) now holds even for input that slips past the scan Task engine env hygiene: - also scrub inherited ROCKETRIDE_DB_DSN (stale parent value must not leak into unrelated pipelines or survive a broker failure pointing at another tenant); env building extracted to _build_subprocess_env and covered by tests incl. the credential-scrub assertions - broker failures now pass the real reason to the node via ROCKETRIDE_DB_RESOLVE_ERROR — no more "sign into cloud" when the provisioner is down Broker client (account/base): - ROCKETRIDE_DB_BROKER_URL must be https (plain http only for localhost — the local rig): the token must never travel cleartext - broker DSN validated: postgres scheme required, sslmode=require pinned when absent, so a broker regression can't silently downgrade tenant transport security Nodes: - vector: restore depends() in validateConfig (CONFIG mode installs nothing else — save-time probes actually run on a fresh worker); render() assembles via dict keyed by chunk index (bounded by rows, not chunkId) and is now unit-tested (order, gap, empty, high-id) - graph: assert autocommit off where the safe path opens its transaction (READ ONLY + SET LOCAL are silent no-ops outside one) and prove it end-to-end with SHOW transaction_read_only; validate edge labels against the identifier rule before interpolation; fix stale SQLAlchemy comment CI: - start pgvector + Apache AGE 1.5.0 containers in the Ubuntu build job (MinIO/Azurite pattern) and export RR_TEST_PG_DSN/RR_TEST_AGE_DSN — the 26-test integration suites now RUN on every PR instead of silently skipping; RR_REQUIRE_DB_TESTS turns any skip into a hard failure so a dead container can never go green Docs: firewall docstring no longer promises unwired config knobs; neo4j.md tool prefix example uses the node-id form. Local: 106 passed (45 translator + 35 seam + 26 integration incl. the new READ ONLY proof, run in RR_REQUIRE_DB_TESTS mode). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@asclearuc @dsapandora — all findings addressed in 6d0737c. Mapping, most severe first: asclearuc:
dsapandora:
Local before push: 106 passed (45 translator + 35 seam + 26 integration, the latter in 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nodes/src/nodes/rocketride_graph/IGlobal.py (1)
255-261: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winApply the autocommit invariant to
_validate_query.
_validate_queryrunsSET TRANSACTION READ ONLYbeforeEXPLAINon its own cursor and does not enforce_execute_plan’sautocommit=Falseguard, so validation can bypass the per-transaction timeout and write protection. Reuse_execute_plan/the transaction precondition here or add a regression test that asserts_validate_queryrejects autocommit connections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/rocketride_graph/IGlobal.py` around lines 255 - 261, Update _validate_query to enforce the same autocommit=False precondition as _execute_plan before opening its cursor and executing the read-only transaction. Reuse the existing transaction guard rather than duplicating logic, and ensure autocommit connections are rejected during query validation.
♻️ Duplicate comments (1)
nodes/src/nodes/rocketride_vector/rocketride_vector.py (1)
583-587: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse keyset pagination instead of treating an under-filled ID range as EOF.
Line 588 exits when fewer rows than
renderChunkSizeare returned. SincechunkIdis caller-provided and may be sparse, a document with chunks0andrenderChunkSize + 1emits only the first chunk. Update the render query/loop to paginate from the last returnedchunkId(with a rowLIMIT) so the termination condition uses consistent units.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/rocketride_vector/rocketride_vector.py` around lines 583 - 587, Update the render query and pagination loop near the existing renderChunkSize termination logic to use keyset pagination: order by chunkId, filter subsequent queries to chunkId values greater than the last returned chunkId, and apply renderChunkSize as a row LIMIT. Continue until the query returns no rows, rather than treating an under-filled sparse ID range as EOF; preserve rendering of chunks such as 0 and renderChunkSize + 1.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/_build.yaml:
- Around line 232-234: Update the Docker image in the CI database startup
command to use the supported pgvector 0.8.0 version with the pg16 variant and
its immutable sha256 digest, rather than the moving pgvector/pgvector:pg16 tag;
alternatively, update the workflow’s stated supported target to match the
selected image version.
---
Outside diff comments:
In `@nodes/src/nodes/rocketride_graph/IGlobal.py`:
- Around line 255-261: Update _validate_query to enforce the same
autocommit=False precondition as _execute_plan before opening its cursor and
executing the read-only transaction. Reuse the existing transaction guard rather
than duplicating logic, and ensure autocommit connections are rejected during
query validation.
---
Duplicate comments:
In `@nodes/src/nodes/rocketride_vector/rocketride_vector.py`:
- Around line 583-587: Update the render query and pagination loop near the
existing renderChunkSize termination logic to use keyset pagination: order by
chunkId, filter subsequent queries to chunkId values greater than the last
returned chunkId, and apply renderChunkSize as a row LIMIT. Continue until the
query returns no rows, rather than treating an under-filled sparse ID range as
EOF; preserve rendering of chunks such as 0 and renderChunkSize + 1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8039c5d3-f793-4dac-a6be-8a5d779d81fd
📒 Files selected for processing (5)
.github/workflows/_build.yamlnodes/src/nodes/rocketride_graph/IGlobal.pynodes/src/nodes/rocketride_graph/__init__.pynodes/src/nodes/rocketride_vector/IGlobal.pynodes/src/nodes/rocketride_vector/rocketride_vector.py
…-nodes Adapts to #1686's task-file identity model: the task engine no longer sets ROCKETRIDE_CLIENT_ID in node subprocesses (identity rides the 0600 task file; the ROCKETRIDE_* env namespace is caller-influenced by design), so - _build_subprocess_env keeps the credential scrubs + DSN injection but no longer sets the identity env var (env test updated to assert absence) - the vector store's connection subKey now keys on the DSN's tenant database (the actual isolation unit) instead of env identity - rocketride_db docs updated: current_client_id remains only for the account fallback used by server-process callers and tests Local: 106 passed (45 translator + 35 seam + 26 integration in RR_REQUIRE_DB_TESTS mode). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
asclearuc
left a comment
asclearuc
left a comment
There was a problem hiding this comment.
Thanks @dylan-savage — this is a thorough round, and all twelve of my findings are resolved.
The two I marked High are not just patched but improved on: check_pre_parse plus a new max_nesting_depth cap plus a RecursionError backstop is a better design than the single fix I suggested. I re-ran my original repros against the layer at 6d0737cb rather than reading the diff: the 100-paren query now raises a clean AgeFirewallRejected in 0.000s, and a 3.2 MB query is rejected in 0.000s where 800 KB previously cost 22.6 seconds. I also checked the cap edges — 50 parens pass, 51 reject, and 300 non-nested (1)+(1)+… pass, so the scan tracks depth rather than bracket count. Starting the containers with RR_REQUIRE_DB_TESTS turning a skip into a hard failure is exactly the right shape for that gap.
Requesting changes for one new item only: CodeRabbit's Major on the new CI step is valid — #1713 (comment). I verified it against the registry instead of taking the bot's word: pgvector/pgvector:pg16 is a moving tag that currently resolves to 0.8.3 while the cloud runs 0.8.0, and pgvector/pgvector:0.8.0-pg16 exists, so it is a one-line pin. It matters here specifically because these containers were added to prove test_semantic_search_uses_hnsw_index_not_seq_scan, which asserts planner behaviour — a pass on 0.8.3 does not establish it on 0.8.0, and a floating tag means a future CI break with no code change. The workflow comment discloses the gap honestly, but a disclosure is not a pin. While you are there: apache/age:release_PG16_1.5.0 pins the AGE version correctly, but the Postgres minor inside that image still floats.
One nit inline, on the autocommit guard. I also agree with the withdrawal on _agtype/builder.py, and deferring the vector-store dedup to a committed follow-up PR is the right call for a diff this size.
Not raising it as a finding, but recording that it was tested rather than assumed: check_pre_parse counts brackets inside string literals, so WHERE n.txt = '(((((…50+…' is rejected. Balanced () x60 passes, so only unbalanced runs trip it — your documented trade-off holds.
| return False, 'Database connection is not initialized' | ||
| try: | ||
| with self.client.cursor() as cur: | ||
| cur.execute('SET TRANSACTION READ ONLY') |
There was a problem hiding this comment.
nit — _validate_query did not get the autocommit guard that _execute_plan gained.
The new assert at line 201 is the right fix, and the comment above it explains exactly why:
Both server-side controls below (
SET TRANSACTION READ ONLYand the plan'sSET LOCAL statement_timeout) are silent no-ops outside a transaction.
That reasoning applies word-for-word here. This method runs the same SET TRANSACTION READ ONLY and then the plan's SET LOCAL statement_timeout, with no precondition check.
The risk today is low — EXPLAIN without ANALYZE does not execute the query, so nothing can write. The part that would actually degrade is the timeout: outside a transaction SET LOCAL only emits a warning, so a pathological EXPLAIN would run uncapped.
My concern is less the behaviour than the shape. Someone reading this file later sees one of two identical transaction-opening paths guarded and the other not, and has to work out whether that was deliberate. Either hoist the check into a small helper both call, or add the same three lines here.
There was a problem hiding this comment.
Fixed in 935bf3e — took the hoist option: _require_transactional_client() now carries the precondition (client present + autocommit off) and both transaction-opening paths call it — _execute_plan raising, _validate_query folding the RuntimeError into its return-a-message contract. The two identical paths are now guarded identically, so no future reader has to wonder whether the asymmetry was deliberate.
Round-3 review (asclearuc): - CI: pgvector/pgvector:pg16 is a moving tag (had drifted to 0.8.3) — pin 0.8.0-pg16 so the HNSW planner-behaviour test proves the version the cloud actually runs, and a registry tag bump can't break CI with no code change. AGE stays exactly 1.5.0; remaining drift (PG minor, two-images-vs-one-database) documented, exact-pin GHCR image still the follow-up. - graph: hoist the autocommit precondition into _require_transactional_client(), called by BOTH transaction-opening paths — _execute_plan (raising) and _validate_query (folded into its return-a-message contract), so the two identical paths are guarded identically. Local: 106 passed (RR_REQUIRE_DB_TESTS mode). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@asclearuc — thank you for re-running the repros rather than trusting the diff; the cap-edge checks (50/51, non-nested 300) and the string-literal trade-off verification are exactly the scrutiny this layer should get. Both round-3 items are in
One heads-up since your review: |
3 participants
What this adds
Three RocketRide-branded pipeline nodes that give every tenant a zero-setup managed
Postgres in RocketRide cloud, plus the shared machinery they stand on:
rocketride_sql— SQL tables on the tenant databaserocketride_vector— pgvector store with HNSW indexing (cosine/L2/IP,m/ef_constructionconfigurable)rocketride_graph— Cypher queries translated to Apache AGE, safe path runs in server-sideREAD ONLYtransactionsai/common/rocketride_db.py) — one place that answers "which database is yours", with strict precedence:ROCKETRIDE_DB_DSNinjected by the server at task start (hosted path)Account.resolve_db_dsn(broker → provisioner)AccountBase.resolve_db_dsncalls thedata-core provisioner (
POST /provision {tenant_id}→{database, role, dsn, created},idempotent, derived passwords); the task engine injects the DSN into node env only for
pipelines that use these providers. extension.saas overlay needs zero code — cloud
enablement is two env vars (
ROCKETRIDE_DB_BROKER_URL,ROCKETRIDE_DB_BROKER_TOKEN).ai/common/graph/age/) — vendored openCypher M23parser (generated at ANTLR 4.13.2, committed; no Java for devs), resource firewall
(depth/timeout/length caps on both paths), version-keyed capability table (every 1.5.0
cell empirically verified — zero TBD), collision-proof
cypher()envelope emission withPREPARE/EXECUTEparam binding (transaction-pooling safe), vendored agtype decoder.apache-age-pythondeliberately NOT a dependency (stale on PyPI, pins antlr 4.11.1).Review guide — anatomy of a 21k-line diff
The diff is large but ~71% of it is not human-written code. Where the 21,024 added
lines actually come from:
graph/age/_cypher/gen/— machine-generated parser: deterministic ANTLR 4.13.2 output from the official openCypher M23 grammar, committed so devs never need Java/ANTLR installed. Regenerate from the same grammar → identical code.graph/age/_agtype/— vendored agtype decoder fromapache/age(Apache-2.0). PyPI'sapache-age-pythonis stale and pins an incompatible ANTLR, so the upstream files are carried verbatim: original ASF license headers preserved, provenance banner naming the exact upstream commit (5a254d68), and the two local deviations itemized in__init__.py. Apache-2.0 is MIT-compatible.graph/age/— hand-written translation layer: analysis, resource firewall, capability table, envelope emission, decode glue.nodes/src/nodes/rocketride_{sql,vector,graph}/— the three nodes.packages/ai/core — resolver seam,Account.resolve_db_dsnbroker, task-start DSN injection. Smallest and highest-leverage: this is the part wired into every task start.So the real review surface is ~6,200 lines, and the 288 in
packages/aideserve themost attention.
Why ship a Cypher parser at all?
Because Apache AGE cannot run bare Cypher — every query must be rewritten into a SQL
envelope, and building that envelope correctly requires understanding the query:
SELECT * FROM cypher('graph', $$ ... $$) AS (c0 agtype, c1 agtype, ...)— theASclause must match the query's
RETURNprojection exactly. You cannot know how manycolumns a query returns without parsing it.
PREPARE/EXECUTE. AGE rejects inline::agtypeparameterliterals; the only safe binding path (especially through a transaction-mode pooler) is
a prepared statement — which the layer emits and the cloud smoke verified live.
SET TRANSACTION READ ONLY; deciding a query is read-only requires structure, notstring matching (
CREATEappearing in a property value must not trip it).before the database with an actionable message ("use plain MERGE, then SET…") that
the LLM repair loop can act on — instead of AGE's raw
syntax error at or near ":".Same for caps: recursion depth, statement timeout, query length, unbounded
*paths.A regex can do none of that reliably, so the layer parses with the official openCypher
M23 grammar (the standard AGE itself targets) run through ANTLR — which is the entire
reason for the 12.7k generated lines above. Full rationale in
packages/ai/src/ai/common/graph/age/README.md.Credential model
The tenant is derived server-side from the authenticated connection's
AccountInfoand isnever read from pipeline config (no tenant retargeting).
"sign into RocketRide cloud" error at task start. The OSS sign-in path
(personal
rr_API key exchanged at a cloud door) is designed and was fullybuilt + tested, but is deferred with its server side — see Deferred.
How it's verified
precedence, broker client against a real fake-HTTP server, translation layer).
(PG 16.14 + AGE 1.5.0 + pgvector 0.8.0), incl. planner proof that HNSW indexes are used.
local stack (template-seeded extensions, pgbouncer 1.25.2 transaction mode + TLS +
auth_query): provision → idempotent recall → pooled tenant connect → full node workload.
and pooler — provision slow path, idempotency, TLS connect, public-schema DDL, HNSW+knn,
ag_graphpresence, cypherPREPARE/EXECUTEbound-param through transaction pooling,READ ONLYwrite-block. Also landed as a permanent CI test in the saas repo (Implement co-sponsor API integrations for Mar 30 hackathon #414).formerly-TBD constructs are syntax-level rejects on 1.5.0 and now fail pre-flight with
actionable alternatives.
Cloud-side dependencies (all shipped)
saas #381 (provisioner, live in cluster), #405/#415 (image pins), #408 (pooler-host env
fix — found from this side pre-smoke, would have broken every minted DSN), #414 (CI smoke).
Deliberately deferred (documented in claude/research/connectors-project/rocketride-db-nodes/)
cloud_api_keyfield exchanging a personalrr_key at a cloud door — was fully builtand tested on this branch (
8cf951ba), then deliberately reverted (5e732d22): itsserver half (an authenticated door route on api.rocketride.ai + external pooler
exposure with
verify-fullDSNs) has no owner yet, and shipping a credential fieldthat cannot work would mislead OSS users. Restore point: revert the revert when the
server side is scheduled.
EMULATErewrites (the capability table has the hook; no emulations are implementedand none are planned — unsupported constructs simply reject with guidance).
rocketride_vectorcurrently carries~600 lines duplicated from
store_postgres(per review feedback). The follow-upextracts a shared Postgres store implementation between
DocumentStoreBaseand thetwo nodes, so each overrides only connection resolution + HNSW indexing — the shape
rocketride_sqlalready has. The inheritedrender()loop-exit units mismatch(row count vs chunkId window) gets fixed once there, in the shared code, rather than
twice here.
🤖 Generated with Claude Code
Summary by CodeRabbit