From 65ab8d2432ad43ef983db23bba00f635508d2438 Mon Sep 17 00:00:00 2001 From: jmgasper Date: Mon, 7 Sep 2026 12:17:54 +1000 Subject: [PATCH] PM-5966 Exclude unfinished challenges from copilot history --- ReadMe.md | 24 ++ docs/swagger.yaml | 25 +- src/services/SpecialRoleService.ts | 29 ++- test/integration/SpecialRoleService.test.js | 246 ++++++++++++++++++++ test/unit/SpecialRoleService.test.js | 15 +- 5 files changed, 318 insertions(+), 21 deletions(-) create mode 100644 test/integration/SpecialRoleService.test.js diff --git a/ReadMe.md b/ReadMe.md index 15bdf59..e150d10 100644 --- a/ReadMe.md +++ b/ReadMe.md @@ -26,6 +26,30 @@ pnpm start For development with TypeScript watch mode, use `pnpm start:dev`. Production runs the compiled NestJS entrypoint at `dist/main.js`. +## Copilot History Regression Tests + +Copilot profile summaries, challenge history, and track totals include only +completed or cancelled challenges. Fulfillment is completed challenges divided +by completed plus cancelled challenges, excluding client-request cancellations +from the denominator. Client-request cancellations remain visible in history. +Drafts and other non-terminal challenges, including active challenges in post +mortem, are excluded from all Copilot counts. Reviewer status handling is unchanged. + +The PostgreSQL regression suite executes the service's real SQL against minimal +fixtures in an empty test database. It verifies the PM-5966 clarification example, +all cancellation types, empty fulfillment denominators, visibility, and Reviewer +behavior. Both schemas and all fixtures are created in a transaction and rolled +back afterward; setup fails if either schema already exists. + +```bash +nvm use +SPECIAL_ROLE_TEST_DB_URL=postgresql://user:password@localhost:5432/member_test \ + pnpm exec mocha --require ts-node/register/transpile-only test/integration/SpecialRoleService.test.js +``` + +The suite is skipped when `SPECIAL_ROLE_TEST_DB_URL` is unset. The existing +isolated service tests remain available through `pnpm test`. + ## Database Setup Please setup PostgreSQL first. If you are using docker, you can run: diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 3f06edf..20a1099 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -1656,8 +1656,11 @@ paths: Reviewer, Primary Reviewer, Reviewer, Failure Reviewer, Final Reviewer, Aggregator, Stress Reviewer, Accuracy Reviewer, Primary Screener, Checkpoint Screener, and Checkpoint Reviewer. Copilot assignments use - the Copilot resource role. The summary does not load challenge names or - Copilot track/fulfillment details. + the Copilot resource role and count only COMPLETED, CANCELLED, or + CANCELLED_* challenges, including client-request cancellations. Drafts + and other non-terminal challenges are excluded from Copilot counts; + Reviewer counts have no status restriction. The summary does not load + challenge names or Copilot track/fulfillment details. Authorization: - Public: Anonymous callers may retrieve the count-only summary. @@ -1707,6 +1710,11 @@ paths: no groups, no user-whitelist rows, and are not tasks. Restricted challenge IDs and names are never returned. + Copilot history, detail totals, and track counts include only COMPLETED, + CANCELLED, or CANCELLED_* challenges. Drafts and other non-terminal + challenges, including active challenges in post mortem, are excluded. + Reviewer history and totals have no status restriction. + Copilot `trackCounts` includes only non-zero Development, Design, Quality Assurance, and Data Science keys. Fulfillment is `COMPLETED / (COMPLETED + qualifying CANCELLED*) * 100`. @@ -2803,12 +2811,15 @@ definitions: type: integer format: int32 minimum: 1 - description: Anonymous-visible distinct challenge count for this role family. + description: > + Anonymous-visible distinct challenge count for this role family. + Copilot counts include only completed or cancelled challenges. SpecialRoleTrackCounts: type: object description: > - Non-zero Copilot challenge counts by challenge track. A zero-count track - is omitted. + Non-zero Copilot challenge counts by challenge track, including only + completed or cancelled challenges. Client-request cancellations are + included. A zero-count track is omitted. properties: DEVELOPMENT: type: integer @@ -2919,7 +2930,9 @@ definitions: type: integer format: int32 minimum: 0 - description: Total anonymous-visible distinct challenges for this role. + description: > + Total anonymous-visible distinct challenges for this role. Copilot + totals include only completed or cancelled challenges. trackCounts: description: Present only for Copilot details. $ref: '#/definitions/SpecialRoleTrackCounts' diff --git a/src/services/SpecialRoleService.ts b/src/services/SpecialRoleService.ts index 43a9299..8cfdbfc 100644 --- a/src/services/SpecialRoleService.ts +++ b/src/services/SpecialRoleService.ts @@ -132,6 +132,9 @@ function buildFulfillment (completed, cancelled) { /** * Build the reusable SQL common-table expressions for one role's anonymous- * visible challenges. Resource assignments are de-duplicated per challenge. + * Copilot summaries, history, and metrics include only completed or cancelled + * challenges; Reviewer queries retain all statuses. Client-request cancellations + * remain visible but are excluded separately when calculating fulfillment. * Visibility matches challenge-api's anonymous baseline: no groups, no user * whitelist rows, and no tasks. This helper only constructs SQL and does not * execute or raise database errors. @@ -168,6 +171,12 @@ function buildVisibleRoleChallengesCte (userId, role) { ON challenge."id" = roleAssignment."challengeId" WHERE cardinality(challenge."groups") = 0 AND challenge."taskIsTask" = FALSE + ${role === COPILOT_ROLE + ? ChallengesPrisma.sql`AND ( + challenge."status" = 'COMPLETED' + OR challenge."status"::text LIKE 'CANCELLED%' + )` + : ChallengesPrisma.empty} AND NOT EXISTS ( SELECT 1 FROM challenges."ChallengeUserWhitelist" AS whitelist @@ -180,7 +189,8 @@ function buildVisibleRoleChallengesCte (userId, role) { /** * Count distinct anonymous-visible challenges for one role. The profile * summary uses this count so it matches the anonymous-visible detail set and - * never discloses restricted challenge assignments. + * never discloses restricted challenge assignments. Copilot counts include + * only completed or cancelled challenges. * @param {BigInt|Number|String} userId member user ID from the members database * @param {String} role `copilot` or `reviewer` * @returns {Promise} visible distinct challenge count @@ -198,8 +208,9 @@ async function countVisibleRoleChallenges (userId, role) { } /** - * Load every anonymous-visible challenge detail. PostgreSQL - * orders lifecycle end date, then start/creation fallback, newest first; latest + * Load every anonymous-visible challenge detail, limited to completed or + * cancelled challenges for Copilots. PostgreSQL orders lifecycle end date, + * then start/creation fallback, newest first; latest * Resource assignment and challenge ID are deterministic tie-breakers. The * Resource timestamp is assignment time, not a literal review-completion date. * @param {BigInt|Number|String} userId member user ID from the members database @@ -240,8 +251,8 @@ async function loadVisibleRoleChallenges (userId, role) { /** * Aggregate Copilot tracks and terminal statuses entirely in PostgreSQL over - * the anonymous-visible challenge set. Only the clicked Copilot detail route - * invokes this query; zero-count tracks are naturally omitted from the result. + * the anonymous-visible completed or cancelled challenge set. Only the Copilot + * detail route invokes this query; zero-count tracks are omitted from the result. * @param {BigInt|Number|String} userId member user ID from the members database * @returns {Promise} non-zero track counts and fulfillment statistics * @throws {Error} propagates cross-schema PostgreSQL query failures @@ -338,7 +349,8 @@ function formatChallenge (challenge) { * Get public Copilot and Reviewer badge counts for a member profile. The * summary reuses the detail view's anonymous-visible challenge set so its * counts match the role details. Roles with no visible challenges are omitted, - * and challenges with multiple reviewer assignments count once. + * and challenges with multiple reviewer assignments count once. Copilot counts + * exclude drafts and other non-terminal challenges. * @param {String} handle member handle resolved through the members table * @returns {Promise} optional count-only Copilot/Reviewer summaries * @throws {NotFoundError} when the member handle does not exist @@ -372,8 +384,9 @@ getMemberRoleStats.schema = { /** * Get every newest-first anonymous-visible Copilot or Reviewer challenge for a - * member. Copilot responses also include public-set track counts and terminal - * fulfillment. Details never return restricted challenge IDs. + * member. Copilot history and track counts include only completed or cancelled + * challenges, with client-request cancellations excluded from fulfillment. + * Details never return restricted challenge IDs. * @param {String} handle member handle resolved through the members table * @param {String} role `copilot` or `reviewer` * @returns {Promise} complete challenge list and aggregate role metrics diff --git a/test/integration/SpecialRoleService.test.js b/test/integration/SpecialRoleService.test.js new file mode 100644 index 0000000..3317bf5 --- /dev/null +++ b/test/integration/SpecialRoleService.test.js @@ -0,0 +1,246 @@ +/** + * PostgreSQL regression coverage for PM-5966. Set SPECIAL_ROLE_TEST_DB_URL to + * an empty test database; the suite creates both schemas inside a transaction + * and rolls back all fixtures on completion. See ReadMe.md for the command. + */ +const path = require('path') +const { expect } = require('chai') +const { Client } = require('pg') +const { Prisma } = require('../../prisma/generated/challenge-client') + +const describePostgres = process.env.SPECIAL_ROLE_TEST_DB_URL ? describe : describe.skip +const servicePath = path.resolve(__dirname, '../../src/services/SpecialRoleService.ts') +const helperPath = path.resolve(__dirname, '../../src/common/helper.ts') +const loggerPath = path.resolve(__dirname, '../../src/common/logger.ts') +const prismaPath = path.resolve(__dirname, '../../src/common/prisma.ts') +const cancelledStatuses = [ + 'CANCELLED', + 'CANCELLED_FAILED_REVIEW', + 'CANCELLED_FAILED_SCREENING', + 'CANCELLED_ZERO_SUBMISSIONS', + 'CANCELLED_WINNER_UNRESPONSIVE', + 'CANCELLED_CLIENT_REQUEST', + 'CANCELLED_REQUIREMENTS_INFEASIBLE', + 'CANCELLED_ZERO_REGISTRATIONS', + 'CANCELLED_PAYMENT_FAILED' +] + +describePostgres('special role service PostgreSQL regressions', function () { + this.timeout(20000) + let client + let service + const originalModules = new Map() + + before(async () => { + client = new Client({ connectionString: process.env.SPECIAL_ROLE_TEST_DB_URL }) + await client.connect() + await client.query('BEGIN') + // Existing schemas cause setup to fail; no existing application tables are reused. + await client.query(` + CREATE SCHEMA challenges; + CREATE SCHEMA resources; + CREATE TYPE challenges."ChallengeStatusEnum" AS ENUM ( + 'NEW', 'DRAFT', 'APPROVED', 'ACTIVE', 'COMPLETED', 'DELETED', + 'CANCELLED', 'CANCELLED_FAILED_REVIEW', 'CANCELLED_FAILED_SCREENING', + 'CANCELLED_ZERO_SUBMISSIONS', 'CANCELLED_WINNER_UNRESPONSIVE', + 'CANCELLED_CLIENT_REQUEST', 'CANCELLED_REQUIREMENTS_INFEASIBLE', + 'CANCELLED_ZERO_REGISTRATIONS', 'CANCELLED_PAYMENT_FAILED' + ); + CREATE TABLE challenges."Challenge" ( + "id" text PRIMARY KEY, "name" text, + "status" challenges."ChallengeStatusEnum", + "trackId" text, "typeId" text DEFAULT 'challenge', + "startDate" timestamptz, "endDate" timestamptz, + "createdAt" timestamptz DEFAULT '2026-08-01T00:00:00Z', + "groups" text[] DEFAULT '{}', "taskIsTask" boolean DEFAULT false + ); + CREATE TABLE challenges."ChallengeTrack" ( + "id" text PRIMARY KEY, "name" text, "track" text, "abbreviation" text + ); + CREATE TABLE challenges."ChallengeType" ("id" text PRIMARY KEY, "name" text); + CREATE TABLE challenges."ChallengeUserWhitelist" ("challengeId" text); + CREATE TABLE resources."ResourceRole" ("id" text PRIMARY KEY, "nameLower" text); + CREATE TABLE resources."Resource" ( + "challengeId" text, "memberId" text, "roleId" text, + "createdAt" timestamptz DEFAULT '2026-08-01T00:00:00Z' + ); + INSERT INTO challenges."ChallengeType" VALUES ('challenge', 'Challenge'); + INSERT INTO challenges."ChallengeTrack" VALUES + ('dev', 'Development', 'DEVELOPMENT', 'DEV'), + ('design', 'Design', 'DESIGN', 'DES'), + ('ds', 'Data Science', 'DATA_SCIENCE', 'DS'), + ('qa', 'Quality Assurance', 'QUALITY_ASSURANCE', 'QA'); + INSERT INTO resources."ResourceRole" VALUES + ('copilot', 'copilot'), ('reviewer', 'reviewer'), ('screener', 'primary screener'); + `) + + const stubs = { + [helperPath]: { getMemberByHandle: async () => ({ userId: BigInt(1) }) }, + [loggerPath]: { buildService: () => {} }, + [prismaPath]: { + ChallengesPrisma: Prisma, + getChallengesClient: () => ({ + $queryRaw: async query => (await client.query(query.text, query.values)).rows + }) + } + } + for (const modulePath of [servicePath, ...Object.keys(stubs)]) { + originalModules.set(modulePath, require.cache[modulePath]) + delete require.cache[modulePath] + if (stubs[modulePath]) { + require.cache[modulePath] = { + id: modulePath, filename: modulePath, loaded: true, exports: stubs[modulePath] + } + } + } + service = require(servicePath) + }) + + beforeEach(async () => { + await client.query(` + TRUNCATE challenges."Challenge", challenges."ChallengeUserWhitelist", resources."Resource"; + `) + }) + + after(async () => { + for (const [modulePath, originalModule] of originalModules) { + delete require.cache[modulePath] + if (originalModule) require.cache[modulePath] = originalModule + } + if (client) { + try { + await client.query('ROLLBACK') + } finally { + await client.end() + } + } + }) + + /** + * Seed real challenge and resource rows for a service regression scenario. + * @param {Array} challenges rows with id, status, and optional track, + * groups, task, or whitelisted visibility attributes + * @param {String} role resource role assigned to the test member (default copilot) + * @returns {Promise} resolves after every fixture has been inserted + * @throws {Error} propagates PostgreSQL fixture insertion failures + */ + async function seedChallenges (challenges, role = 'copilot') { + for (const challenge of challenges) { + await client.query(` + INSERT INTO challenges."Challenge" ("id", "name", "status", "trackId", "groups", "taskIsTask") + VALUES ($1, $1, $2, $3, $4, $5) + `, [challenge.id, challenge.status, challenge.track || 'dev', challenge.groups || [], !!challenge.task]) + await client.query(` + INSERT INTO resources."Resource" ("challengeId", "memberId", "roleId") VALUES ($1, '1', $2) + `, [challenge.id, role]) + if (challenge.whitelisted) { + await client.query('INSERT INTO challenges."ChallengeUserWhitelist" VALUES ($1)', [challenge.id]) + } + } + } + + it('returns five terminal challenges and 60% fulfillment for the eight-challenge QA example', async () => { + await seedChallenges([ + { id: 'completed-1', status: 'COMPLETED' }, + { id: 'completed-2', status: 'COMPLETED' }, + { id: 'completed-3', status: 'COMPLETED' }, + { id: 'cancelled-review', status: 'CANCELLED_FAILED_REVIEW', track: 'design' }, + { id: 'cancelled-submissions', status: 'CANCELLED_ZERO_SUBMISSIONS', track: 'design' }, + { id: 'draft-1', status: 'DRAFT', track: 'ds' }, + { id: 'draft-2', status: 'DRAFT', track: 'ds' }, + // Post mortem is a phase of an ACTIVE challenge, not a terminal status. + { id: 'post-mortem', status: 'ACTIVE', track: 'qa' } + ]) + + expect(await service.getMemberRoleStats('copilot')).to.deep.equal({ copilot: { challengeCount: 5 } }) + const result = await service.getMemberRoleChallenges('copilot', 'copilot') + expect(result.total).to.equal(5) + expect(result.challenges.map(challenge => challenge.id)).to.deep.equal([ + 'cancelled-review', 'cancelled-submissions', 'completed-1', 'completed-2', 'completed-3' + ]) + expect(result.trackCounts).to.deep.equal({ DEVELOPMENT: 3, DESIGN: 2 }) + expect(result.fulfillment).to.deep.equal({ completed: 3, cancelled: 2, total: 5, rate: 60 }) + }) + + it('omits the Copilot badge and metrics when every challenge is non-terminal', async () => { + await seedChallenges(['NEW', 'DRAFT', 'APPROVED', 'ACTIVE', 'DELETED'].map(status => ({ id: status, status }))) + + expect(await service.getMemberRoleStats('copilot')).to.deep.equal({}) + expect(await service.getMemberRoleChallenges('copilot', 'copilot')).to.deep.equal({ + role: 'copilot', total: 0, challenges: [], trackCounts: {}, + fulfillment: { completed: 0, cancelled: 0, total: 0, rate: 0 } + }) + }) + + it('retains every cancellation type while excluding client requests from failures', async () => { + await seedChallenges(cancelledStatuses.map(status => ({ id: status, status }))) + + expect(await service.getMemberRoleStats('copilot')).to.deep.equal({ copilot: { challengeCount: 9 } }) + const result = await service.getMemberRoleChallenges('copilot', 'copilot') + expect(result.total).to.equal(9) + expect(result.challenges.map(challenge => challenge.status)).to.have.members(cancelledStatuses) + expect(result.trackCounts).to.deep.equal({ DEVELOPMENT: 9 }) + expect(result.fulfillment).to.deep.equal({ completed: 0, cancelled: 8, total: 8, rate: 0 }) + }) + + it('returns zero fulfillment with a visible history when only client cancellations exist', async () => { + await seedChallenges([{ id: 'client', status: 'CANCELLED_CLIENT_REQUEST' }]) + + expect(await service.getMemberRoleStats('copilot')).to.deep.equal({ copilot: { challengeCount: 1 } }) + const result = await service.getMemberRoleChallenges('copilot', 'copilot') + expect(result.total).to.equal(1) + expect(result.challenges[0].id).to.equal('client') + expect(result.trackCounts).to.deep.equal({ DEVELOPMENT: 1 }) + expect(result.fulfillment).to.deep.equal({ completed: 0, cancelled: 0, total: 0, rate: 0 }) + }) + + it('keeps fulfillment at 99.5% when client requests account for most cancellations', async () => { + await seedChallenges([ + ...Array.from({ length: 199 }, (_, index) => ({ id: `completed-${index}`, status: 'COMPLETED' })), + ...Array.from({ length: 24 }, (_, index) => ({ id: `client-${index}`, status: 'CANCELLED_CLIENT_REQUEST' })), + { id: 'failed-review', status: 'CANCELLED_FAILED_REVIEW' } + ]) + + expect(await service.getMemberRoleStats('copilot')).to.deep.equal({ copilot: { challengeCount: 224 } }) + const result = await service.getMemberRoleChallenges('copilot', 'copilot') + expect(result.total).to.equal(224) + expect(result.challenges).to.have.length(224) + expect(result.fulfillment).to.deep.equal({ completed: 199, cancelled: 1, total: 200, rate: 99.5 }) + }) + + it('continues to hide restricted challenges and deduplicate resource assignments', async () => { + await seedChallenges([ + { id: 'public', status: 'COMPLETED' }, + { id: 'group', status: 'COMPLETED', groups: ['private'] }, + { id: 'task', status: 'COMPLETED', task: true }, + { id: 'whitelist', status: 'CANCELLED_ZERO_SUBMISSIONS', whitelisted: true } + ]) + await client.query(`INSERT INTO resources."Resource" ("challengeId", "memberId", "roleId") + VALUES ('public', '1', 'copilot')`) + + expect(await service.getMemberRoleStats('copilot')).to.deep.equal({ copilot: { challengeCount: 1 } }) + const result = await service.getMemberRoleChallenges('copilot', 'copilot') + expect(result.challenges.map(challenge => challenge.id)).to.deep.equal(['public']) + expect(result.total).to.equal(1) + expect(result.trackCounts).to.deep.equal({ DEVELOPMENT: 1 }) + expect(result.fulfillment).to.deep.equal({ completed: 1, cancelled: 0, total: 1, rate: 100 }) + }) + + it('preserves Reviewer history across statuses and multiple review roles', async () => { + await seedChallenges([ + { id: 'draft', status: 'DRAFT' }, + { id: 'active', status: 'ACTIVE' }, + { id: 'completed', status: 'COMPLETED' }, + { id: 'cancelled', status: 'CANCELLED_CLIENT_REQUEST' } + ], 'reviewer') + await client.query(`INSERT INTO resources."Resource" ("challengeId", "memberId", "roleId") + VALUES ('completed', '1', 'screener')`) + + expect(await service.getMemberRoleStats('reviewer')).to.deep.equal({ reviewer: { challengeCount: 4 } }) + const result = await service.getMemberRoleChallenges('reviewer', 'reviewer') + expect(result.total).to.equal(4) + expect(result.challenges.map(challenge => challenge.id)).to.deep.equal(['active', 'cancelled', 'completed', 'draft']) + expect(result).not.to.have.property('fulfillment') + expect(result).not.to.have.property('trackCounts') + }) +}) diff --git a/test/unit/SpecialRoleService.test.js b/test/unit/SpecialRoleService.test.js index 13d82d4..f61c684 100644 --- a/test/unit/SpecialRoleService.test.js +++ b/test/unit/SpecialRoleService.test.js @@ -79,10 +79,11 @@ function renderSqlValue (value) { * Create the subset of Prisma's parameterized SQL helper used by the service. * It preserves interpolations in readable text for assertions and does not * connect to a database. - * @returns {Object} `sql` tagged-template and `join` test helpers + * @returns {Object} `sql`, `join`, and `empty` test helpers */ function createPrismaSqlStub () { return { + empty: { __sqlText: '' }, sql: (strings, ...values) => ({ __sqlText: strings.reduce((text, part, index) => ( text + part + (index < values.length ? renderSqlValue(values[index]) : '') @@ -317,8 +318,8 @@ describe('special role service unit tests', () => { }, { id: 'copilot-3', - name: 'Newest Copilot Challenge', - status: 'ACTIVE', + name: 'Failed Review Copilot Challenge', + status: 'CANCELLED_FAILED_REVIEW', startDate: new Date('2024-03-01T00:00:00Z'), endDate: null, resourceCreatedAt: new Date('2024-03-02T00:00:00Z'), @@ -370,7 +371,7 @@ describe('special role service unit tests', () => { challengeCount: 1 }, { - status: 'ACTIVE', + status: 'CANCELLED_FAILED_REVIEW', track: 'DESIGN', trackName: 'Design', trackAbbreviation: 'DES', @@ -400,9 +401,9 @@ describe('special role service unit tests', () => { }) result.fulfillment.should.deep.equal({ completed: 1, - cancelled: 1, - total: 2, - rate: 50 + cancelled: 2, + total: 3, + rate: 33.33 }) result.challenges.should.have.length(4) result.challenges.map(challenge => challenge.id).should.deep.equal([