feat: add exact qpk-vnext N2 sqlite foundation#260
Conversation
🤖 Codex PR Review🚫 Merge blocked: 2 serious issue(s) found in high-risk files ⚖️ Codex Review Arbitration🚫 block: Prior schema finding is fixed and does not conflict: 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🔴 [CRITICAL] Reliability in
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af539563aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| item = decode_wire(data) | ||
| if item.key != key: | ||
| _fail() | ||
| return item | ||
| except (sqlite3.Error, StoreError, TypeError, UnicodeError, json.JSONDecodeError): |
There was a problem hiding this comment.
When a persisted row contains syntactically valid JSON that fails N1 validation, such as b"{}", a bad digest, or a wrong namespace, decode_wire(data) raises ContractError. This handler does not catch that type, so get() leaks the N1 contract exception instead of normalizing corrupted-row reads to StoreError; callers that only handle store failures will see an unexpected exception despite the store's fail-closed/sanitized contract.
Useful? React with 👍 / 👎.
| meta_cols = {row[1] for row in conn.execute("PRAGMA table_info(store_meta)")} | ||
| result_cols = {row[1] for row in conn.execute("PRAGMA table_info(results)")} | ||
| if meta_cols != {"key", "value"} or result_cols != {"key", "payload"}: |
There was a problem hiding this comment.
When an existing non-empty database has the expected table and column names but not the constraints this store creates, for example results(key TEXT, payload BLOB) without PRIMARY KEY/NOT NULL, this check still accepts it. put() then relies on the database uniqueness contract to preserve create-once semantics, so preexisting duplicate keys or nullable payloads can be accepted and later make reads fail or return whichever duplicate SQLite happens to fetch, instead of rejecting the non-exact schema up front.
Useful? React with 👍 / 👎.
| item = decode_wire(data) | ||
| if item.key != key: |
There was a problem hiding this comment.
If an existing or corrupted row contains a valid N1 wire payload with persist_mode="ephemeral", decode_wire() succeeds and item.key still matches because N1 keys intentionally exclude persist_mode. get() then returns an ephemeral contract from the durable store even though put() rejects the same contract before opening the DB, so seeded/corrupted databases can bypass the durable-only invariant instead of failing closed.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
Pigbibi
force-pushed
the
codex/qpk-vnext-n2-sqlite-exact
branch
from
af53956 to
bd8b9d0
Compare
|
Concentrated closure (head |
|
PERMANENT_FREEZE: exact-store closure exhausted; durability/pragma semantics remain deployment-sensitive and no consumer contract exists. Frozen evidence retained; N2 deferred pending consumer. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd8b9d0c62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if item.persist_mode != "durable": | ||
| _fail() | ||
| return item | ||
| except (sqlite3.Error, StoreError, TypeError, UnicodeError, json.JSONDecodeError): |
There was a problem hiding this comment.
When a persisted/corrupted BLOB contains syntactically valid JSON that fails inside the stdlib parser before N1 validation runs, such as an integer over Python's digit limit raising plain ValueError or deeply nested arrays raising RecursionError, this handler does not catch it. In those cases get() leaks a raw runtime exception instead of the store's fail-closed StoreError, so callers that handle store failures can still crash on corrupted rows.
Useful? React with 👍 / 👎.
| def _validate_schema(conn: sqlite3.Connection) -> None: | ||
| try: | ||
| objects = {(row[0], row[1]) for row in conn.execute( | ||
| "SELECT name,type FROM sqlite_master WHERE name NOT LIKE 'sqlite_%'")} |
There was a problem hiding this comment.
When validating an existing database, this LIKE 'sqlite_%' treats _ as a single-character wildcard, so user-created objects such as sqliteXextra or sqlitefoo are filtered out along with internal sqlite_ objects. Fresh evidence is the unescaped LIKE pattern on this line: an otherwise exact database with an extra sqliteX... table still passes validation, bypassing the documented invariant that only store_meta and results may exist.
Useful? React with 👍 / 👎.
1 participant
Fresh exact put/get foundation
Fresh base:
origin/main@a06a99f74566ce60ae91fc39317cdfc16558e325; no code from frozen PR #259 is reused.This KISS N2 slice intentionally contains only:
sqlite3exact durableput/getsqlite3.connect(str(path)), including?/#/%/space/Unicodepaths; no URI interpolationstore_meta(namespace)+results(key PRIMARY KEY,payload BLOB)schemasynchronous=FULLNo selector/list/latest, identity columns, legacy/compatibility/migration, network/S3, caller/orchestrator, or live behavior.
Validation
5 passed619 passed, 1 skippedReview mode: preflight then one concentrated closure maximum; remediation_count=0.