Repository navigation
fix(inputlayer-js): compile timestamps, count(), in()/notIn() and meta commands the engine accepts - #233
Merged
Conversation
… commands the engine accepts Phase-1 item P1-B of the SDK redesign: the JS SDK sent several forms the engine rejects or misreads. - A `timestamp` column is declared as `int` holding Unix milliseconds (the engine's schema parser rejects `timestamp`); `Timestamp` and `Date` values are written as milliseconds. - `count()` with no column compiled to `count<>`, which the engine rejects. It now counts the first variable of the first atom (`count<Id>`), in kg.query() aggregates and in rule heads; the result column is still labelled `Count`. - `col.in(other)` compiled to `rel(..., X, ...)`, which is not IQL. It now emits an atom of the target relation with the source variable in the target column and `_` elsewhere; `notIn` negates it. Rule conditions are compiled before the body atoms, so a filtered column that is not selected gets its variable in the atom instead of `_`. - Meta commands come from one table (`meta`), pinned by the shared fixture packages/conformance/meta-commands.json that the Python SDK will render against too. session.listRules() sent `.session list` and dropRule(name, index) sent `.session remove`, both rejected; ruleDefinition() sent `.rule show`, which the engine reads as a query of a rule named `show` and answers empty; listRules() returned the `Rules:` header as a rule. They now send `.session`, `.session drop <n>` and `.rule def`, and parse the replies with the grammar the fixture records. - The stale integration test expected a program mixing writes with `.rel drop` to apply partially; the engine rejects it as `unsupported` and applies nothing. tests/integration.test.ts now runs in `make js-test-live` (the full-suite js-sdk-live job) beside the docs query examples, including a test that sends every command of the shared table to the engine and checks the rejected spellings fail.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
So fix the SDKs by improving them. (Context: the captain asked to rework the SDKs so agents and agentic loops use InputLayer without writing Datalog, and to fix the SDK release blockers by improving the SDKs rather than patching. The agreed design, signed off by an independent challenger after three rounds, is the InputLayer SDK redesign report; this item is its phase-1 PR P1-B (JS).)
Substance of the referenced item P1-B (JavaScript SDK, packages/inputlayer-js), from that design:
timestampcolumns: the engine rejects thetimestampschema type, soDate/Timestampcolumns declareintand carry Unix milliseconds (rule R-TYPE).count()without a column compiled tocount<>, which the engine rejects; it must compile tocount<V>over the first variable of the first atom (rule R-AGG).in()(compileIn) emittedrel(..., X, ...), which is not IQL;col.in(Other.col)must emit a positive atom of the other relation with the variable in that column and_elsewhere, andnotInthe negated atom (rule R-IN)..session list(rejected); the command is.session(and drops are.session drop <n|name>). Meta commands come from one shared table, with the same JSON fixture used by both SDKs, so a wrong spelling cannot exist in one SDK only (rule R-META:.session,.rule def,.rule drop,.subscribe,.unsubscribe,.why,.why_not).unsupportedplus rollback, and that test file runs in CI in the js-sdk-live job.Each rule is pinned by unit tests on the compiled text and live tests against a real engine that run in CI. Other phase-1 items (Python compile path, connection cores, subscriptions, claims, README hero) are separate PRs and out of scope.
What Changed
timestampcolumns are now declared asintand store Unix milliseconds, andDatevalues are written as milliseconds. Acount()with no column now compiles tocount<V>over the first column of the first relation, where it used to emitcount<>.col.in(Other.col)/notInnow compile to a positive or negated atom of the target relation, with the variable in the target column and_in every other position. Targets that aren't real relation columns are rejected.src/meta.ts, which also has parsers for the.session,.rule listand.rule defreplies. The table is pinned by a shared fixture,packages/conformance/meta-commands.json. This fixes the wrong spellings.session list,.session removeand.rule show.session.dropRule(name, index)now works out the clause's position across all session rules and sends.session drop <n>.unsupportedand a rollback.make js-test-livenow also runstests/integration.test.ts. The JS SDK guide (and the GUI docs bundle) are updated for the timestamp type and session rule drops.Risk Assessment
Testing
I built the SDK from HEAD and started a disposable real inputlayer-server (the prebuilt release binary from main, copied to a scratch dir with a fresh data dir). Against it I ran the JS live integration suites (81 tests, all passing, including the new timestamp, count(), in()/notIn(), meta-table and mixed-program tests). I also ran a wire-logging driver for 10 end-user scenarios, all passing. The wire log showscount<Id>,mgr(_, Id)/!mgr(_, Id)atoms,.session/.session drop <n|name>/.rule def, a misspelled in() target raising a clear error, and a mixed program rejected asunsupportedat index 1 with no writes applied. The server and scratch dir were torn down afterwards.+event(id: int, occurredAt: int), filter returns [[2,1704153600000]]); vitest-live.log 'defines a timestamp column and stores Unix milliseconds'.session listis rejectedEvidence: Live vitest run (integration + docs-queries) against real engine
Evidence: End-user driver transcript with wire statements
Evidence: Driver script
Evidence: Key compiled/wire output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
packages/inputlayer-js/src/derived.ts:90-in()/notIn()throws when its target comes from afrom(...).where((a, b) => ...)callback. The callback'sRelationProxy.col()(proxy.ts:205) builds aColumnProxywithoutrelationColumns, sotargetColumnsstays undefined andcompileIn(compiler.ts:216) throws 'needs a column taken from a relation definition'. Example:from(Employee, Manager).where((e, m) => e.col('id').in(m.col('employeeId'))). Of the three ways to get a column,RelationDef.col(relation.ts:53) andRelationRef.col(proxy.ts:226) pass the column list, but this one doesn't. Fix: giveRelationProxythe column list (derived.ts:90 has it asr.def.columns) and pass it through inRelationProxy.col.packages/inputlayer-js/src/index.ts:319- This change adds to the SDK's public API:meta,sessionRules,ruleList,ruleClausesand theMetaCommandNametype are now exported from the package entry point. The intent (R-META) asks for one internal table pinned by the shared fixture. It does not ask to make the raw command builders and reply parsers public API, which then has to stay stable. The only consumer is integration.test.ts, which could import from '../src/meta' instead. Recommendation: remove the public export unless publishing these is a deliberate product decision.packages/inputlayer-js/src/meta.ts:4- Some comments describe the Python SDK as already using this fixture, which it doesn't yet. meta.ts:4 says 'The Python SDK keeps the same table'. integration.test.ts:991 says the fixture is 'The fixture the Python SDK renders its command table against too'. The fixture description in packages/conformance/meta-commands.json says the same. At this commit packages/inputlayer-py has no meta table and doesn't read the fixture; that work is a separate phase-1 PR. Reword these comments as planned work (or as applying only to the JS SDK) until the Python PR lands.🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:
packages/inputlayer-js/src/index.ts:319- This change adds to the SDK's public API:meta,sessionRules,ruleList,ruleClausesand theMetaCommandNametype are now exported from the package entry point. The intent (R-META) asks for one internal table pinned by the shared fixture. It does not ask to make the raw command builders and reply parsers public API, which then has to stay stable. The only consumer is integration.test.ts, which could import from '../src/meta' instead. Recommendation: remove the public export unless publishing these is a deliberate product decision.packages/inputlayer-js/src/compiler.ts:244-compileInputs_in every column whose name doesn't equaltarget.nameand never checks thattarget.nameis one of the target's columns. If the target column name is misspelled, it compiles to an all-_atom such asmanager(_, _).in()then just checks that the relation has at least one row, andnotIn()excludes everything once it does. Either way the result set is wrong and nothing raises an error.RelationDef.col(relation.ts:47) validates the name. Columns from the other two sources do not:RelationProxy.col(proxy.ts:207), whose column list the round-1 fix added, andRelationRef.col(proxy.ts:227). Example:from(Employee, Manager).where((e, m) => e.col('id').in(m.col('employeId')))compiles tomanager(_, _). Fix: incompileIn, throw when!expr.targetColumns.includes(target.name). That one check covers all three column sources.packages/inputlayer-js/src/index.ts:102- The round-1 fix removed the meta exports but left a// Meta commandssection header with nothing under it in the public entry point. Remove the header.🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:
packages/inputlayer-js/src/index.ts:319- This change adds to the SDK's public API:meta,sessionRules,ruleList,ruleClausesand theMetaCommandNametype are now exported from the package entry point. The intent (R-META) asks for one internal table pinned by the shared fixture. It does not ask to make the raw command builders and reply parsers public API, which then has to stay stable. The only consumer is integration.test.ts, which could import from '../src/meta' instead. Recommendation: remove the public export unless publishing these is a deliberate product decision.packages/inputlayer-js/src/compiler.ts:244-compileInputs_in every column whose name doesn't equaltarget.nameand never checks thattarget.nameis one of the target's columns. If the target column name is misspelled, it compiles to an all-_atom such asmanager(_, _).in()then just checks that the relation has at least one row, andnotIn()excludes everything once it does. Either way the result set is wrong and nothing raises an error.RelationDef.col(relation.ts:47) validates the name. Columns from the other two sources do not:RelationProxy.col(proxy.ts:207), whose column list the round-1 fix added, andRelationRef.col(proxy.ts:227). Example:from(Employee, Manager).where((e, m) => e.col('id').in(m.col('employeId')))compiles tomanager(_, _). Fix: incompileIn, throw when!expr.targetColumns.includes(target.name). That one check covers all three column sources.packages/inputlayer-js/src/index.ts:102- The round-1 fix removed the meta exports but left a// Meta commandssection header with nothing under it in the public entry point. Remove the header.cargo build --release --bin inputlayer-server, started in this run) completed. Nothing has been confirmed against a real engine yet. To get it, re-run this step, or runmake js-test-liveplus the driver at ~/.no-mistakes/evidence/01M43ARJ94W366ZYWX6PEVYDQP/driver.mjs against the built server.npx vitest run tests/compiler.test.ts tests/meta.test.ts(44 passed)cargo build --release --bin inputlayer-server(started; still compiling when the step returned)Built the SDK bundle withnpx tsup src/index.ts --format esm --out-dir <evidence>/sdk-distand wrote an end-user driver script (driver.mjs) that logs the wire text for each scenario; not yet run against a live server🔧 No changes applied.
1 info still open:
packages/inputlayer-js/src/knowledge-graph.ts:235- This branch didn't cause this: during the live run, kg.describe() treated the data rows from the.rel <name>reply as columns (columns [{name:'1',type:'1704067200000'},...]). The parser was there before this change and this diff doesn't touch it. It's outside P1-B and is noted only for follow-up.+event(id: int, occurredAt: int), filter returns [[2,1704153600000]]); vitest-live.log 'defines a timestamp column and stores Unix milliseconds'.session listis rejectednpx tsup src/index.ts --format esm --out-dir <evidence>/sdk-dist(SDK bundle built from HEAD 1a931c46)Started a scratch copy of the release inputlayer-server built from origin/main 23712b83:INPUTLAYER_ADMIN_PASSWORD=admin inputlayer-server --host 127.0.0.1 --port 46005 --data-dir <fresh tmp dir>INPUTLAYER_TEST_SERVER=ws://127.0.0.1:46005/ws INPUTLAYER_TEST_USER=admin INPUTLAYER_TEST_PASSWORD=admin npx vitest run tests/integration.test.ts tests/docs-queries.integration.test.ts --reporter=verbose(81/81 passed live)IL_URL=ws://127.0.0.1:46005/ws node driver.mjs: an end-user driver that uses the built SDK, logs every statement sent on the wire, and checks timestamp, count(), in()/notIn() (including where-callback targets and a misspelled column), conditional delete, session/persistent meta commands, mixed-program rollback, and rejection of.session list(10/10 PASS)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.