Repository navigation
Conversation
Adds references/ontology-design.md: fourteen principles for designing a schema that many agents read and write, each mapped to OmniGraph features. The unifying goal is convergence, meaning that independent agents given the same input write the same graph and read the same meaning back. SKILL.md gains a short summary section after the provenance rule and a Deep Dives entry for the new reference.
Design guidance lived in four places: Gruber's criteria, a twelve-rule summary whose full text did not exist, and the provenance rule, all in SKILL.md, plus a brief list in references/schema.md. It now lives in references/schema-design.md, which also absorbs the multi-agent principles. SKILL.md keeps one short Schema Design section, and schema.md links to the new page. The page also recommends a GraphPolicy node type, so rules the schema cannot express become data that agents read and lint the graph against. It is distinct from the server's Cedar policy.
A schema is too loose when one meaning has several valid encodings and too tight when a meaning has none. The two are not ends of one slider, since a misplaced distinction causes both, so the section argues for placing constraints where writers must converge and leaving declared extension points open. It also covers how to measure each failure and which way to err in OmniGraph, where loosening applies in place and tightening is a rebuild.
Schema design gains two principles. The first layers extracted and synthesized knowledge over raw spans, with lineage back to them, so re-extraction and retraction stay possible. The second designs for both halves of retrieval: links for recall; specific edges, filters and answer-sized units for precision. Composition now covers module facets that never modify the core, and general primitives that combine. Every principle has a short example in one running company-knowledge domain. All eleven schema snippets initialize and all four queries lint against a combined example schema.
ragnorc
left a comment
There was a problem hiding this comment.
Recommendation: request changes for the two documentation defects below. I am submitting a COMMENT review because this account owns the PR.
This PR brings scattered schema advice into one guide. It also explains how agents can share identities, preserve evidence, and retrieve the same facts. The short skill summary points to worked schema and query examples. No engine code or storage format changes. The practical effect is on the schemas and write workflows that agents create next.
The direction is useful. However, two examples promise more than their mechanisms provide:
- P2 — Extraction keys can collapse independent facts. “Span + extractor” identifies an extraction, but cannot identify each assertion it produces. Two merge loads with that key retain one assertion.
- P2 — Required-link advice overstates
@card(1..). A source node can enter the graph without any edge. Such a node escapes the edge-delta cardinality check and disappears from link-based retrieval.
The inline comments give reproductions and corrections. Both issues need changes before agents use this guidance as a contract.
The intended workload is repeated extraction from documents, shared reads, and reviewed enrichment by several agents. It requires distinct facts to survive retries. It also requires readers to distinguish complete evidence from missing links. Atomic publication protects the submitted graph change. It cannot repair a key that identifies two facts as one.
The tradeoffs are concrete:
- Deterministic keys make retries converge. Their components must distinguish every fact that users need to retain. Similarity search can suggest duplicates, but cannot replace exact identity.
- Typed provenance and separate knowledge layers support correction and attribution. They add rows, edges, traversals, and lifecycle work. On object storage, those operations have a cost. This PR provides no latency or throughput measurements.
- Tight constraints reduce ambiguous writes. They also make later changes harder. The schema planner refuses cardinality changes and required-property additions. Agents must plan evolution through the existing administrative workflow.
GraphPolicymakes shared rules discoverable and reviewable. It also adds an application convention and a query-maintenance obligation. Reading and running those rules remains a writer responsibility. These nodes do not add an engine enforcement step.
Liability: consolidation removes competing homes for the advice. The net addition of 636 lines is not itself a defect. Examples can prevent repeated implementation mistakes. However, the two incorrect guarantees add liability: downstream schemas can lose facts or omit evidence silently. As written, I assess this as a net increase in liability until the examples reflect the actual contract. Correct those guarantees at their source. Do not add another storage primitive or recovery layer to compensate.
Optional improvement: scope the “every graph” policy-node recommendation to graphs that need shared rules beyond schema constraints. Five more unconditional conventions would create five more contracts for each agent to maintain. Prefer the existing schema, queries, and publication workflow where they meet the need.
Validation:
- Reviewed clean, isolated head
7147a47b74f7870f6f8fcbb2da25005a479f3a79. - Local documentation check passed: 155 Markdown files. Repository-link checks and skill spelling checks passed.
- Local
cargo +1.97.1 test -p omnigraph-engine --locked --test validators --features failpointspassed all 34 tests before temporary probes. - Two temporary assertions failed as predicted: two facts retained one row, and a node without its required edge entered the graph.
- Controls passed all 34 tests. A per-fact key retained two assertions after two loads each. An explicit orphan query found the missing link. I restored all temporary source edits.
- Inspected the loader, mutation staging, validator, migration planner, search guidance, and deployment boundaries. Checked the relevant full Lance documentation against the pinned Lance 11.0.0 source, including its upsert behavior. OmniGraph selects
UpdateAllfor upserts. - The CI run for this exact head passed workspace tests and its configured S3/Azure checks. This is CI evidence, not a local cloud test.
I did not measure agent agreement, search quality, or production performance. The review does not treat the reported small extraction experiment as a general guarantee.
| start_ms: I64 | ||
| } | ||
| node Assertion { // extracted | ||
| slug: String @key // span + extractor, so re-runs upsert |
There was a problem hiding this comment.
[P2] Include the assertion identity in the extraction key
One source span can contain several independent facts from the same extractor. This key gives all those assertions the same identity. For example, one passage can say “Acme signed the contract” and “Acme requested support”. Two merge loads with slug = segment-1:extractor-v1 keep only one assertion. The second statement replaces the first.
This follows the actual write path: normalize_strict_node_rows derives the row ID from @key, and stage_keyed_write_from_stream uses Lance's WhenMatched::UpdateAll. A local probe with this Assertion schema confirmed the collision.
Include a stable per-assertion discriminator in the key, alongside the span and extractor. Update both this example and the rule above it. Check that two facts from one span remain distinct, while a repeat extraction does not add duplicates.
| **In Omnigraph:** write the questions first as `.gq` queries and lint them | ||
| against the schema. Use `@card(1..)` for links that must exist. Scope with |
There was a problem hiding this comment.
[P2] State the zero-link limit of cardinality enforcement
At this head, @card(1..) does not ensure that every source node has a link. A load that creates only the source node succeeds. The validator calls evaluate_cardinality only when the edge table has a change. It then checks sources from changed or removed edges. A source with no edge never enters that set (crates/omnigraph/src/validate.rs:810-828,1167-1182).
A local probe loaded a Person without its required WorksAt edge. The load succeeded, and a not { $p worksAt $c } query returned the orphan. The same gap applies to the new ExtractedFrom example. Traversal can silently miss such assertions, despite the promised required link.
Document this limit and show an explicit orphan check in the write/review workflow. Do not present cardinality alone as proof of complete provenance or retrieval.
…nk gap A key of span plus extractor gives every fact from one span the same address: one load carrying two of them fails on the duplicate key, and separate merge loads keep only the last. Key each fact on the span, the extractor and a digest of its normalized statement. @card counts a source node's edges only when a write adds, moves or removes one of them, so a node written with no edge of that type is never counted. Document the gap where the page relies on @card(1..) and give required links an orphan check. Scope the GraphPolicy recommendation to graphs with rules the schema cannot express.
ragnorc
left a comment
There was a problem hiding this comment.
Recommendation: approve. Both findings from the first review are addressed at b16e1397672791781135d89e9fef7e29094b55ff. I submit a COMMENT review because the authenticated account owns the PR.
This PR puts schema-design advice in one guide. It explains how several agents can share identities, retain evidence, and find the same facts. The revision corrects two examples that could mislead writers. Each extracted fact now gets its own key. Required links now have an explicit orphan query, because declaring @card(1..) does not detect a node created without an edge. This changes guidance, not the engine's enforcement contract.
The two fixes match the code and local results:
- Fact identity: span, extractor, and statement digest distinguish facts from one extraction. Two different example keys retained two assertions after repeated merge loads. The keyed-write adapter selects Lance's
UpdateAllandInsertAllbehavior. A transaction cannot recover a distinction that the input key erased. This correction addresses the cause of the earlier overwrite risk. - Required links: the guide now states the zero-link gap. The validator starts cardinality checks from changed edge tables and checks affected sources. It does not enumerate every source node. The exact
assertions_without_spanquery returned two orphans before their links existed and zero after both links were added.
The intended workload is repeated extraction, reviewed enrichment, and shared retrieval over growing source material. The tradeoffs remain explicit. A statement digest makes identical normalized output repeatable; changed wording can create another key. Writers still need a shared normalization rule and a decision about semantic equivalence. A nearest-neighbor lookup supplies candidates, not proof that two assertions mean the same thing. Provenance and separate knowledge layers make correction possible, but add rows, links, reads, and lifecycle work. Orphan checks add validation reads and remain a writer obligation. A successful check is an observation of that snapshot, not a new atomic enforcement step.
Liability is lower with these corrections. The PR adds 683 net documentation lines, but consolidates advice and removes two false guarantees. It adds no storage format, execution primitive, or public engine API. The narrower GraphPolicy recommendation also helps: graphs need it only for rules beyond schema enforcement, and the guide now names the cost of maintaining each check query. Five more application conventions would still add five more obligations. Keeping each rule in one place and deriving its checks from existing queries is the preferable direction.
Validation:
- Clean isolated checkout at the exact head. The merge from main matches Git's automatic merge; I reviewed the subsequent documentation delta.
- Fresh local Rust 1.97.1 build: all 35
validatorstests passed. A temporary extension used the guide's exact lineage schema, assertion keys, and orphan query. All 35 tests passed with that extension. I restored the source, rebuilt, and reran the original suite successfully. - Documentation checks passed for 218 Markdown files. Repository instruction-link checks and
typos skills/omnigraph/passed. - Exact-head main CI, GQT, and DST passed. Main CI includes the configured RustFS and Azurite checks. Local validation used filesystem storage.
- Inspected the loader, keyed-write adapter, validator, and pinned Lance 11.0.0 merge-insert implementation against the relevant full upstream guides. No agent-convergence, semantic-deduplication, retrieval-quality, or performance claim was measured in this follow-up.
No remaining blocking finding in this scope. The earlier two inline findings can be considered addressed.
ragnorc
left a comment
There was a problem hiding this comment.
Recommendation: approve fe7075bd8ab70849e42e48cbc32ff53c3056bf47. No new blocking finding. I submit a COMMENT review because this account owns the PR.
This PR gives agents one place to learn how to design an OmniGraph schema. It covers shared identities, evidence links, and repeatable extraction. The practical aim is to keep distinct facts intact and make missing evidence visible as several agents grow the graph. It changes guidance, not the engine's enforcement contract.
This follow-up covers the merge since the previous review. The merge has no manual conflict resolution. Only .github/workflows/gq-logic-tests.yml and docs/dev/ci.md differ from the approved b16e1397672791781135d89e9fef7e29094b55ff. The complete crates/ and skills/ trees, Cargo manifests, and lockfile are byte-identical.
The two earlier corrections remain sound:
- Fact keys distinguish assertions from the same span and extractor. This addresses the identity error at its source. The keyed-write adapter still selects Lance
UpdateAllfor an existing key. Atomic publication cannot recover a distinction that the key erased. - Required links retain the zero-link caveat and an explicit orphan query. The validator still starts from changed edge tables. The guide therefore assigns the missing check to the writer without claiming new engine enforcement.
For repeated extraction and shared retrieval, the tradeoffs remain important. A statement digest requires a shared normalization rule; different wording can still produce another key. Provenance adds rows, links, reads, and lifecycle work. An orphan query checks a snapshot; it does not add atomic enforcement. The scoped GraphPolicy advice leaves application checks as explicit maintenance obligations. Consolidation and removal of false guarantees reduce liability. No new runtime API, persistent state, or storage primitive is added. Five more application conventions would still require five more maintained checks, so each should justify its existence.
The inherited CI change selects the same three packages for dispatch and unit tests. This avoids changing the resolved feature set between those commands. It also runs all benchmark units instead of only fixture parity. The tradeoff is more test execution for less repeated compilation. I did not benchmark the build-time claim.
Validation:
- Clean isolated checkout at the stated head. Git tree/blob comparisons confirmed the unchanged code, examples, tests, dependencies, and required review guides.
- Local documentation checks passed for 218 Markdown files. Instruction links, skill spelling, action pins, merge-group contexts, required test names, reader closure, and diff whitespace checks passed.
- The additional local change-class replay check passed its first 17 scenarios, then stopped when a temporary Git fixture could not create a commit (exit 128). This is a setup limitation, not a behavioral failure or a passing full replay check.
- GQT CI executed the changed commands: 10 dispatch tests passed; the three unit suites passed 312, 35, and 176 tests, with one ignored benchmark test. Fixture parity ran and passed. All 8 seam-guard tests passed. The complete GQT/DST job also passed.
- CI checked out merge
925a7d76e790b71c397197dd19c4123b81988dfb. Its tree and the reviewed head's tree are both2f7672307fec0e3ece1293de81dc6e17faec7c50. Main CI passed, including workspace, RustFS, Azurite, and storage compatibility jobs. These are CI results, not local cloud tests. - The prior local schema/key/orphan proofs remain applicable because their code and examples are unchanged. I rechecked the adapter and validator, and verified the Lance 11.0.0 archive against
Cargo.lockand the installed merge-insert source against that archive. The previous full upstream audit is reused for this unchanged substrate.
No new runtime fix is introduced in this follow-up. A GQT result case cannot test Cargo package selection; the existing workflow and dispatch owners cover that boundary. I did not rerun local Rust/GQT suites or claim a new before/after regression proof. No temporary source or test edits remain. I have no new optional improvement to add.
Summary
The skill's schema design guidance lived in four places:
SKILL.md.SKILL.mdthat pointed to full text inreferences/schema.md. That full text did not exist.SKILL.md.references/schema.md.This PR moves all of it into one page,
skills/omnigraph/references/schema-design.md, and adds what the skill lacked: how to design a graph that many agents read and write.The page has three parts:
.gqqueries.The page recommends a
GraphPolicynode type for every graph agents share. Rules the schema cannot express become versioned, reviewable data that agents read before writing and lint the graph against:check_querynames a stored query for mechanical rules.statementcovers rules that need judgment.GraphPolicyis distinct from the server's Cedar policy.SKILL.mdreplaces the four sections with one short "Schema Design" section that keeps the operational rule about which schema changes apply in place, and points to the page. It is 2 lines longer than on main.references/schema.mdnow links to the new page.The multi-agent principles come from building an argument graph on OmniGraph. Two independent agents encoded the same six passages from a written rule sheet. Claim-level agreement was 94% and reasoning structure 85%. The disagreements pointed to a missing category, a missing rule, or a misplaced property, which became principle 15.
Testing
python3 scripts/check-docs.pytypos skills/omnigraph/bash scripts/check-agents-md.shomnigraph init --schema).omnigraph lint).Every OmniGraph feature the page names was checked against the v0.11.0 skill references and CLI: decorators, keyed edges, enum widening, implicit aggregate grouping,
schema showreturning descriptions,changes poll,commit listandsnapshot.