mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
DottaandPaperclip
a30194e008
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task reads use one shared privacy predicate for detail, list, badge, and derived access. > - Its recursive parent join can scan all tasks for each frontier, including tasks with no parent. > - A 1,200-task local reproduction inspects 1,440,000 recursive task rows without finding any parent. > - Each task has at most one same-company parent, so the query can probe that parent directly. > - This pull request exposes that single-parent contract to the planner and keeps the existing access rules. > - The result removes repeated scans and avoids JIT in the measured large callers. ## Linked Issues or Issue Description Refs #14718 and #17973. I searched public issues and open/closed PRs for privacy ancestry and privacy performance work. No competing open fix was found. **What happened?** Task privacy checks repeat company task scans and private-project checks during list and unread badge reads. The recursive join may do this work even when every parent is NULL. **Expected behavior** Read each actual same-company parent through the existing unique task index. Preserve current grants, project membership, user intersections, cycle termination, and visibility before pagination. **Steps to reproduce** Run `authorization-privacy-query.test.ts` against the pre-fix query. The semantic ID check passes. Its plan-work assertion fails at 1,440,000 rows against a 3,600-row bound. The same test passes after this change. **Paperclip version or commit** Measured baseline `8279c990080b15abdcd9981f5793290fab37f9ad`. The branch was replayed onto `71b448b9cb841e6fa18fe883585430c5081245da`. Both have identical pre-fix authorization, issue-service and participation-helper sources. Treatment source SHA-256 is `a99faf974070a35f2786c92407e023c668c1f7b99b6604f6281c89bce089b1a8`. **Deployment mode** Source checkout, isolated local PostgreSQL 17.11 on ARM64 Linux. No hosted write, migration, merge, or deployment was performed. ## What Changed - Skip NULL privacy parents. Use a lateral lookup for the exact same-company parent through existing indexes. - Keep `UNION` cycle detection and every existing principal, grant, private-project and membership expression. Both `LIMIT 1` clauses are redundant: one anchor and one unique parent give at most one row per recursive generation. The query still follows a complete deep chain. - Cover open ancestors inside private projects, including current project visibility and ancestor grant revocation. Place the benchmark under `server/scripts/` with ordinary package imports. - Add real PostgreSQL regressions and a pinned-baseline benchmark for complete badge/list calls. Keep all 36 measured cells, ranges, neutral paths, failed experiments, and the single-chain proof in [the query proof](https://github.com/paperclipai/paperclip/blob/codex/perf-task-privacy-ancestry/doc/plans/2026-10-28-task-privacy-query.md). ## Verification Local warm full-service medians, with two warmups and five alternating measured calls per arm: | Scoped caller | Baseline | Treatment | Ratio | | --- | --- | --- | --- | | Dense badge, default JIT | 645.33 ms | 77.11 ms | 8.37x | | Dense list, default JIT | 214.77 ms | 29.87 ms | 7.19x | | Sparse list, default JIT | 117.92 ms | 13.22 ms | 8.92x | | Dense list, JIT off | 55.69 ms | 30.62 ms | 1.82x | | Sparse list, JIT off | 16.61 ms | 15.12 ms | 1.10x | All 36 cases match every positional SQL field and row order across all captured queries. Complete final badge IDs and list objects also match. Both badge phases and ordinary list enrichment remain in the measured caller. Dense/sparse fixtures have 1,200 tasks, 300,000 activity rows and 24,000 comments, with wide random content. Small, unrestricted, private fan-out, 64-level ancestry, cycle, and JIT on/off controls are included. The dense ranking plan reduces recursive task and project work from 720,000 visits each to zero. Its cost estimate falls from 103,000.61 to 20,458.73 and JIT disappears. Separate EXPLAIN execution falls from 524.04 to 34.34 ms. The dense JIT-off main list statement falls from 29.52 to 1.89 ms; remaining service work limits the full-call gain. **Retained failed goals:** `--assert-performance --max-dense-ms=60` exited 1. The original 60 ms dense badge goal remains unmet at 77.11 ms. Four full-service cases also miss the exploratory blanket 2x goal. These criteria remain in the harness and are not counted as green. The delivered optimization is proved by the deterministic query-work red/green test and complete result equality. Small and unrestricted controls meet their original 25% or 2 ms allowance. All 12 unrestricted cells use byte-identical SQL and parameters in both arms; their overlapping timing variation is neutral/inconclusive. Modest slower scoped controls are retained, including small JIT-off list 16.18→18.78 ms and cycle JIT-off list 14.78→16.29 ms. There is no claim that all paths improve. - Final head `328078f494d6ec585936fb75a9410ad39eefd099`: full CI passes (55 success, 1 neutral, 2 skipped); fresh Greptile is 5/5 with zero unresolved review threads. The optional Paperclip Review ended neutral because its worker stopped; its earlier two specific recommendations are both addressed below. - Final-runtime focused Docker PostgreSQL run: **44/44 tests passed** across ancestry work, production privacy, unread badge, and creation visibility suites. Includes fresh revocation, private projects, grant direction, three-principal agent intersections, cycles, deep inheritance, foreign-company parents, and access before LIMIT. - `pnpm -r typecheck` and `pnpm build`: passed before replay. The real PostgreSQL ancestry suite passed after replay (5/5 tests, 17.44 seconds). The added private-project ancestor case then passed in the six-test suite (43.04 seconds). Database and server typechecks and builds passed after replay; review follow-ups change tests and benchmark location only. A small relocated-command smoke preserves byte-identical generated SQL and complete result comparisons. - `pnpm test:run`: attempted, then stopped with exit 143 after 16 native database bootstrap failures in 940 executor cases. An independent empty native `initdb` fails with `semget` semaphore exhaustion before application SQL. This local full suite is incomplete, not passed. All 16 affected cases then passed on real PostgreSQL 17 Docker in 155.04 seconds (924 other cases were intentionally filtered out). - An earlier focused command passed 35 tests but skipped eight after a 20-second database setup timeout. The later 44/44 pass resolves that coverage gap; the failed command is retained. - The first CI e2e shard attempt completed functional split-handoff assertions, then cleanup run cancellation hit a one-second row-lock timeout. The unchanged local-trusted cleanup path does not use the changed privacy predicate. A single same-head job retry passed. The original failure is retained. - The first lateral-only draft removed scans but raised estimated ancestry rows from 11 to 101 and retained JIT. The per-generation fence fixes that estimate. No failed attempt was discarded. Reproduce the measured caller pair: ```sh pnpm --filter @paperclipai/server exec tsx scripts/benchmark-task-privacy.ts \ --baseline-ref=71b448b9cb841e6fa18fe883585430c5081245da \ --evidence-dir=/tmp/task-privacy-proof ``` The full matrix takes about 107 seconds locally, including 8 seconds of setup. Capture and EXPLAIN run outside ordinary timing samples. Only the old authorization module changes between arms; caller source and all relevant query helpers remain constant. Hosted request latency remains unverified. ## Risks - The per-generation bound depends on the single anchor and unique scalar parent contract. A future multi-parent model must revise this query and its deep/cycle tests. - Tiny tables can still use a cheap sequential lookup for a real parent. The guard prevents broad scans for absent parents; it does not force index use everywhere. - The 60 ms badge goal and blanket full-service 2x goal remain failed. Unchanged enrichment, application work and local variation remain visible in the measurements. - This changes shared read authorization SQL. No permissions are cached, no authorization moves after LIMIT, and no schema or startup change is included. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, local code execution and real PostgreSQL profiling. The exact model ID and context-window size are not exposed by this session. No more specific ID or window size is claimed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>