Skip to content

Skip sketch planning for scoped permission reads - #2909

Merged
stopachka merged 1 commit into
mainfrom
codex/scoped-permission-entity-planning
Sep 13, 2026
Merged

stopachka merged 1 commit into
mainfrom
codex/scoped-permission-entity-planning

Conversation

@stopachka

Copy link
Copy Markdown
Contributor

Simple UUID entity reads during permission checks currently load and decompress attribute sketches to choose an index that is already determined by the query shape. Add a default-off path for one opted-in app that preserves the EA index hint, SQL and parameters while skipping that planning work.

The guard requires the existing permission-fetch entry point and standalone UUID/EA patterns. Other apps, custom query functions, lookup references and more complex shapes retain the existing planner. Permission checks and reads before and after writes are unchanged; an existing-span boolean records when the shortcut runs.

Validation:

  • Unit tests cover exact SQL/parameter equality, app and flag gates, disabled hints, debug mode and fallback behavior.
  • Local PostgreSQL checks cover own-write visibility, denied writes, linked data, lookup permissions and rollback.
  • An independent 240-case planner comparison produced identical query maps, SQL and parameters.
  • A synthetic benchmark with real decompression reduced planning allocation by 98–99%; this is a per-query local measurement, not a production GC claim.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 731d63fe-a3c4-46ef-89b5-48b8cccfa794

📥 Commits

Reviewing files that changed from the base of the PR and between 2cc9ef9 and c0c5f15.

📒 Files selected for processing (4)
  • server/src/instant/db/datalog.clj
  • server/src/instant/db/permissioned_transaction.clj
  • server/test/instant/db/scoped_permission_entity_fetch_integration_test.clj
  • server/test/instant/db/scoped_permission_entity_fetch_test.clj

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds scoped permission entity-fetch planning. Eligible queries receive a context marker and use direct ea_index annotations without sketch lookups. Existing planning remains for ineligible queries, disabled flags, unsupported shapes, debug mode, or non-ea_index selections. Tests cover SQL output and transaction behavior.

Changes

Scoped permission entity fetch

Layer / File(s) Summary
Planning eligibility and annotation
server/src/instant/db/datalog.clj, server/test/instant/db/scoped_permission_entity_fetch_test.clj
Adds app and flag gating, exact query-shape validation, and ea_index annotation using an empty sketch map. Tests cover eligible and invalid shapes.
Specialized query planning
server/src/instant/db/datalog.clj, server/test/instant/db/scoped_permission_entity_fetch_test.clj
Updates query-nested to select specialized planning when the context marker and feature conditions match. Existing hint and unannotated paths remain available.
Context propagation and transaction validation
server/src/instant/db/permissioned_transaction.clj, server/test/instant/db/scoped_permission_entity_fetch_test.clj, server/test/instant/db/scoped_permission_entity_fetch_integration_test.clj
Updates load-entities-map to propagate the marker for standard queries. Tests validate marker propagation, lookup counts, result equivalence, writes, deletes, and mixed reference batches.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant load_entities_map
  participant query_nested
  participant PostgreSQL
  load_entities_map->>load_entities_map: Check scoped permission entity-fetch flag
  load_entities_map->>query_nested: Pass ::d/permission-entity-fetch? context marker
  query_nested->>query_nested: Validate query shape and select ea_index
  query_nested->>PostgreSQL: Execute SQL with ea_index scans
  PostgreSQL-->>load_entities_map: Return entity-fetch results
Loading

Suggested reviewers: dwwoelfel

Merge Risk: ⚪ Minimal · up to c0c5f

The optimization is disabled by default and limited to the opted-in app and narrow query shape, with fallback behavior and read results validated.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the scoped permission-read optimization, its guard conditions, preserved behavior, and validation coverage.
Title check ✅ Passed The title concisely identifies the main change: skipping sketch planning for scoped permission reads.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@stopachka
stopachka merged commit 1065a05 into main Sep 13, 2026
34 checks passed
@stopachka
stopachka deleted the codex/scoped-permission-entity-planning branch September 13, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant