mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 14:02:36 +01:00
9fb06afbfab284f8dcecf6ebc3dee3bfcda4e09d
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandPaperclip
|
00fea0da52 |
refactor(perf): reuse database test setup for profile benchmark (#18683)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tool profiles govern the tools that agents can use. > - Profile listing and app attention shared nested selector scans that consumed the server CPU. > - #18681 and #18673 replaced those scans with indexes for each computation. > - Their performance benchmarks need consistent setup and reproducible per-service measurements. > - This pull request uses the shared helper entry point and retains the exact measurement harness and results. > - Reviewers can inspect both the CPU savings and the limits of the evidence. ## Linked Issues or Issue Description Refs #18681 and #18673. **What existing behavior does this improve?** The consistency of benchmark setup and the reproducibility of evidence for the two merged CPU fixes. **Current behavior** The profile-review benchmark imports the database starter directly. The matching benchmark uses the shared server test-helper entry point. On the current base, that helper re-exports the database starter, which already supports `PAPERCLIP_TEST_POSTGRES_ADMIN_URL`. The previous committed benchmark measures both services together; the per-service measurements and separate HTTP/Inspector runs lacked a committed harness. **Proposed behavior** Use the same test-helper entry point in both benchmarks. Retain the compile, per-service timing, Inspector, and HTTP-probe programs with their exact commands and raw samples. Preserve the existing embedded and admin-server database behavior. **Reason and benefit** A shared entry point keeps benchmark setup consistent. The committed harness lets reviewers reproduce and compare each real owning service before and after the merged fixes. The failure-path checks verify that result-write errors still release the database and probe resources. **Breaking changes** None. This change affects benchmark setup and documentation. ## What Changed - Import the database starter from the repository test helper in `benchmark-profile-review-projection.ts`. - Record separate CPU and elapsed measurements for profile listing and app attention. - Retain the actual compile, per-service measurement, Inspector capture, and separate-process HTTP probe harness. - Always tear down the test database, HTTP server, and probe even when result writing fails. - Retain all timing and responsiveness samples as sanitized JSON. - State the runtime, fixture, source revisions, and limits of the local comparison. ## Verification - 24 focused tests passed across selector matching, pending projection, review-service freshness, and effective assignments. - Four selected real database-backed service/route tests passed for summaries, review changes, auto-allow behavior, and company isolation. The other 494 tests in that file were not selected. - Injected an `EISDIR` result-write failure into both profiling programs on an owned local admin server. Each child exited with the expected error and removed its disposable database. The responsiveness child also terminated its probe/server handles. - The changed benchmark completed with a configured local PostgreSQL admin URL. Its full responses matched the baseline, and query/row counts matched. - The exact documented compile, benchmark, and responsiveness commands completed from the committed harness at `ca458baddc6461fb6c888984246e329e2bb4bdfe`. The JSON records that build revision and the four owner source hashes. - Five alternating samples per service and implementation used Node 24.22.2 with compiled ES modules. No runtime TypeScript loader ran in those measurements. - Median process CPU from the committed harness: profile listing 3,164 ms to 385 ms; app attention 2,466 ms to 221 ms. Both versions returned identical ordered results and made seven queries per service. - Separate Inspector profiles confirmed that the repeated scope and summary scans were removed. Three separate-process HTTP probe trials per service showed less event-loop blocking. These are local synthetic service results, not hosted page-load acceptance. - Initial local setup failed on missing embedded PostgreSQL library aliases and an unbuilt plugin SDK. Restoring local aliases and building the SDK resolved those setup failures. The original failed setup logs were retained; the 28 selected tests then passed. - The merged summary fix CI run [41535304968](https://github.com/paperclipai/paperclip/actions/runs/41535304968) hit two 15-second timeouts in the unchanged attention-retention history cases. The server shard passed unchanged on one failed-jobs-only retry. Original and retry logs are retained. The compared runs used different runners and synthetic merge trees, so the original timeout cause is not established. No timeout or assertion was changed. - Full repository typecheck, test matrix, and build passed in CI on final head `2f484ed4284f3fa632d8e73d9be084827d1c987e`: 54 successful check runs, three expected skips, one non-blocking Paperclip review, and a successful Snyk status. No failures or pending checks remain. Those broad commands were not rerun locally for this benchmark-tooling follow-up; the 28 selected local regressions and the actual profiling/failure-path programs ran locally. - Fresh Greptile is 5/5 on that exact head. Both review P2 threads are resolved. The Paperclip review's current-base documentation note is corrected in this description; its optional tooling suggestions remain outside this bounded change. ## Risks - The test helper creates a unique disposable database on the configured admin server and removes it afterward. The operator must use a database server where those test operations are appropriate. - Synthetic data and shared-host activity affect absolute timings. The measurements do not establish a production latency percentage or capacity limit. - Production behavior is covered by the two linked merged fixes. This follow-up changes only benchmark setup and documentation. ## Model Used OpenAI GPT-6 through Codex, with code execution and repository tools. The exact runtime model revision and context window were not exposed in this session. The original linked performance fixes record their own model details. ## 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> |
||
|
DottaandPaperclip
|
d0ffa179b4 |
perf(tools): index profile selectors per resolution (#18673)
## Thinking Path > - Paperclip manages AI agents and their work. > - Core resolves tool profiles before it prepares runtime MCP assignments. > - A selected profile can contain a rule for each individual tool. > - The resolver and profile summary scan these rules for each catalog row. > - This repeats millions of exact comparisons for a large selected profile. > - This pull request indexes each selector kind once per computation. > - The benefit is less server CPU with the same current access decisions and result contract. ## Linked Issues or Issue Description **What existing behavior does this improve?** Effective tool assignment and profile summary computation for large selected profiles. **Current behavior** Each catalog row scans the profile's includes and excludes. The work grows with catalog size times rule count. The runtime assignment path also computes a profile summary with the same scans. **Proposed behavior** Build five selector sets for each include or exclude group. Match catalog rows with exact set membership. Retain all current database reads and the full result contract. **Reason and benefit** Reduce repeated JavaScript comparisons. On the synthetic PostgreSQL fixture below, median process CPU per resolution falls from 50.068 ms to 10.129 ms. This is independent of [#18371](https://github.com/paperclipai/paperclip/pull/18371), which removes unused administrative results and narrows instruction descriptors. This PR retains those results and does not include that change. ## What Changed - Index application, connection, catalog-entry, tool-name, and risk selectors in the profile owner. - Use indexed matching in effective access resolution and profile summaries. - Add a differential test against the original single-rule matcher. Cover null applications, exact special-character names, unknown selectors, and empty rule groups. - Add a PostgreSQL regression for per-profile exclusion precedence, the union of multiple effective profiles, and current rule removal. - Add a real PostgreSQL resolver benchmark and document reproduction and limits. ## Verification Baseline: `410b0e27015495ab8951768b5b0b94727de33785`. Treatment: `895ee2eec15cf4cdedee4c257fcdddb2a7000a1b`. Node 24.22.2 on macOS arm64. Source modules run with `tsx`; these are local service measurements, not production-build or hosted acceptance. The real owning resolver ran against disposable PostgreSQL with 1,890 active catalog tools, 1,575 individual include/exclude rules, one deny-by-default effective profile, and 12 agents. The installation dependency returns an empty list because the fixture has no installed connections. There are no provider calls. Full returned assignments, profile summaries, rules, bindings, and names are identical to baseline. After two warmups per version, seven alternating samples ran ten resolutions each. Other workers paused CPU measurements and builds. Unrelated host work was not controlled. | Per resolution | Baseline median (range) | Change median (range) | | --- | --- | --- | | Process CPU | 50.068 ms (46.380–58.880) | 10.129 ms (8.089–11.072) | | Elapsed time | 50.362 ms (44.799–106.408) | 8.853 ms (7.719–18.590) | | Queries | 6 | 6 | | Returned database rows | 3,480 | 3,480 | Median process CPU falls 79.8%; median elapsed time falls 82.4%. The long baseline elapsed sample had no matching process CPU increase. Query and row counts do not decrease. Small or broadly allowed profiles can save less. This does not establish whole-run latency or hosted performance. Reproduce with `server/scripts/benchmark-tool-profile-matching.ts`; baseline module preparation is in `doc/mcp-discovery-performance.md`. Passed on the treatment source: ```sh node node_modules/vitest/vitest.mjs run server/src/__tests__/tool-profile-matching.test.ts server/src/__tests__/tool-effective-assignments.test.ts server/src/services/native-runtime/runtime-context.test.ts server/src/__tests__/heartbeat-runtime-mcp-servers.test.ts ``` All 45 tests in four suites passed. `git diff --check` passed. The first PostgreSQL test attempt failed at setup because the sandbox blocked its loopback listener (`EPERM`); the authorized unsandboxed retry passed. No behavior failure was discarded. At measured head `895ee2eec15cf4cdedee4c257fcdddb2a7000a1b`, CI typecheck, build, canary, Runner checks, and all eight browser shards passed. Server general shard 8/12 failed an unchanged background-quietness assertion in `heartbeat-run-dispatch-work.test.ts`: expected 235 select calls, observed 236 after advancing fake timers. Its cause remains unverified; no baseline reproduction was run. That head also failed the module-boundary policy because the benchmark inserted synthetic agents outside the test fixture tree. Follow-up head `e20165b6eb` moves the fixture body byte-for-byte into `server/src/__tests__/helpers/tool-profile-matching-fixture.ts`, following the existing listing benchmark pattern. The production matcher and benchmark measurement loop are unchanged. `node scripts/check-module-boundaries.mjs`, a Node 24/tsx fixture import check, and `git diff --check` pass. The benchmark was not rerun for this extraction. New-head CI and review are pending. Greptile gave the measured head 5/5 with no actionable issues. No deployment or live runtime mutation was performed. ## Risks Exact equality must remain identical to the original matcher, especially for null application selectors and exclusions. The differential and PostgreSQL tests cover these cases. Sets add linear temporary memory in rule count. They live only for the current computation. Current company-scoped state reads, profile precedence, revocation, and runtime assignment digest checks remain in place. Revert this commit to roll back. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) with source inspection, code execution, tests, and local PostgreSQL benchmarking. The context window size is not available in this session. ## 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 - [ ] All Paperclip CI gates are green - [ ] 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> |
||
|
DottaandPaperclip
|
482706e7dd |
perf(tools): index pending profile review scopes (#18681)
## Thinking Path > - Paperclip manages AI agents and their work. > - The Apps UI lists tool profiles and shows new tools that need review. > - Both services project new-tool review for each profile. > - The projection scans include rules and exact rules for each catalog row. > - This repeats millions of comparisons when many selected profiles exist. > - This pull request builds scope and selector indexes once per projection. > - The benefit is less server CPU with the same fresh review results. ## Linked Issues or Issue Description **What existing behavior does this improve?** New-tool review projection in profile listings, profile review, and app attention. **Current behavior** Each newer catalog row scans the profile's included scopes. Included catalog entries require a map lookup for each scope comparison. Rows in scope then scan all rules again to omit tools that were already reviewed. Profile listing and app attention repeat this work for every profile. **Proposed behavior** Index covered application and connection scopes once. Expand included catalog entries to their existing connection and non-null application. Index exact selectors across all rules once. Keep the ordered catalog filter and returned metadata. **Reason and benefit** A recent main-thread profile identified this scope-matching path as a large CPU consumer. The local reproduction below confirms multi-second CPU work in the actual owning services. This PR removes the repeated scans without caching authorization or changing database reads. Related [#18673](https://github.com/paperclipai/paperclip/pull/18673) indexes effective access and profile summaries. It leaves this pending-review path unchanged. This PR targets the separate pending-review mechanism and has no dependency on that PR. The selector helper is identical so the owners can share it when both changes land. ## What Changed - Build covered application and connection sets for included scopes. - Build exact selector sets for all rules, including exclusions, to omit already reviewed tools. - Preserve null application selectors, missing catalog references, review watermarks, status checks, output order, and names. - Add differential tests against the former scope/matching evaluator. - Add PostgreSQL checks through all three affected service methods, including current catalog and rule changes. - Add a reproducible PostgreSQL benchmark of complete profile-list and app-attention results. ## Verification Baseline: `1f8f2e195f4897cd1c237bdc068de95780ab0c3c`. Treatment: `fd59c0183ce64c44696f79604516f4d51ca5662b`. Node 24.22.2 on macOS arm64. The production projection file at this baseline is identical to the file at `410b0e27015495ab8951768b5b0b94727de33785`. The benchmark exercises the real `createToolProfileService(...).listProfiles` and `createToolAppAttention(...)` methods against disposable PostgreSQL. It uses 200 selected profiles, 1,900 active tools, 76,000 rules, and 12 agents. Profiles alternate between 65 and 695 rules. Half reference 695 removed catalog entries; half select current tools. This tests both the slow failed scope scans and the slow exact-rule scans. No provider calls or hosted traffic are generated. The complete ordered results from both services match baseline. After one equality/warmup invocation per version, five samples alternate implementation order. Forced GC runs before each measured pair of service calls. Other workers paused owned builds and benchmarks; unrelated host activity was not controlled. | One profile listing plus one attention listing | Baseline median (range) | Change median (range) | | --- | --- | --- | | Process CPU | 5,406.512 ms (5,221.533–5,648.262) | 1,770.116 ms (1,596.232–2,045.617) | | Elapsed time | 5,279.231 ms (4,932.419–5,537.105) | 1,532.414 ms (1,499.199–1,610.428) | | Queries | 14 | 14 | | Returned database rows | 156,215 | 156,215 | Median process CPU falls **67.3%** and median elapsed time falls **71.0%**. Profile summaries retain their old nested matching in this comparison, so the result isolates pending-review indexing. Query counts and row counts do not decrease. This is local source/tsx service proof. It does not establish hosted improvement, whole-run latency, or HTTP tail latency. Reproduce with `server/scripts/benchmark-profile-review-projection.ts`; baseline-module setup is documented in `doc/mcp-discovery-performance.md`. Passed: ```sh node node_modules/vitest/vitest.mjs run server/src/__tests__/tool-profile-pending-projection.test.ts server/src/__tests__/tool-profile-review-service.test.ts server/src/__tests__/tool-profile-entry-order.test.ts node scripts/check-module-boundaries.mjs ``` All five tests in three suites passed. The differential tests cover all selector kinds, include/exclude effects, null scope behavior, missing references, review watermarks, and exact special-character names. PostgreSQL checks preserve current pending counts after exclusions, catalog removal, and review; another company remains separate. `git diff --check` passes. The first new integration assertion used `summary` instead of the existing attention response's `totals` field. It failed before the mutation checks. The test assertion was corrected; no production code changed for that correction. The final run above passed. Server typecheck was attempted but did not pass local setup. The packaged command did not reach TypeScript because a global pnpm child tried to fetch and verify the project's pnpm version through an unavailable registry. A direct Node 24 TypeScript invocation then reported missing built `@paperclipai/paperclip-runner` and related package declarations, with cascading type errors in unchanged consumers. No error points to the changed projection. The prerequisite build was not expanded into a full workspace build. Full local unsharded tests and build were not run; final-head CI must supply broad acceptance. No merge or deployment was performed. ## Risks Scope expansion must preserve existing semantics. An individual catalog rule covers its connection and only a non-null application. An explicit null application selector still matches null applications. Removed references contribute no scope when absent from the supplied catalog. The oracle and PostgreSQL tests cover these boundaries. The indexes add linear temporary memory in rule count and live only for one projection. Current company-scoped database reads and review mutations remain in their owners. Exclusions remain explicitly reviewed; they do not create new review items. Revert this commit to roll back. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) with source inspection, code execution, tests, and local PostgreSQL benchmarking. The context window size is not available in this session. ## 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 - [ ] All Paperclip CI gates are green - [ ] 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> |
||
|
|
cad26c6bfb |
fix(tool-gateway): bound MCP discovery memory and concurrency (#14864)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents discover governed tools through the MCP gateway. > - A listing repeated policy and full run-row reads for each catalog tool. > - Parallel listings multiplied those allocations during run startup. > - One 900-tool baseline listing used 11,489 queries and about 1.7 GiB of extra heap in a fixture. > - This pull request shares reads within a listing and bounds whole listings across the process. > - The benefit is lower discovery memory use while execution still checks current policy. ## Linked Issues or Issue Description Refs #13115. Its on-demand target change affects the same listing loop. **What happened?** MCP discovery repeated roughly 13 reads per tool. Full run snapshots and repeated connection configurations caused large allocations. Per-listing bounds alone did not limit concurrent listings across gateways. **Expected behavior** Discovery reads shared inputs once per listing. The process bounds active and queued listings. Catalog payload size and policy evaluation still grow with the catalog. Tool execution checks current access rules. **Steps to reproduce** Create a company with a remote MCP connection, 900 catalog tools, large schemas, and a large run snapshot. Send concurrent tools/list requests using a run-bound gateway token. Run the committed benchmark for a deterministic reproduction. **Paperclip version or commit** Baseline: |