Skip to content

test(server): cover PR project-shell edge cases on getProjectShells - #11421

Open
Adamulek123 wants to merge 1 commit into
pingdotgg:mainfrom
Adamulek123:t3-prperf-15-snapshot
Open

Adamulek123 wants to merge 1 commit into
pingdotgg:mainfrom
Adamulek123:t3-prperf-15-snapshot

Conversation

@Adamulek123

@Adamulek123 Adamulek123 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Upstream #11299 landed the same workspace-scan fix this branch originally implemented, with the getProjectShells(projectIds) shape. The duplicate listActiveProjectShells implementation is dropped in favor of #11299. Merged state of this PR is tests-only.

What changed

Keeps only the incremental tests #11299 lacks: narrow-query fanout (the service never reads getShellSnapshot), deleted-singleton failing as provider-unsupported, projectId plus projectIds intersection, and created_at ordering on reverse insertion. No production code changes.

Validation

  • PullRequestService.test.ts plus ProjectionSnapshotQuery.test.ts: 148 passed
  • t3 typecheck clean

Measured impact

Compared with main-latest.json from main commit b1e223e (which includes #11299), using the focused snapshot suite:

PR RPC project reads Main (#11299) This PR Change
SQL statements 1 (narrow) 1 (narrow) unchanged, now pinned
getShellSnapshot calls (fanout test) unasserted 0 +1 regression guard
Focused tests 144 pass 148 pass +4 edge tests

The production saving (5-statement snapshot down to 1 narrow read per PR RPC) ships via #11299; this PR locks the edge cases it lacks. Coverage: SPAWN-ASSERTED.

Built with Muse Spark (opencode/muse-spark-1.3) via implement and audit subagent loops with strict branch-audit reviews.

Summary by CodeRabbit

  • Tests
    • Added regression coverage to ensure projects appear in consistent creation-date and ID order.
    • Added coverage for pull request details, statistics, and lists using streamlined project lookups.
    • Added validation for unavailable projects and combined project filters in pull request lists.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 12, 2026
@Adamulek123 Adamulek123 changed the title perf(server): narrow project-shell reads for PR RPCs test(server): cover PR project-shell edge cases on getProjectShells Sep 12, 2026
@Adamulek123

Copy link
Copy Markdown
Contributor Author

Perf proof (pr-load-perf harness, baseline b1e223e)

  • Branch 8c27e72 (tests-only, on top of perf(server): avoid workspace scans when loading pull requests #11299): harness PASS; scenario prperf-15 148/148 (baseline 144).
  • Asserted: zero shellSnapshotCalls on detail plus list plus stats fanout; single-statement narrow reads; created_at ordering on reverse insertion.
  • Coverage: SPAWN-ASSERTED. Own scenario 16 percent faster than baseline; no regression.

Upstream pingdotgg#11299 landed the same workspace-scan fix with the getProjectShells(projectIds?) shape, subsuming this branch's listActiveProjectShells approach. Rebase onto origin/main, drop the duplicate prod code, and keep only the net-new tests: narrow-query fanout (never getShellSnapshot), deleted singleton mapping to provider-unsupported, projectId+projectIds intersection, and created_at ordering regardless of insertion order.
@Adamulek123
Adamulek123 marked this pull request as ready for review September 13, 2026 15:04
@macroscopeapp

macroscopeapp Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d50a009

Macroscope's review found this PR approvable — The PR adds regression tests only, using isolated in-memory persistence and mocked providers to cover project-shell ordering and pull-request lookup edge cases. It does not modify production code, defaults, schemas, deployment behavior, or static-analysis configuration.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 20f38980-1b2a-4bab-a71e-9ddbd87af93a

📥 Commits

Reviewing files that changed from the base of the PR and between c542b78 and d50a009.

📒 Files selected for processing (2)
  • apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts
  • apps/server/src/pullRequest/PullRequestService.test.ts

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


📝 Walkthrough

Walkthrough

The pull request adds regression tests for project shell ordering and PullRequestService project resolution, deleted-project handling, and combined project filters.

Changes

Pull request query regressions

Layer / File(s) Summary
Project shell ordering
apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts
Adds an integration test that verifies getProjectShells() returns projects ordered by created_at ASC and project_id ASC.
PullRequestService project queries and filters
apps/server/src/pullRequest/PullRequestService.test.ts
Adds tests for narrow project lookups, provider-unsupported handling when a project is absent, and intersection semantics for projectId and projectIds filters.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Other

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to d50a0

The added tests protect ordering, deleted-project handling, narrow queries, and combined filters without changing production behavior. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies a tests-only change that covers project-shell edge cases in getProjectShells. It is concise and related to the main changes.
Description check ✅ Passed The description clearly explains the tests added, the reason for the changes, validation results, and measured impact. It does not use every template heading, but it provides the required information …
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 2…
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant