Feat/embedding api spec - #678
Open
iheitlager wants to merge 3 commits into
Open
Conversation
…(013/Req-7) 010 gains Requirement 8: a write MUST NOT succeed while leaving an index it could not read unmaintained. Measured at 0.18.5 against stock sqlite3 3.51.0 -- inserting into a stock-created composite-PK table leaves rows out of sqlite_autoindex_*, after which the oracle undercounts and integrity_check reports rows missing, while the write returns rc=0. Names both acceptable fixes (recover the autoindex column list from the declared constraint, or refuse the write) and scopes autoindex *creation* out to V3/V7. The embedding-api spec gains Requirement 7: statements must yield rows incrementally. It also upgrades the composite-PK prerequisite from inferred to measured, flagging it as the highest-priority item in or around that spec -- it is silent corruption of a valid SQLite file, not an ergonomic gap, and its Req 6 byte-identity scenario cannot pass while it stands. That spec is numbered 012 as of this commit and is renumbered to 013 by the next one, after main landed its own spec 012. Refs: 010/Req-8, 013/Req-7
Main landed its own spec 012 (`012-query-constraints`) and its own ADR-0033 (constant propagation / OR-to-IN) while this branch was open, so both numbers collided on rebase. The newcomer moves: - `.openspec/specs/012-embedding-api/` -> `013-embedding-api/`, heading updated - `adr/0033-embedding-api-owns-the-connection-driver-out-of-tree.md` -> `0034-`, heading updated, and its five `spec 012` prose references retargeted to 013 - `adr/index.md` gains the 0034 row, which 0ee936b omitted entirely Renumbering ADR-0034 does not violate the immutability convention: it has never been on main and is still `Status: Proposed`. Also fixes four `**Implementation:**` lines in spec 013 that were already marked `(planned)` but written in a form `tools/assurance.py:474` does not match -- its regex only accepts `(planned)` immediately after a *single* backticked path, so `` `a`, `b` (planned) `` scored as active and its not-yet-written test links counted as dead. With that fixed, the dashboard is byte-identical to main: 86 active requirements, Completeness 85/86 (99%), Coverage 270/270 (99%), zero dead links, planned 2 -> 10. Refs: 013/Req-1, 013/Req-7
This was referenced Sep 1, 2026
dpsiderius
added a commit
that referenced
this pull request
Sep 4, 2026
…ent (#682) Part 1 found three problems with spec 013 but left the fixes as open design decisions, each with a defensible alternative a spike had no business picking unilaterally. Part 2 builds each alternative and prices it. In all three cases the answer is the better option, not a compromise. 1. Value -> Arc is almost free, and nobody had measured it. ADR-0013/0017 rejected Arc, but they were arguing about the pager; Value was never the subject. The change is +22/-17 across 6 files, because construction sites use `.into()`, which is identical for Rc<str> and Arc<str> — only the ~12 sites that name the type needed editing. Rc<dyn PageSource>/Rc<RefCell<Pager>> untouched, so both ADRs stay intact. 1562 tests pass, and Value becomes Send + Sync. No measurable read-path tax (differences sit inside a few percent of run-to-run variance). The boundary copy then disappears entirely: worker_direct/1024 runs 5.13 ms against worker_chunked/1024's 7.34 ms, matching single-threaded batch because sync_channel(1) pipelines once it stops copying. 2. The ~8 MB memory floor is one constant, not an architecture. Sweeping DEFAULT_PAGE_CACHE_CAPACITY on a 1M-row result: 2000 pages -> 8.68 MB, 256 -> 1.10 MB, 64 -> 291 KB, flat in result size at every setting. Cost of 64 pages is +4.5% on streaming and nothing on batch. So Req 7 needs neither a weakened promise nor a new eviction subsystem — it needs a streaming connection to be allowed a smaller cache. Measured on sequential scans only; random access is the obvious follow-up. 3. Adaptive chunking removes the throughput/latency trade outright. Growing 1,2,4,...,1024: full drain 5.52 ms (fixed-1024 is 5.27 ms, batch 5.26 ms) and first row 50.2 us (unchunked 45.5 us, fixed-1024 357 us). Within 5% of the best throughput and 10% of the best latency simultaneously, so the chunk size needs no knob and stays out of the public API. This also dissolves Part 1's question about which SQE consumer to optimise for — one strategy serves both. Adds worker_direct and worker_adaptive prototypes (both require the Arc-Value patch), extends the agreement gate to cover them, and records all of it in README Part 2. Still branch-only evidence: the Arc change ships via its own ticket and ADR, not from here. Refs: 013/Req-4, 013/Req-7, #682, #678 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 4, 2026
dpsiderius
added a commit
that referenced
this pull request
Sep 4, 2026
…in spec 013 (#678) My reconciliation commit said four of spec 013's claims had stopped being true. There were five. Item 6 read "`Pager` syncs but nothing states what is guaranteed, and `synchronous` has no handler". The second clause was already false when #678 was written: #645 implemented `PRAGMA synchronous` in full — the bare query form reporting `0`/`1`/`2` like stock SQLite, all three levels, and a decided per-level fsync-skip policy recorded in ADR-0036 (`src/vdbe/pragma.rs:79`, dispatched at `src/vdbe/exec.rs:760`, with unit tests). Nothing about it is a stub. That also means Requirement 5 is further along than it claims. It asks the API to "honor `PRAGMA synchronous` at least to distinguish FULL from OFF", which is a weaker ask than what already exists, and says "what is missing is a documented guarantee and any way to trade it" — the trade mechanism is exactly what #645 added. Requirement 5's remaining work is the written guarantee, the transaction surface, and the busy/retryable error handling, not the PRAGMA. I found this while answering "will this work with SQE yet", by checking each of the spec's seven gaps against the tree instead of trusting the list. Worth noting for anyone reviewing #693: the list was written against 0.18.5 and the tree is 0.18.10, so treat every "is missing" line as a claim to re-verify rather than a fact. The four I corrected first were the ones my own branches falsified; this one had been stale for longer and nothing I built touched it. `make check-assurance` unchanged at 86/86 and 276/276, no dead links — Requirement 5 stays `(planned)`, since the requirement as a whole is not discharged even though this part of it is. Refs: 013/Req-5, #645, #678 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Proposal for a public api to support applications to link to sqlite-rs