Skip to content

fix(core): issue panels reported nothing fixed on a renamed board - #2871

Merged
gsxdsm merged 3 commits into
mainfrom
fix/issue-analytics-lanes
Jul 30, 2026
Merged

fix(core): issue panels reported nothing fixed on a renamed board#2871
gsxdsm merged 3 commits into
mainfrom
fix/issue-analytics-lanes

Conversation

@gsxdsm

@gsxdsm gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Fourth and last of the lane-bound analytics sites from #2839, after #2864, #2866 and #2870.

The defect

aggregateGithubIssueAnalytics and its GitLab twin filtered their resolved-issue query on "column" = 'done'. On a renamed board that matches nothing, so fixed is zero, the resolved-issue list is empty, and net reports every filed issue as still outstanding — while the team closes issues all week. Nothing errors.

Same fix as the previous three: resolve per project via resolveProjectColumnsForRoles, bind an IN list, thread the store from each Command Center caller so the parameters have suppliers immediately.

Both providers in one change, deliberately

These two files are copies — same query, only the provider literal differs — and a copy is exactly what gets half-fixed. Converting one and not the other type-checks, passes that provider's test, and leaves the second silently broken with no signal anywhere. The suite runs every case against both, so the pair cannot drift.

Measured

Reverted, exactly the two renamed cases fail — one per provider — while both default-vocabulary controls, both WIP-lane negatives, and both omitted-store legacy cases stay green:

✓ github: default vocabulary counts a resolved issue
× github: renamed vocabulary counts a resolved issue
✓ github: renamed vocabulary does NOT count an issue still in the WIP lane
✓ github: without a lane store, the legacy id still answers
✓ gitlab: default vocabulary counts a resolved issue
× gitlab: renamed vocabulary counts a resolved issue
✓ gitlab: renamed vocabulary does NOT count an issue still in the WIP lane
✓ gitlab: without a lane store, the legacy id still answers
  Tests  2 failed | 6 passed (8)

That the failures are symmetric is itself the check on the copy-paste risk.

Scope

The sync SQLite arms keep their literals: they throw in backend mode and have no production caller, the same dead-arm conclusion as cleanupStaleMergeQueueRowsImpl on #2839.

Verification

pnpm test:gate green · Command Center + GitLab issue analytics suites 10/10 · tsc core 0, dashboard 0 · lint 0 · changeset included.


This closes the lane-bound half of #2839. All 14 sites the hand-review identified as genuinely vocabulary-bound are now converted across four PRs. What remains there is the 11 != 'archived' exclusions, which are probably correct as literals — archiving writes task.column = 'archived' unconditionally as a state rather than a lane — plus one dead SQLite arm. Those need per-site judgment, not conversion.

🤖 Generated with Claude Code

aggregateGithubIssueAnalytics and its GitLab twin filtered their resolved-issue query on
"column" = 'done'. On a renamed board that matches nothing, so `fixed` is ZERO, the resolved-issue
list is empty, and `net` reports every filed issue as still outstanding — while the team closes
issues all week. Nothing errors.

Fourth and last of the lane-bound analytics sites from #2839, same fix as #2864/#2866/#2870: resolve
per PROJECT via resolveProjectColumnsForRoles, bind an IN list, thread the store from each Command
Center caller so the parameters have suppliers immediately.

BOTH PROVIDERS IN ONE CHANGE, because they are copies and a copy is what gets half-fixed. The two
files carry the same query with only the provider literal differing, so converting one and not the
other type-checks, passes that provider's test, and leaves the second silently broken. The suite runs
every case against both.

MEASURED: reverted, exactly the two renamed cases fail — one per provider — while both
default-vocabulary controls, both WIP-lane negatives, and both omitted-store legacy cases stay green.
With the fix, 8 passed.

The sync SQLite arms keep their literals: they throw in backend mode and have no production caller.

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

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@gsxdsm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1ebb5c6b-3869-4690-b094-7aca9aba165f

📥 Commits

Reviewing files that changed from the base of the PR and between ea477f3 and 92a21c8.

📒 Files selected for processing (6)
  • .changeset/issue-analytics-renamed-lanes.md
  • packages/core/src/__tests__/postgres/issue-analytics-renamed-lanes.pg.test.ts
  • packages/core/src/github-issue-analytics.ts
  • packages/core/src/gitlab-issue-analytics.ts
  • packages/dashboard/src/routes/register-command-center-routes.ts
  • scripts/lib/sql-column-literals-baseline.json

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.

@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes GitHub and GitLab issue analytics for renamed completion lanes.

  • Resolves project completion-lane IDs before querying resolved issues.
  • Passes the project-scoped store from both Command Center analytics routes.
  • Adds PostgreSQL regression coverage for default, renamed, WIP, and legacy fallback behavior.
  • Updates the SQL-literal baseline and includes a patch changeset.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/core/src/github-issue-analytics.ts Replaces the PostgreSQL resolved-issue literal with a bound project completion-lane list while preserving legacy fallback behavior.
packages/core/src/gitlab-issue-analytics.ts Applies the same project completion-lane resolution to GitLab issue analytics.
packages/dashboard/src/routes/register-command-center-routes.ts Supplies each request's project-scoped store to the GitHub and GitLab aggregators.
packages/core/src/tests/postgres/issue-analytics-renamed-lanes.pg.test.ts Covers both providers across default lanes, renamed completion lanes, WIP exclusions, and omitted-store fallback.
.changeset/issue-analytics-renamed-lanes.md Adds the required patch release note for the user-visible analytics fix.
scripts/lib/sql-column-literals-baseline.json Reduces the tracked literal counts to reflect the converted analytics queries.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Request[Command Center analytics request] --> Store[Project-scoped task store]
    Store --> Resolver[Resolve project complete lanes]
    Resolver --> Query[GitHub or GitLab resolved-issue query]
    Query --> Result[Fixed, net, and resolved-issue analytics]
Loading

Reviews (2): Last reviewed commit: "chore(gate): re-record the SQL baseline ..." | Re-trigger Greptile

gsxdsm and others added 2 commits July 30, 2026 16:12
This branch removes legacy comparisons, and the gate (#2841, merged after this was opened) fails on
a LOWERED count so a migrated slot cannot be silently regrown into. Without this the branch turns
main red on merge, which is what #2864 did.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Added a baseline re-record for the SQL column-literal gate (#2841), which merged after this PR was opened.

That gate fails on a lowered count — deliberately, so a migrated site cannot leave an unused allowance behind for the surface to regrow into. This branch removes legacy comparisons, so without the re-record it turns main red on merge. That is exactly what happened when #2864 landed; #2878 unblocks it.

Baseline entries are per file and this PR only touches its own, so it can merge in any order relative to the other analytics conversions. The coupling was mine to spot when I opened five PRs against a gate that did not exist yet, and I did not.

gsxdsm added a commit that referenced this pull request Jul 30, 2026
**`pnpm test:gate` and both `pretest` hooks fail on `main` right now.**
Merge this first.

## What happened

#2841 (the SQL gate) merged, then #2864 merged. #2864 removed three
legacy comparisons from `team-analytics.ts`, but its baseline entry
still allows six — and this gate **fails on a lowered count by design**,
so a migrated slot cannot be silently regrown into later.

Baseline 30 → 28.

## This is my sequencing error

The four analytics conversions were branched and reviewed **before** the
gate existed, so none of them carries a baseline update. The gate then
landed first, which means **each of them breaks `main` as it merges**. I
opened all five without thinking about the order they would land in.

The three still open — #2866, #2870, #2871 — will each do this again. I
am adding baseline updates to them next so they land clean.

## Note on the downward check

The "count went down" failure looks like pedantry until it fires. It
exists so a migrated site cannot leave an unused allowance behind for
the surface to regrow into — the same rot as an allow-list entry for a
deleted function. The real cost is that a conversion and its gate have
to land in a known order, which is a coupling I created and did not plan
for.

## Verification

`pnpm test:gate` green with the re-recorded baseline · lint 0 · `node
scripts/check-sql-column-literals.mjs` exit 0.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm
gsxdsm merged commit 890e1f8 into main Jul 30, 2026
7 checks passed
@gsxdsm
gsxdsm deleted the fix/issue-analytics-lanes branch July 30, 2026 23:53
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