fix: approval rate now counts unique projects, not individual cert rows by dhamariT · Pull Request #82 · hackclub/shipwrights

dhamariT · 2026-03-25T18:55:18Z

A project rejected 3x then approved was inflating the rejection count.
Now only the latest verdict per ftProjectId is counted, giving an
accurate picture of how many projects ultimately succeed vs fail.
Added tooltip explaining the methodology on both dashboard views.

A project rejected 3x then approved was inflating the rejection count. Now only the latest verdict per ftProjectId is counted, giving an accurate picture of how many projects ultimately succeed vs fail. Added tooltip explaining the methodology on both dashboard views.

github-actions · 2026-03-25T18:55:55Z

run pnpm pretty please :3

Copilot

Pull request overview

Updates ship certification stats so approval rate reflects unique projects (latest verdict per ftProjectId) rather than counting every certification row, and adds UI tooltips explaining the methodology.

Changes:

Added fetchUniqueProjectStats() to compute approved/rejected totals and approval rate by unique project (latest verdict per ftProjectId).
Switched dashboard/logs stats to use the unique-project methodology for approval rate (and related counts).
Added hover tooltips on both admin views to explain how the rate is calculated.

Reviewed changes

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

File	Description
sw-dash/src/lib/certs.ts	Adds unique-project aggregation query and wires it into the main stats fetch.
sw-dash/src/app/api/admin/ship_certifications/logs/route.ts	Uses the new unique-project stats in the logs API response.
sw-dash/src/app/admin/ship_certifications/logs/page.tsx	Adds tooltip explaining unique-project approval rate on logs view.
sw-dash/src/app/admin/ship_certifications/certs-view.tsx	Adds tooltip explaining unique-project approval rate on dashboard view.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T19:51:36Z

sw-dash/src/lib/certs.ts

+  const dateFilter = before ? `AND sc.reviewCompletedAt < '${before.toISOString()}'` : ''
+


dateFilter is built with the sc alias ("AND sc.reviewCompletedAt < ...") but then reused inside the subquery that aliases the table as sc2. When before is provided, this will produce invalid SQL (unknown column/alias) or apply the filter to the wrong scope. Consider building separate filters for sc vs sc2 (or parameterizing and repeating the condition with the correct alias).

Copilot · 2026-03-25T19:51:36Z

sw-dash/src/lib/certs.ts

+  const rows = await prisma.$queryRawUnsafe<UniqueProjectRow[]>(`
+    SELECT


This query uses $queryRawUnsafe with direct string interpolation of before.toISOString() into the SQL. Even though before is a Date today, this pattern is brittle (MySQL datetime parsing of ISO strings with Z can fail) and bypasses parameterization. Prefer $queryRaw (tagged template) or $queryRawUnsafe with placeholders and passing before as a bound parameter.

Copilot · 2026-03-25T19:51:37Z

sw-dash/src/lib/certs.ts

+          WHERE sc2.ftProjectId = sc.ftProjectId
+            AND sc2.status IN ('approved', 'rejected')
+            ${dateFilter}
+          ORDER BY sc2.reviewCompletedAt DESC


The inner "latest verdict" selection orders only by reviewCompletedAt DESC. If two rows share the same completion timestamp for a project, the chosen row is nondeterministic. Add a stable tie-breaker (e.g., ORDER BY sc2.reviewCompletedAt DESC, sc2.id DESC).

Suggested change

ORDER BY sc2.reviewCompletedAt DESC

ORDER BY sc2.reviewCompletedAt DESC, sc2.id DESC

Copilot · 2026-03-25T19:51:37Z

sw-dash/src/app/admin/ship_certifications/logs/page.tsx

+              <span className="text-gray-400 cursor-help">Rate ⓘ:</span>
              <span className="text-amber-400 font-bold">{stats?.approvalRate || 0}%</span>
+              <div className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 pointer-events-none transition-opacity z-50">


This tooltip is hover-only and the trigger isn’t keyboard-focusable or exposed to screen readers (no aria-describedby, and group-hover won’t activate on focus). Consider using a <button> (or tabIndex=0) for the info trigger and showing the tooltip on group-focus-within/focus, with appropriate ARIA attributes.

Suggested change

<span className="text-gray-400 cursor-help">Rate ⓘ:</span>

<span className="text-amber-400 font-bold">{stats?.approvalRate || 0}%</span>

<div className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 pointer-events-none transition-opacity z-50">

<button

type="button"

className="text-gray-400 cursor-help focus:outline-none focus-visible:ring-2 focus-visible:ring-amber-500 rounded"

aria-describedby="approval-rate-tooltip"

>

Rate ⓘ:

</button>

<span className="text-amber-400 font-bold">{stats?.approvalRate || 0}%</span>

<div

id="approval-rate-tooltip"

role="tooltip"

className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 pointer-events-none transition-opacity z-50"

>

Copilot · 2026-03-25T19:51:37Z

sw-dash/src/app/admin/ship_certifications/certs-view.tsx

+              <div className="text-gray-500 font-mono text-xs mb-1 cursor-help">
+                Approval rate ⓘ
+              </div>
              <span className="text-xl font-bold font-mono text-white">{stats.approvalRate}%</span>
+              <div className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 pointer-events-none transition-opacity z-50">


Same accessibility issue here: the info tooltip appears only on hover and isn’t reachable via keyboard or announced to assistive tech. Use a focusable trigger (button/tabIndex), show tooltip on focus (e.g., group-focus-within), and wire up aria-describedby/role="tooltip".

Suggested change

<div className="text-gray-500 font-mono text-xs mb-1 cursor-help">

Approval rate ⓘ

</div>

<span className="text-xl font-bold font-mono text-white">{stats.approvalRate}%</span>

<div className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 pointer-events-none transition-opacity z-50">

<button

type="button"

className="text-gray-500 font-mono text-xs mb-1 cursor-help focus:outline-none"

aria-describedby="approval-rate-tooltip"

>

Approval rate ⓘ

</button>

<span className="text-xl font-bold font-mono text-white">{stats.approvalRate}%</span>

<div

id="approval-rate-tooltip"

role="tooltip"

className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 pointer-events-none transition-opacity z-50"

>

…ccessibility - Split dateFilter into separate aliases (sc/sc2) to fix wrong-scope filter in subquery - Use bound parameters instead of string interpolation to prevent SQL injection - Add sc2.id DESC tie-breaker for deterministic ordering - Make tooltip triggers keyboard-focusable with aria-describedby

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T15:20:39Z

sw-dash/src/lib/certs.ts

+      FROM ship_certs sc
+      WHERE sc.status IN ('approved', 'rejected')
+        AND sc.ftProjectId IS NOT NULL
+        ${scDateFilter}
+        AND sc.id = (
+          SELECT sc2.id FROM ship_certs sc2
+          WHERE sc2.ftProjectId = sc.ftProjectId
+            AND sc2.status IN ('approved', 'rejected')
+            ${sc2DateFilter}
+          ORDER BY sc2.reviewCompletedAt DESC, sc2.id DESC
+          LIMIT 1
+        )
+      UNION ALL
+      SELECT NULL AS ftProjectId, status
+      FROM ship_certs
+      WHERE status IN ('approved', 'rejected')
+        AND ftProjectId IS NULL
+        ${nullDateFilter}


fetchUniqueProjectStats treats any status IN ('approved','rejected') row as a verdict even if reviewCompletedAt is NULL. Because reviewCompletedAt is nullable in the schema, this can (a) count incomplete/inconsistent rows and (b) affect the “latest verdict” tie-break ordering. Consider adding reviewCompletedAt IS NOT NULL to both the sc and sc2 filters (and the ftProjectId IS NULL branch) so only completed verdicts participate in the dedupe/counting.

Copilot · 2026-03-26T15:20:40Z

sw-dash/src/lib/certs.ts

+      SELECT ftProjectId, status
+      FROM ship_certs sc
+      WHERE sc.status IN ('approved', 'rejected')
+        AND sc.ftProjectId IS NOT NULL
+        ${scDateFilter}
+        AND sc.id = (
+          SELECT sc2.id FROM ship_certs sc2
+          WHERE sc2.ftProjectId = sc.ftProjectId
+            AND sc2.status IN ('approved', 'rejected')
+            ${sc2DateFilter}
+          ORDER BY sc2.reviewCompletedAt DESC, sc2.id DESC
+          LIMIT 1
+        )


This query uses a correlated subquery per ship_certs row (sc.id = (SELECT ... WHERE sc2.ftProjectId = sc.ftProjectId ... ORDER BY ... LIMIT 1)), which can become expensive as ship_certs grows. Since the table only has an index on ftProjectId (not a composite covering ftProjectId + reviewCompletedAt), consider rewriting using a window function (ROW_NUMBER() OVER (PARTITION BY ftProjectId ORDER BY reviewCompletedAt DESC, id DESC)) or a join against a derived table of latest (ftProjectId, reviewCompletedAt, id) to keep this O(n) and index-friendly.

Copilot · 2026-03-26T15:20:40Z

sw-dash/src/lib/certs.ts

+  const rows = await prisma.$queryRawUnsafe<UniqueProjectRow[]>(
+    `
+    SELECT


$queryRawUnsafe isn’t necessary here (the only dynamic parts are on/off SQL fragments and the values are already passed as parameters). Using $queryRaw with Prisma.sql/Prisma.join for conditional fragments keeps the query safely parameterized by construction and reduces the risk of future edits accidentally interpolating unsanitized values into the SQL string.

style: run prettier on modified files

4804ffb

dhamariT marked this pull request as ready for review March 25, 2026 19:48

Copilot AI review requested due to automatic review settings March 25, 2026 19:48

Copilot started reviewing on behalf of dhamariT March 25, 2026 19:48 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

dhamariT requested a review from Copilot March 26, 2026 15:15

Copilot started reviewing on behalf of dhamariT March 26, 2026 15:16 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: approval rate now counts unique projects, not individual cert rows#82

fix: approval rate now counts unique projects, not individual cert rows#82
dhamariT wants to merge 3 commits intomainfrom
investigate-high-rejection-rate

dhamariT commented Mar 25, 2026

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const dateFilter = before ? `AND sc.reviewCompletedAt < '${before.toISOString()}'` : ''

		const rows = await prisma.$queryRawUnsafe<UniqueProjectRow[]>(`
		SELECT

	ORDER BY sc2.reviewCompletedAt DESC
	ORDER BY sc2.reviewCompletedAt DESC, sc2.id DESC

-              <span className="text-gray-400 cursor-help">Rate ⓘ:</span>
-              <span className="text-amber-400 font-bold">{stats?.approvalRate || 0}%</span>
-              <div className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 pointer-events-none transition-opacity z-50">
+              <button
+                type="button"
+                className="text-gray-400 cursor-help focus:outline-none focus-visible:ring-2 focus-visible:ring-amber-500 rounded"
+                aria-describedby="approval-rate-tooltip"
+              >
+                Rate ⓘ:
+              </button>
+              <span className="text-amber-400 font-bold">{stats?.approvalRate || 0}%</span>
+              <div
+                id="approval-rate-tooltip"
+                role="tooltip"
+                className="absolute bottom-full left-0 mb-2 w-64 p-2 bg-zinc-800 border border-zinc-700 rounded-lg text-xs text-gray-300 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 pointer-events-none transition-opacity z-50"
+              >

Conversation

dhamariT commented Mar 25, 2026

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants