Skip to content

feat: phase 22 — performance baselines completion#355

Merged
FSM1 merged 28 commits into
mainfrom
feat/phase-22-performance-baselines-completion
Mar 25, 2026
Merged

feat: phase 22 — performance baselines completion#355
FSM1 merged 28 commits into
mainfrom
feat/phase-22-performance-baselines-completion

Conversation

@FSM1

@FSM1 FSM1 commented Mar 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • SDK Performance Instrumentation: Added withPerf wrapper using browser Performance API (environment-gated via PERF_ENABLED), instrumented 11 sdk-core functions across upload, download, IPFS, IPNS, and folder modules
  • E2E Journey Timing: Playwright test suite measuring wall-clock time for 3 critical user journeys (login-to-vault, upload-to-visible, share-to-accessible) with structured JSON output
  • Load Test Thresholds: checkThresholds module with expectThresholdsPassed helper integrated into all 5 load test scenarios, plus 347-line capacity model document
  • Code review cleanup: Fixed PerformanceMeasure memory leak, extracted duplicated threshold assertion boilerplate, removed unused fields/imports

Requirement Coverage

ID Requirement Implementation
PERF-05 SDK instrumentation packages/sdk-core/src/perf.ts + 5 instrumented modules
PERF-06 Journey timing baselines tests/web-e2e/tests/journey-timing.spec.ts
PERF-07 Load test thresholds tests/load/src/harness/thresholds.ts + 5 scenarios
PERF-08 Capacity model docs/CAPACITY.md

Test plan

  • sdk-core unit tests pass (34/34)
  • Load test threshold unit tests pass (5/5)
  • Run journey timing tests against live services to populate baselines
  • Verify Performance API marks visible in browser DevTools

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Automated performance regression detection added to load tests (threshold-based pass/fail).
  • Tests

    • End-to-end journey timing suite added to capture login/upload/share timings and emit structured timing output.
    • Load-test scenarios now validate p95 latency and error-rate thresholds.
  • Documentation

    • Added capacity planning, performance baselines, phase completion summaries, and baseline capture guidance.

FSM1 and others added 20 commits March 25, 2026 01:50
Entire-Checkpoint: 76b95ac159b3
Entire-Checkpoint: 35000a89c030
Entire-Checkpoint: e4a7fc6a9315
Entire-Checkpoint: d83247189e1d
- 6 test cases covering withPerf, markStart, markEnd
- Tests verify env gating, mark cleanup, error propagation
- All tests fail (RED phase) — perf.ts not yet created

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 33c88d096f16
… wrappers

- PERF_ENABLED constant evaluated at module load (no-op in production)
- markStart/markEnd create and clean up performance marks
- withPerf async wrapper for transparent instrumentation
- __CIPHERBOX_PERF__ global opt-in for production debugging
- All 6 unit tests pass

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: ba944ef18fb0
- 5 test cases covering pass/fail, p95 violations, error rate violations
- Tests skip operations not in metrics (no false positives)
- Violation messages include operation name, observed, and threshold values

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 197eddfb5b35
- Login-to-vault: mock wallet auth through vault load with phase breakdown
- Upload-to-visible: 100KB file upload through file list appearance
- Share-to-accessible: share creation through recipient access with phase breakdown
- All timings use performance.now() with JOURNEY_TIMING: JSON output prefix
- Graceful failure handling for multi-account share journey

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: ef87d5191af2
- ThresholdConfig type with operation, p95MaxMs, errorRateMax fields
- checkThresholds function compares metrics against limits
- Skips operations not in metrics, returns descriptive violations
- All 5 unit tests pass

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 35feaa95419d
- Login-to-vault, upload-to-visible, share-to-accessible sections
- Capture information table with environment details
- PENDING markers for values to be filled after test execution
- How to Capture section with playwright command and grep pattern
- Historical comparison table for SDK vs E2E overhead analysis

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 946420b9dd05
- upload-throughput: uploadFile p95 <= 10s, error rate <= 5%
- mixed-workload: uploadFile p95 <= 10s, createFolder p95 <= 5s, error <= 10%
- ipns-publish-storm: createFolder p95 <= 10s, error <= 10%
- sustained-load: uploadFile p95 <= 10s, createFolder p95 <= 5s, error <= 5%
- spike-test: burst phase uploadFile/createFolder p95 <= 15s, error <= 15%
- Thresholds are 2-3x observed Phase 19.2 baselines to avoid CI flakiness

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 4c58196e9d12
…appers

- upload:full, download:full (upload/download modules)
- ipfs:upload, ipfs:download (IPFS module)
- ipns:publish, ipns:batch-publish, ipns:resolve (IPNS module)
- folder:fetch-decrypt, folder:load, folder:update-publish (folder module)
- All function signatures and return values unchanged
- All 34 existing tests pass, build succeeds

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: cfaa84c37d6f
- SUMMARY.md with self-check passed
- STATE.md updated with plan progress and metrics
- ROADMAP.md updated with phase 22 progress
- REQUIREMENTS.md marks PERF-06 complete

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 4a516da82d28
- Observed limits from Phases 18, 19, 19.2 with real baseline data
- Infrastructure bottleneck analysis (Kubo pin ~95% of upload time)
- Scaling recommendations with trigger thresholds and actions
- Growth projections with formulas for storage, IPNS names, DB rows
- Cost estimates for 100/1,000/10,000 user deployments
- Load test threshold reference table linked to thresholds.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: c54dcb4a1bc8
- SUMMARY.md with task commits and self-check
- STATE.md updated with metrics and decisions
- ROADMAP.md progress updated
- PERF-05 requirement marked complete

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: a332b0bbb7b0
- 22-03-SUMMARY.md with task commits and self-check
- STATE.md updated: plan 3/3, progress 100%, metrics, decisions
- ROADMAP.md updated: phase 22 complete (3/3 plans)
- REQUIREMENTS.md: PERF-07 and PERF-08 marked complete

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 8facff4bded9
Entire-Checkpoint: d56174c486b8
- Fix PerformanceMeasure leak: clear measures after creation in perf.ts
- Extract expectThresholdsPassed() helper, replace 5 identical 8-line blocks
- Instrument unpinFromIpfs with withPerf for consistency
- Remove unused partial field and tighten account scope in journey-timing
- Remove unused imports (expect, PrivateKeyAccount)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: f8f1237175c6
@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a2b50c04-af88-445b-8d57-a6607549ded9

📥 Commits

Reviewing files that changed from the base of the PR and between e730262 and b778925.

⛔ Files ignored due to path filters (1)
  • packages/api-client/CHANGELOG.md is excluded by !packages/api-client/**
📒 Files selected for processing (17)
  • .planning/PROJECT.md
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • CHANGELOG.md
  • apps/desktop/src-tauri/tauri.conf.json
  • crates/api-client/CHANGELOG.md
  • crates/core/CHANGELOG.md
  • crates/crypto/CHANGELOG.md
  • crates/fuse/CHANGELOG.md
  • crates/sdk/CHANGELOG.md
  • packages/core/CHANGELOG.md
  • packages/crypto/CHANGELOG.md
  • packages/sdk-core/CHANGELOG.md
  • packages/sdk-core/src/ipfs/index.ts
  • packages/sdk-core/src/upload/index.ts
  • packages/sdk/CHANGELOG.md
✅ Files skipped from review due to trivial changes (13)
  • crates/fuse/CHANGELOG.md
  • packages/core/CHANGELOG.md
  • crates/sdk/CHANGELOG.md
  • crates/core/CHANGELOG.md
  • packages/sdk-core/CHANGELOG.md
  • crates/api-client/CHANGELOG.md
  • apps/desktop/src-tauri/tauri.conf.json
  • crates/crypto/CHANGELOG.md
  • CHANGELOG.md
  • packages/sdk/CHANGELOG.md
  • packages/sdk-core/src/ipfs/index.ts
  • packages/crypto/CHANGELOG.md
  • .planning/STATE.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • .planning/PROJECT.md
  • packages/sdk-core/src/upload/index.ts
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md

Walkthrough

Phase 22 (Performance Baselines Completion) is marked complete. The PR adds SDK client-side performance instrumentation (perf.ts) and applies it to multiple sdk-core operations, introduces Playwright journey-timing E2E tests, implements load-test threshold checks, and adds a capacity model and extensive Phase 22 planning/verification documentation.

Changes

Cohort / File(s) Summary
Planning & Roadmap Metadata
.planning/PROJECT.md, .planning/REQUIREMENTS.md, .planning/ROADMAP.md, .planning/STATE.md
Marked Phase 22 complete (2026-03-25); updated PERF-05..PERF-08 to complete; bumped progress counters and last-updated lines.
Phase 22 Phase Docs
.planning/phases/22-performance-baselines-completion/...
Added plans, research, context, validation, verification, and per-plan summaries (22-01..22-03) describing instrumentation, E2E timing, thresholds, and capacity modeling.
Journey Baselines Template
.planning/baselines/22-journey-baselines.md
New Playwright journey-baseline template capturing capture metadata, three journey tables, raw JSON lines, and recapture instructions.
SDK Perf Module & Tests
packages/sdk-core/src/perf.ts, packages/sdk-core/src/__tests__/perf.test.ts
New perf module exporting PERF_ENABLED, markStart, markEnd, withPerf (env- and API-gated) plus unit tests covering gating, measures, cleanup, error propagation, and return passthrough.
SDK Function Instrumentation
packages/sdk-core/src/download/index.ts, packages/sdk-core/src/upload/index.ts, packages/sdk-core/src/folder/index.ts, packages/sdk-core/src/ipfs/index.ts, packages/sdk-core/src/ipns/index.ts
Wrapped exported async function bodies in withPerf(...) (e.g., upload:full, download:full, ipfs:*, ipns:*, folder:*). Signatures unchanged; internal logic preserved but executed via the perf wrapper.
E2E Journey Timing Tests
tests/web-e2e/tests/journey-timing.spec.ts
New Playwright serial suite measuring three journeys (login-to-vault, upload-to-visible, share-to-accessible) using performance.now() and emitting JOURNEY_TIMING: JSON lines with sanity assertions and cleanup.
Load Test Thresholding Module & Tests
tests/load/src/harness/thresholds.ts, tests/load/src/harness/thresholds.test.ts
Added ThresholdConfig/ThresholdResult, checkThresholds and expectThresholdsPassed with tests validating p95 and error-rate violation detection and formatted messages.
Scenario Integrations (Load)
tests/load/src/scenarios/*.test.ts
tests/load/src/scenarios/ipns-publish-storm.test.ts, .../mixed-workload.test.ts, .../spike-test.test.ts, .../sustained-load.test.ts, .../upload-throughput.test.ts
Each scenario now captures aggregated metrics and invokes expectThresholdsPassed(metrics, THRESHOLDS) with scenario-specific p95/error-rate limits.
Capacity Model & Docs
docs/CAPACITY.md
New capacity/performance model documenting observed metrics, bottlenecks (e.g., Kubo pinning), scaling recommendations, thresholds reference, growth projections, and operational notes.
Changelog & Tauri Config Formatting
CHANGELOG.md, apps/desktop/src-tauri/tauri.conf.json, multiple */CHANGELOG.md files
Minor markdown reformatting across changelogs and compacted JSON array formatting in tauri.conf.json (no behavioral changes).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: phase 22 — performance baselines completion' clearly summarizes the main change: completing Phase 22 of the roadmap by delivering performance baseline infrastructure (SDK instrumentation, journey timing, load test thresholds, and capacity model).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/phase-22-performance-baselines-completion

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 and usage tips.

@codecov

codecov Bot commented Mar 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 70.21%. Comparing base (d2ef0c5) to head (fd80a77).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/sdk-core/src/perf.ts 97.14% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #355       +/-   ##
===========================================
+ Coverage   56.55%   70.21%   +13.66%     
===========================================
  Files         115      104       -11     
  Lines        8342     6138     -2204     
  Branches      748      892      +144     
===========================================
- Hits         4718     4310      -408     
+ Misses       3445     1624     -1821     
- Partials      179      204       +25     
Flag Coverage Δ
api 83.54% <97.14%> (+0.65%) ⬆️
api-client 83.54% <97.14%> (+0.65%) ⬆️
core 83.54% <97.14%> (+0.65%) ⬆️
crypto 83.54% <97.14%> (+0.65%) ⬆️
desktop 15.88% <ø> (-15.34%) ⬇️
sdk 83.54% <97.14%> (+0.65%) ⬆️
sdk-core 83.54% <97.14%> (+0.65%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/sdk-core/src/perf.ts 97.14% <97.14%> (ø)

... and 50 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Completes Phase 22 performance baselines work by adding SDK-level Performance API instrumentation, Playwright E2E journey timing output, load-test threshold assertions, and capacity planning documentation.

Changes:

  • Added withPerf / markStart / markEnd utilities and wrapped key sdk-core operations (upload/download/IPFS/IPNS/folder) for timing visibility.
  • Introduced Playwright “journey timing” spec that outputs structured JOURNEY_TIMING: JSON lines for baseline capture.
  • Added load-test threshold checking utilities + unit tests, integrated thresholds into all load scenarios, and documented capacity baselines in docs/CAPACITY.md.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/web-e2e/tests/journey-timing.spec.ts Adds 3 timed E2E journeys with structured JSON output.
tests/load/src/scenarios/upload-throughput.test.ts Runs aggregate report and asserts p95/error-rate thresholds.
tests/load/src/scenarios/sustained-load.test.ts Adds scenario-specific threshold assertions.
tests/load/src/scenarios/spike-test.test.ts Adds burst-phase threshold assertions for spike scenario.
tests/load/src/scenarios/mixed-workload.test.ts Adds threshold assertions for mixed workload operations.
tests/load/src/scenarios/ipns-publish-storm.test.ts Adds threshold assertions for IPNS contention scenario.
tests/load/src/harness/thresholds.ts Implements threshold evaluation + failure helper.
tests/load/src/harness/thresholds.test.ts Unit tests for threshold evaluation behavior/messages.
packages/sdk-core/src/perf.ts Adds Performance API wrapper utilities and gating.
packages/sdk-core/src/upload/index.ts Wrapes uploadFile with perf measurement.
packages/sdk-core/src/download/index.ts Wrapes downloadAndDecrypt with perf measurement.
packages/sdk-core/src/ipfs/index.ts Wraps IPFS upload/download/unpin with perf measurement.
packages/sdk-core/src/ipns/index.ts Wraps IPNS publish/batch/resolve with perf measurement.
packages/sdk-core/src/folder/index.ts Wraps folder load/fetch+decrypt/update+publish with perf measurement.
packages/sdk-core/src/tests/perf.test.ts Adds unit tests validating perf gating/cleanup/error propagation.
docs/CAPACITY.md Adds capacity model doc consolidating observed limits and scaling guidance.
.planning/phases/22-performance-baselines-completion/22-VERIFICATION.md Adds verification report for Phase 22 deliverables.
.planning/phases/22-performance-baselines-completion/22-VALIDATION.md Adds Phase 22 validation strategy/spec.
.planning/phases/22-performance-baselines-completion/22-RESEARCH.md Adds research notes and rationale for approach.
.planning/phases/22-performance-baselines-completion/22-CONTEXT.md Adds phase context and constraints.
.planning/phases/22-performance-baselines-completion/22-03-SUMMARY.md Adds plan 03 summary (thresholds + capacity).
.planning/phases/22-performance-baselines-completion/22-03-PLAN.md Adds plan 03 execution doc.
.planning/phases/22-performance-baselines-completion/22-02-SUMMARY.md Adds plan 02 summary (journey timing + baselines template).
.planning/phases/22-performance-baselines-completion/22-02-PLAN.md Adds plan 02 execution doc.
.planning/phases/22-performance-baselines-completion/22-01-SUMMARY.md Adds plan 01 summary (SDK perf instrumentation).
.planning/phases/22-performance-baselines-completion/22-01-PLAN.md Adds plan 01 execution doc.
.planning/baselines/22-journey-baselines.md Adds template baseline doc with [PENDING] placeholders.
.planning/STATE.md Updates current phase/plan state tracking.
.planning/ROADMAP.md Marks Phase 22 complete and adds plan list.
.planning/REQUIREMENTS.md Marks PERF-05..08 complete.
.planning/PROJECT.md Updates “Last updated” to reflect Phase 22 completion.

Comment thread packages/sdk-core/src/perf.ts
Comment thread packages/sdk-core/src/ipns/index.ts Outdated
Comment thread .planning/ROADMAP.md Outdated
Comment thread .planning/ROADMAP.md Outdated
Comment thread .planning/STATE.md Outdated
Comment thread .planning/REQUIREMENTS.md Outdated
Comment thread packages/sdk-core/src/perf.ts Outdated
FSM1 and others added 2 commits March 25, 2026 03:58
Captured against staging (api-staging.cipherbox.cc) on 2026-03-25:
- login-to-vault: 23.6s (23.5s wallet auth + 86ms vault load)
- upload-to-visible: 1.4s (100KB file)
- share-to-accessible: 3.0s (2.2s share create + 0.8s recipient access)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: a69ecb4e0a1a
- Make perf mark names unique per invocation (concurrent-safe)
- Tighten PERF_ENABLED to require defined NODE_ENV string (safe in browser prod)
- Fix 404 handling to check error.response.status for AxiosError
- Fix ROADMAP plan checkboxes, table alignment, and STATE status
- Update PERF-07 requirement text from k6 to Vitest harness

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: e5c4e67212f2
@FSM1 FSM1 requested a review from Copilot March 25, 2026 14:32
@FSM1 FSM1 self-assigned this Mar 25, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
tests/load/src/scenarios/spike-test.test.ts (1)

15-15: Threshold integration looks correct, but uploadFile threshold is unreachable.

The spike test uses runFolderWorkload, which only records createFolder, renameItem, and deleteItem operations (per tests/load/src/workloads/folder-workload.ts). The uploadFile threshold at line 82 will never be checked since checkThresholds silently skips operations not present in the metrics.

Consider removing the uploadFile entry to avoid confusion:

🔧 Suggested fix
     const THRESHOLDS: ThresholdConfig[] = [
-      { operation: 'uploadFile', p95MaxMs: 15_000, errorRateMax: 0.15 },
       { operation: 'createFolder', p95MaxMs: 15_000, errorRateMax: 0.15 },
     ];

Also applies to: 79-86

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/load/src/scenarios/spike-test.test.ts` at line 15, The spike test
declares an unreachable "uploadFile" threshold because the scenario uses
runFolderWorkload which only emits createFolder, renameItem, and deleteItem
metrics; remove the "uploadFile" entry from the thresholds in spike-test.test.ts
(or replace it with one of the emitted ops) so
expectThresholdsPassed/checkThresholds won't silently skip it, and keep
references to runFolderWorkload and the uploadFile threshold key when editing so
reviewers can locate the change.
tests/load/src/harness/thresholds.test.ts (1)

24-135: Add explicit coverage for count = 0 error-rate handling.

The implementation has a divide-by-zero guard path; add a dedicated test to lock that behavior (error rate should remain 0 and not violate unless latency does).

As per coding guidelines, "**/*.test.ts: Focus on test coverage, edge cases, and test quality."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/load/src/harness/thresholds.test.ts` around lines 24 - 135, Add a test
case in thresholds.test.ts that covers the count = 0 path for checkThresholds:
create an OperationMetrics via makeMetrics with operation 'uploadFile', count: 0
and errors: 0 (or errors non-zero to still ensure guard path yields 0% if
designed that way), supply a ThresholdConfig for that operation with a low
errorRateMax to ensure no divide-by-zero causes a false violation, call
checkThresholds(metrics, thresholds) and assert result.passed is true and that
no 'error rate' violation appears; reference checkThresholds, makeMetrics, and
ThresholdConfig when adding the test.
packages/sdk-core/src/__tests__/perf.test.ts (1)

70-100: Add a test for production opt-in (__CIPHERBOX_PERF__ = true).

You cover “enabled in test” and “disabled in production,” but not the explicit production opt-in path. Add one assertion for this branch to prevent regression in PERF gating behavior.

As per coding guidelines, "**/*.test.ts: Focus on test coverage, edge cases, and test quality."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/sdk-core/src/__tests__/perf.test.ts` around lines 70 - 100, Add a
test case that covers the production opt-in path by setting process.env.NODE_ENV
= 'production' and setting globalThis.__CIPHERBOX_PERF__ = true, then import the
perf helpers and assert behavior when enabled: for withPerf('prod:optin', ...)
assert it returns the wrapped value and that
performance.getEntriesByType('measure') contains entries whose names start with
'cipherbox:'; and for markStart('optin:test') assert a non-empty start mark and
that markEnd('optin:test', start) returns a non-null measure. Target the
existing helpers withPerf, markStart, and markEnd in your new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.planning/ROADMAP.md:
- Line 27: Update the Phase 22 description text that currently reads “k6 load
testing” to “Vitest-based SDK load harness” and refine the PERF-07 entry to
state “Vitest-based SDK load harness” (preserving the same concurrent actions
scope: upload/download/publish/resolve); locate the string "Phase 22:
Performance Baselines Completion" and the PERF-07 mention in
.planning/ROADMAP.md and replace the k6 wording with the new phrasing while
leaving all scope and checklist semantics unchanged.
- Line 160: The roadmap entry is stale: update the checklist item that currently
reads "instrument 10 sdk-core functions" to reflect the new total of 11
functions because the PR also instruments unpinFromIpfs; specifically edit the
ROADMAP.md entry referencing 22-01-PLAN.md / perf.ts to say "instrument 11
sdk-core functions" (or otherwise note that unpinFromIpfs was added) so the
document matches the implemented instrumentation.

In `@packages/sdk-core/src/__tests__/perf.test.ts`:
- Around line 9-20: Remove the repeated `(globalThis as any).__CIPHERBOX_PERF__`
casts by declaring the property on the global type for the test module and then
use the typed global directly; add a module-level `declare global { var
__CIPHERBOX_PERF__: boolean | undefined }` (or appropriate type) and replace all
uses (e.g., `originalPerfFlag = (globalThis as any).__CIPHERBOX_PERF__`, the
afterEach restore block, and the other occurrences around lines 73 and 91) with
`globalThis.__CIPHERBOX_PERF__` so ESLint warnings go away while preserving the
existing logic that reads, deletes, or assigns the flag (refer to the
`originalPerfFlag` variable and the afterEach restore logic).

---

Nitpick comments:
In `@packages/sdk-core/src/__tests__/perf.test.ts`:
- Around line 70-100: Add a test case that covers the production opt-in path by
setting process.env.NODE_ENV = 'production' and setting
globalThis.__CIPHERBOX_PERF__ = true, then import the perf helpers and assert
behavior when enabled: for withPerf('prod:optin', ...) assert it returns the
wrapped value and that performance.getEntriesByType('measure') contains entries
whose names start with 'cipherbox:'; and for markStart('optin:test') assert a
non-empty start mark and that markEnd('optin:test', start) returns a non-null
measure. Target the existing helpers withPerf, markStart, and markEnd in your
new test.

In `@tests/load/src/harness/thresholds.test.ts`:
- Around line 24-135: Add a test case in thresholds.test.ts that covers the
count = 0 path for checkThresholds: create an OperationMetrics via makeMetrics
with operation 'uploadFile', count: 0 and errors: 0 (or errors non-zero to still
ensure guard path yields 0% if designed that way), supply a ThresholdConfig for
that operation with a low errorRateMax to ensure no divide-by-zero causes a
false violation, call checkThresholds(metrics, thresholds) and assert
result.passed is true and that no 'error rate' violation appears; reference
checkThresholds, makeMetrics, and ThresholdConfig when adding the test.

In `@tests/load/src/scenarios/spike-test.test.ts`:
- Line 15: The spike test declares an unreachable "uploadFile" threshold because
the scenario uses runFolderWorkload which only emits createFolder, renameItem,
and deleteItem metrics; remove the "uploadFile" entry from the thresholds in
spike-test.test.ts (or replace it with one of the emitted ops) so
expectThresholdsPassed/checkThresholds won't silently skip it, and keep
references to runFolderWorkload and the uploadFile threshold key when editing so
reviewers can locate the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ebc8acc9-9e25-4442-aa9f-959e326bf10c

📥 Commits

Reviewing files that changed from the base of the PR and between 6aa9d11 and 128e8f2.

📒 Files selected for processing (31)
  • .planning/PROJECT.md
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/baselines/22-journey-baselines.md
  • .planning/phases/22-performance-baselines-completion/22-01-PLAN.md
  • .planning/phases/22-performance-baselines-completion/22-01-SUMMARY.md
  • .planning/phases/22-performance-baselines-completion/22-02-PLAN.md
  • .planning/phases/22-performance-baselines-completion/22-02-SUMMARY.md
  • .planning/phases/22-performance-baselines-completion/22-03-PLAN.md
  • .planning/phases/22-performance-baselines-completion/22-03-SUMMARY.md
  • .planning/phases/22-performance-baselines-completion/22-CONTEXT.md
  • .planning/phases/22-performance-baselines-completion/22-RESEARCH.md
  • .planning/phases/22-performance-baselines-completion/22-VALIDATION.md
  • .planning/phases/22-performance-baselines-completion/22-VERIFICATION.md
  • docs/CAPACITY.md
  • packages/sdk-core/src/__tests__/perf.test.ts
  • packages/sdk-core/src/download/index.ts
  • packages/sdk-core/src/folder/index.ts
  • packages/sdk-core/src/ipfs/index.ts
  • packages/sdk-core/src/ipns/index.ts
  • packages/sdk-core/src/perf.ts
  • packages/sdk-core/src/upload/index.ts
  • tests/load/src/harness/thresholds.test.ts
  • tests/load/src/harness/thresholds.ts
  • tests/load/src/scenarios/ipns-publish-storm.test.ts
  • tests/load/src/scenarios/mixed-workload.test.ts
  • tests/load/src/scenarios/spike-test.test.ts
  • tests/load/src/scenarios/sustained-load.test.ts
  • tests/load/src/scenarios/upload-throughput.test.ts
  • tests/web-e2e/tests/journey-timing.spec.ts

Comment thread .planning/ROADMAP.md Outdated
Comment thread .planning/ROADMAP.md Outdated
Comment thread packages/sdk-core/src/__tests__/perf.test.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.

Comment thread packages/sdk-core/src/perf.ts
Comment thread tests/load/src/harness/thresholds.ts
Comment thread tests/web-e2e/tests/journey-timing.spec.ts
- Update ROADMAP Phase 22 description from k6 to Vitest-based load testing
- Update instrumented function count from 10 to 11 (unpinFromIpfs added)
- Replace `as any` casts on globalThis with `declare global` type declaration

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 60737b3c4487

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.

Comment thread .planning/baselines/22-journey-baselines.md Outdated
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 10337a3e9326

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.

Comment thread docs/CAPACITY.md
@FSM1

FSM1 commented Mar 25, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai please review

@coderabbitai

coderabbitai Bot commented Mar 25, 2026

Copy link
Copy Markdown

@FSM1, I'll kick off a full review of PR #355 right away!


🧠 Learnings used
Learnt from: FSM1
Repo: FSM1/cipher-box PR: 318
File: tests/load/src/scenarios/upload-throughput.test.ts:8-15
Timestamp: 2026-03-22T18:39:51.469Z
Learning: In `tests/load/src/scenarios/*.test.ts` (load test scenarios in FSM1/cipher-box): Load tests are measurement tools, not pass/fail correctness tests. `Promise.allSettled` is used intentionally — partial client completion is expected, and error counts are captured and reported in the metrics JSON via `aggregateAndReport`. Do NOT flag the absence of `expect(failures).toHaveLength(0)` assertions or suggest switching to `Promise.all`. The test should remain green even when some client workloads reject.

Learnt from: FSM1
Repo: FSM1/cipher-box PR: 296
File: .release-please-manifest.json:3-6
Timestamp: 2026-03-20T14:24:58.097Z
Learning: In FSM1/cipher-box, the newly extracted packages `packages/core`, `packages/api-client`, `packages/sdk-core`, and `packages/sdk` are all `private: true` and share the monorepo version (`0.24.2`). Their entries in `.release-please-manifest.json` must match the version declared in their respective `package.json` files (currently `0.24.2`). Do NOT suggest initialising them at `0.1.0` — that would create a manifest/package.json mismatch and break Release Please bump calculations. Only `packages/crypto` and the root `"."` were pre-existing; the new packages simply join the same coupled versioning scheme.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.

Comment thread docs/CAPACITY.md
Comment thread tests/load/src/harness/thresholds.ts
FSM1 and others added 4 commits March 25, 2026 16:26
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: f0cb8c856004
Resolves conflicts between Phase 21 BYO-IPFS completion and Phase 22
performance baselines. Both phases now marked complete in ROADMAP,
STATE, and PROJECT. Upload pinFn override from Phase 21 preserved
alongside withPerf instrumentation from Phase 22.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 18d127586f12
Allows distinguishing CipherBox-pinned uploads (ipfs:upload) from
BYO-pinned uploads (ipfs:upload:byo) in the Performance timeline,
enabling diagnosis of BYO-specific latency issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: ef23b97d131d
Consistent naming with ipfs:upload:byo for clear distinction in
Performance timeline.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 3b4cc5bc6dba
@FSM1 FSM1 requested a review from Copilot March 25, 2026 16:18
@FSM1 FSM1 merged commit 25bc1b3 into main Mar 25, 2026
26 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 4 comments.

Comment thread .planning/STATE.md
- pinWithMode treats Pinata like Kubo: direct upload bypasses CipherBox relay entirely
- Connection test probe order updated: Kubo -> Pinata -> PSA; pinata.cloud URLs skip Kubo probe
- BYO Pinata baselines: pin p50=2.0s (+47% vs local Kubo), tail latency p99 13.5% better, 98% CipherBox API load reduction per file
- perf.ts PERF_ENABLED evaluated once at module load (zero overhead in production); **CIPHERBOX_PERF** global for opt-in production debugging

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The state summary references a CIPHERBOX_PERF global flag, but the actual opt-in flag used by sdk-core perf gating is globalThis.__CIPHERBOX_PERF__ (double-underscore). This mismatch will mislead anyone trying to enable perf marks in production/debug builds; please update the flag name here to match the implementation.

Suggested change
- perf.ts PERF_ENABLED evaluated once at module load (zero overhead in production); **CIPHERBOX_PERF** global for opt-in production debugging
- perf.ts PERF_ENABLED evaluated once at module load (zero overhead in production); `globalThis.__CIPHERBOX_PERF__` global for opt-in production debugging

Copilot uses AI. Check for mistakes.
Comment thread .planning/STATE.md
- Connection test probe order updated: Kubo -> Pinata -> PSA; pinata.cloud URLs skip Kubo probe
- BYO Pinata baselines: pin p50=2.0s (+47% vs local Kubo), tail latency p99 13.5% better, 98% CipherBox API load reduction per file
- perf.ts PERF_ENABLED evaluated once at module load (zero overhead in production); **CIPHERBOX_PERF** global for opt-in production debugging
- Load test thresholds set at 2-3x observed baselines; spike test most generous (15s/15%); vitest expect() for CI failure on breach

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line says load-test thresholds use vitest expect() to fail CI on breach, but the scenarios now call expectThresholdsPassed(...), which logs violations and throws an Error (no vitest expect() involved). Please update the text to reflect the actual enforcement mechanism so the state doc stays accurate.

Suggested change
- Load test thresholds set at 2-3x observed baselines; spike test most generous (15s/15%); vitest expect() for CI failure on breach
- Load test thresholds set at 2-3x observed baselines; spike test most generous (15s/15%); enforced via expectThresholdsPassed(...) which logs violations and throws on breach to fail CI

Copilot uses AI. Check for mistakes.
expected: 'JOURNEY_TIMING: JSON lines printed for login-to-vault, upload-to-visible, share-to-accessible, and summary; each timing under its sanity-check limit'
why_human: 'Requires running API + frontend locally; Playwright tests cannot be verified without live services'
- test: 'Open browser DevTools Performance tab while using the app and start a recording'
expected: 'cipherbox:upload:full, cipherbox:ipfs:upload, cipherbox:folder:update-publish etc. appear as named measures in the timeline'

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual verification expectation lists cipherbox:ipfs:upload, but the SDK instrumentation uses more specific operation names (e.g. ipfs:upload:cipherbox and ipfs:upload:byo, which become cipherbox:ipfs:upload:cipherbox / cipherbox:ipfs:upload:byo). Please update this example list so someone following the steps in DevTools knows what to look for.

Suggested change
expected: 'cipherbox:upload:full, cipherbox:ipfs:upload, cipherbox:folder:update-publish etc. appear as named measures in the timeline'
expected: 'cipherbox:upload:full, cipherbox:ipfs:upload:cipherbox, cipherbox:ipfs:upload:byo, cipherbox:folder:update-publish etc. appear as named measures in the timeline'

Copilot uses AI. Check for mistakes.
| 4 | Existing sdk-core function signatures and return values are unchanged | VERIFIED | `withPerf` is a transparent wrapper using `return withPerf(op, async () => { ...existing body... })` - signatures identical |
| 5 | Login-to-vault journey timing is captured as wall-clock milliseconds in a Playwright test | VERIFIED | `journey-timing.spec.ts` Journey 1 uses `performance.now()`, outputs `JOURNEY_TIMING:` JSON with `walletAuthMs` and `vaultLoadMs` |
| 6 | Upload-to-visible and share-to-accessible journey timings are captured | VERIFIED | Journey 2 and Journey 3 both present with `performance.now()` timing and structured JSON output |
| 7 | Load test scenarios have automated pass/fail thresholds that fail the test when p95 latency or error rate exceeds defined limits | VERIFIED | All 5 scenarios import `checkThresholds` and call `expect(thresholdResult.passed).toBe(true)` with violation messages |

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the "Observable Truths" table, the evidence for threshold enforcement says scenarios import checkThresholds and call expect(thresholdResult.passed).toBe(true), but the current implementation imports expectThresholdsPassed and throws on violations. Please update the evidence text to match the current code so the verification report remains trustworthy.

Suggested change
| 7 | Load test scenarios have automated pass/fail thresholds that fail the test when p95 latency or error rate exceeds defined limits | VERIFIED | All 5 scenarios import `checkThresholds` and call `expect(thresholdResult.passed).toBe(true)` with violation messages |
| 7 | Load test scenarios have automated pass/fail thresholds that fail the test when p95 latency or error rate exceeds defined limits | VERIFIED | All 5 scenarios import `expectThresholdsPassed` and rely on it to throw when any threshold is violated; thrown errors carry violation messages |

Copilot uses AI. Check for mistakes.
@FSM1 FSM1 deleted the feat/phase-22-performance-baselines-completion branch March 25, 2026 22:41
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.

2 participants