feat(lattice): add ShadowDatabase simulator for the v2 state lattice (closes #657) - #674
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds a ChangesShadow database simulation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant QueryIntent
participant ShadowDatabase
participant QueryMutationResult
QueryIntent->>ShadowDatabase: simulate(intent)
ShadowDatabase->>ShadowDatabase: resolve affected tables and rows
ShadowDatabase->>ShadowDatabase: evaluate invariants
ShadowDatabase-->>QueryMutationResult: return structured result
QueryIntent->>ShadowDatabase: apply(intent)
ShadowDatabase-->>QueryMutationResult: return applied or refused result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
🧪 PR Test Results
Python 3.12 · commit 98cc80d |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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 `@agentwatch/lattice/shadow_database.py`:
- Around line 262-279: Update _resolve_rows so operations in
_WHOLE_TABLE_WHEN_UNQUALIFIED always return table_size with was_estimated=True
before consulting intent.rows_affected, ensuring DROP/TRUNCATE cannot be
overridden by a supplied count. Preserve caller-supplied counts for other
operations, and update test_apply_handles_insert_and_truncate to omit the
explicit count or expect refusal under MAX_ROW_DELETE_PCT.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f739551-7d69-4ed5-b867-a4d8e3653997
📒 Files selected for processing (3)
agentwatch/lattice/__init__.pyagentwatch/lattice/shadow_database.pytests/test_shadow_database.py
|
The one failing check is pre-existing on
I checked out One command fixes it: Worth doing on its own, since the lint job currently fails on every PR regardless of content — which makes it hard to tell a real failure from the standing one. Happy to open that separately rather than bundle unrelated formatting into this change. |
SHAURYASANYAL3
left a comment
SHAURYASANYAL3
left a comment
There was a problem hiding this comment.
Excellent work on the ShadowDatabase simulator! The code is clean and perfectly aligns with the V2 lattice architecture. Thank you!
|
I tried to merge this, but there are merge conflicts. Please resolve the conflicts and update the PR so I can merge it. |
2 participants
Closes #657
Part of #651
Mirrors the structure
ShadowFilesystemestablished in #655 — same module layout, samesimulate/applysplit, same "supplied rather than discovered" stance on external state.Queries are accepted pre-parsed
The issue allows either parsing or accepting pre-parsed intentions. I took the second.
There's no SQL parser in
pyproject.toml, and adding one is a real dependency decision. More to the point, a regex-based parser gets maybe 90% of statements right — and the 10% it misreads are quoted identifiers, subqueries and CTEs, which is precisely the shape a crafted query takes. A simulator that can be fooled by unusual syntax is worse than none, because it produces a verdict people trust.QueryIntentdescribes what a statement would do. A caller with a parser feeds it; a caller with an ORM already has the structured form.Blast radius
rows_affected=Nonemeans "the caller could not determine the reach", and it is treated as the whole table for DELETE, UPDATE, DROP and TRUNCATE.That's the single most important decision here. Reading an unknown as zero is how an unqualified
DELETE FROM usersgets waved through — the risk is identical to an unbounded statement, so the estimate has to be the upper bound.QueryMutationResult.estimatedmarks when a count was derived rather than supplied.The three invariants
max_row_delete_pct— more than 10% of a table removed in one statement. Exactly 10% passes; the check is "more than", not "at least". A table of unknown size yields no verdict rather than a pass: with no denominator there is no proportion, and silence is not approval.protected_tables_immutable— any mutation touching credentials, permissions, billing or audit history, regardless of row count. One altered row inpermissionsis worth more than a thousand inlogs. Reads are allowed. The check also fires when a protected table is reached by cascade rather than named directly.no_cascade_deletes— a cascade into any table outsideapproved_cascade_tables.All violations are reported together rather than short-circuiting, so a caller fixing one doesn't have to re-run to discover the next.
Two details worth flagging
Table names are matched case-insensitively and unquoted. SQL identifiers are case-insensitive unless quoted, so
USERS,"users",`users`and[users]all normalise to one table. An invariant thatUSERSslips past is not an invariant, and there's a test for each form.QueryMutationResult, notMutationResult.shadow_filesystemalready exportsMutationResultfrom this package. Reusing the name would meanShadowDatabase.simulate()returns a type that can't be imported fromagentwatch.lattice— the filesystem one wins the export. Both are lattice results but they carry different fields, so two clear names beat one ambiguous one.Tests
26 tests in
tests/test_shadow_database.py, covering blast-radius resolution, each invariant separately, the multi-violation case,applyfolding results into the model, and the refusal path leaving it untouched.Worth calling out one:
test_apply_leaves_the_model_alone_when_the_query_is_refused. A blocked statement never ran, so pretending its rows are gone would make every later simulation wrong.Verification
python -m pytest tests/test_shadow_database.py tests/test_shadow_filesystem.py— 62 passed, no regressions in the existing lattice suite.ruff checkandruff format --checkboth clean on the new files.The wider suite has pre-existing collection errors from optional dependencies (
celery,litellm,redis,prometheus_client) in my environment. Comparing against unmodifiedmain: identical 116 failures and 14 errors before and after, with passes going 820 → 846 — exactly the 26 tests added here.Summary by CodeRabbit