From cc2b502874619a17197270a1cee5645f51b29c31 Mon Sep 17 00:00:00 2001 From: cheyras Date: Mon, 26 Oct 2026 14:52:42 -0600 Subject: [PATCH 1/2] Knowledge notes review screen, in Administration and in Dev tools Phase 6, part two. One component, mounted at /admin/knowledge (an Administration tab) and /dev/knowledge (a Dev tools entry, for a contributor given knowledge.review, who never holds admin.access). Waiting, approved and rejected notes, with cards, statement, quote, the full source link (noopener noreferrer nofollow) and the automatic-check flags; approve with an optional reason, reject with a required one. Browser case at 1280 in both deployments: text stays text, flags show, a reasonless rejection is not sent, decided notes move tabs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015oYErNsTyzvYNppwoipzdY --- apps/web/src/lib/api.ts | 8 ++ apps/web/src/main.tsx | 10 ++- apps/web/src/routes/Devtools.tsx | 1 + apps/web/src/routes/admin/Admin.tsx | 1 + apps/web/src/routes/admin/Knowledge.tsx | 80 +++++++++++++++++++ ...ewed-in-administration-and-in-dev-tools.md | 33 ++++++++ tests/browser/admin.mjs | 49 +++++++++++- tests/browser/case-inventory.json | 2 + 8 files changed, 181 insertions(+), 3 deletions(-) create mode 100644 apps/web/src/routes/admin/Knowledge.tsx create mode 100644 decisions/2026/2026-10-26-knowledge-notes-are-reviewed-in-administration-and-in-dev-tools.md diff --git a/apps/web/src/lib/api.ts b/apps/web/src/lib/api.ts index 498181ade..7d48845ed 100644 --- a/apps/web/src/lib/api.ts +++ b/apps/web/src/lib/api.ts @@ -31,6 +31,10 @@ export interface DeckeImprovementLeg { id: string; leg: number; asked: string | export interface DeckeImprovementEvent { ordinal: number; batch: number; batchOrdinal: number; legId: string | null; kind: string; payload: unknown } export interface DeckeImprovementTurn { seq: number; asked: string; answered: string; tools: unknown; feedback: number | null; feedbackComment: string | null; offsetSeconds: number; tokens: DeckeImprovementTokens; costUsd: number | null; costCoverage: DeckeImprovementCoverage; buildSha: string | null; buildPr: number | null; finishReason: string | null; hasError: boolean; legs: DeckeImprovementLeg[]; events: DeckeImprovementEvent[] } export interface DeckeImprovementDetail { conversation: DeckeImprovementConversation; turns: DeckeImprovementTurn[] } +/** A knowledge note as a reviewer sees it (`GET /admin/knowledge`). Every text field is the proposer's, not DeckPal's. */ +export type KnowledgeNoteStatus = 'pending' | 'approved' | 'rejected' +export interface KnowledgeNote { id: number; cards: string[]; statement: string; sourceUrl: string; quote: string; status: KnowledgeNoteStatus; proposedAt: string; flags: string[]; reviewedAt: string | null; reviewReason: string | null } +export interface KnowledgeQueue { notes: KnowledgeNote[]; next: number | null } const BASE = isCloudMode ? '/api' : '/deckpal/api' @@ -2296,6 +2300,10 @@ export const api = { /** Shared Deck-E chats (pseudonymised), using the SQL reader's bucketed shape. */ adminDeckeImprovementList: (params: string, signal?: AbortSignal) => get('/admin/decke-improvement?' + params, signal), adminDeckeImprovement: (id: string, signal?: AbortSignal) => get('/admin/decke-improvement/' + encodeURIComponent(id), signal), + /** Knowledge notes waiting for (or past) review; needs knowledge.review in a browser session. */ + adminKnowledgeNotes: (params: string, signal?: AbortSignal) => get('/admin/knowledge?' + params, signal), + adminReviewKnowledgeNote: (id: number, body: { decision: 'approved' | 'rejected'; reason: string }) => + send<{ note: KnowledgeNote }>('POST', `/admin/knowledge/${id}/review`, body), adminAiConversation: (id: string, params: string, signal?: AbortSignal) => get<{ items: AiUsageDetail[]; total: number; limit: number; offset: number }>('/admin/ai-usage/conversations/' + encodeURIComponent(id) + '?' + params, signal), meFeatures: (signal?: AbortSignal) => get<{ features: FeatureAccess[] }>('/me/features', signal), setMeFeature: (key: string, optedIn: boolean, expectedRevision: number) => send<{ features: FeatureAccess[] }>('PATCH', '/me/features/' + encodeURIComponent(key), { optedIn, expectedRevision }), diff --git a/apps/web/src/main.tsx b/apps/web/src/main.tsx index 113362881..c3f6cb585 100644 --- a/apps/web/src/main.tsx +++ b/apps/web/src/main.tsx @@ -92,6 +92,8 @@ const AdminAudit = lazyRoute('./routes/admin/Audit', () => import('./routes/admi const AdminFeatures = lazyRoute('./routes/admin/Features', () => import('./routes/admin/Features'), 'AdminFeatures') const AdminAiUsage = lazyRoute('./routes/admin/AiUsage', () => import('./routes/admin/AiUsage'), 'AdminAiUsage') const AdminDeckeChats = lazyRoute('./routes/admin/DeckeChats', () => import('./routes/admin/DeckeChats'), 'AdminDeckeChats') +const AdminKnowledge = lazyRoute('./routes/admin/Knowledge', () => import('./routes/admin/Knowledge'), 'KnowledgeReview') +const DevKnowledge = lazyRoute('./routes/admin/Knowledge', () => import('./routes/admin/Knowledge'), 'DevKnowledgeReview') // The two things `RootComponent` mounts on every page and almost nobody sees: // Deck-E's host (two accounts) and the support prompt (signed in, cloud only). @@ -257,6 +259,7 @@ const TOOL_PERMISSIONS: Record = { '/devtools': 'devtools.access', '/scan': 'scanner.use', '/design': 'design.view', '/dev/decke': 'diagnostics.view', '/dev/chat-ui': 'diagnostics.view', '/dev/decke-compare': 'diagnostics.view', '/dev/scan-harness': 'diagnostics.view', '/dev/quad-labeler': 'scanner.label', '/dev/quad-harvest': 'scanner.label', + '/dev/knowledge': 'knowledge.review', } function SecureOutlet() { const pathname = useRouterState({ select: s => s.location.pathname }) @@ -840,6 +843,9 @@ const adminRoute = createRoute({ getParentRoute: () => rootRoute, path: '/admin' const adminFeaturesRoute = createRoute({ getParentRoute: () => adminRoute, path: '/features', component: AdminFeatures }) const adminUsageRoute = createRoute({ getParentRoute: () => adminRoute, path: '/usage', component: AdminAiUsage }) const adminDeckeChatsRoute = createRoute({ getParentRoute: () => adminRoute, path: '/decke-chats', beforeLoad: () => requireCapability('decke.improvement.read'), component: AdminDeckeChats }) +const adminKnowledgeRoute = createRoute({ getParentRoute: () => adminRoute, path: '/knowledge', beforeLoad: () => requireCapability('knowledge.review'), component: AdminKnowledge }) +// The same review screen for a contributor given knowledge.review: /admin needs admin.access, which a contributor never holds. +const devKnowledgeRoute = createRoute({ getParentRoute: () => rootRoute, path: '/dev/knowledge', beforeLoad: () => requireCapability('knowledge.review'), component: DevKnowledge }) const adminIndexRoute = createRoute({ getParentRoute: () => adminRoute, path: '/', component: AdminOverview }) const adminUsersRoute = createRoute({ getParentRoute: () => adminRoute, path: '/users', beforeLoad: () => requireCapability('users.read'), component: AdminUsers }) const adminUserRoute = createRoute({ getParentRoute: () => adminRoute, path: '/users/$userId', beforeLoad: () => requireCapability('users.read'), component: AdminUserDetail }) @@ -857,8 +863,8 @@ const creditsRoute = createRoute({ getParentRoute: () => rootRoute, path: '/cred }), component: Credits }) const routeTree = rootRoute.addChildren([ - devtoolsRoute, adminToolsRoute, - adminRoute.addChildren([adminFeaturesRoute, adminUsageRoute, adminDeckeChatsRoute, adminIndexRoute, adminUsersRoute, adminUserRoute, adminRolesRoute, adminSettingsRoute, adminAuditRoute]), + devtoolsRoute, adminToolsRoute, devKnowledgeRoute, + adminRoute.addChildren([adminFeaturesRoute, adminUsageRoute, adminDeckeChatsRoute, adminKnowledgeRoute, adminIndexRoute, adminUsersRoute, adminUserRoute, adminRolesRoute, adminSettingsRoute, adminAuditRoute]), creditsRoute, ...coreRoutes, designRoute, diff --git a/apps/web/src/routes/Devtools.tsx b/apps/web/src/routes/Devtools.tsx index 15df922ac..d15691d62 100644 --- a/apps/web/src/routes/Devtools.tsx +++ b/apps/web/src/routes/Devtools.tsx @@ -9,6 +9,7 @@ export const DEV_TOOLS = [ { to: '/dev/scan-harness', name: 'Scanner harness', permission: 'diagnostics.view', description: 'Exercise detector fixtures and camera diagnostics.' }, { to: '/dev/quad-labeler', name: 'Quad labeler', permission: 'scanner.label', description: 'Capture and annotate training photos in the shared photo corpus.' }, { to: '/dev/quad-harvest', name: 'Training photo review', permission: 'scanner.label', description: 'Inspect captured training photos and labels.' }, + { to: '/dev/knowledge', name: 'Knowledge notes', permission: 'knowledge.review', description: 'Approve or reject the rules notes agents propose before every account sees them.' }, ] export function Devtools() { const access = useAccess() diff --git a/apps/web/src/routes/admin/Admin.tsx b/apps/web/src/routes/admin/Admin.tsx index 032ac49e8..064921e3d 100644 --- a/apps/web/src/routes/admin/Admin.tsx +++ b/apps/web/src/routes/admin/Admin.tsx @@ -13,6 +13,7 @@ export const ADMIN_SECTIONS = [ { key: 'audit', label: 'Audit', to: '/admin/audit', permission: 'audit.read' }, { key: 'usage', label: 'AI usage', to: '/admin/usage', permission: 'admin.access' }, { key: 'decke-chats', label: 'Deck-E chats', to: '/admin/decke-chats', permission: 'decke.improvement.read' }, + { key: 'knowledge', label: 'Knowledge notes', to: '/admin/knowledge', permission: 'knowledge.review' }, { key: 'features', label: 'Features', to: '/admin/features', permission: 'roles.manage' }, ] export default function Admin() { diff --git a/apps/web/src/routes/admin/Knowledge.tsx b/apps/web/src/routes/admin/Knowledge.tsx new file mode 100644 index 000000000..20b78273f --- /dev/null +++ b/apps/web/src/routes/admin/Knowledge.tsx @@ -0,0 +1,80 @@ +import { useState } from 'react' +import { useQuery } from '@tanstack/react-query' +import { api, type KnowledgeNote, type KnowledgeNoteStatus } from '../../lib/api' +import { useAccess } from '../../lib/access' +import { Button, Content, DataTable, EmptyState, Tabs } from '../../components/ui' +import { ConfirmAction, fmtDate } from './shared' + +/** + * Knowledge notes review (Phase 6): what agents proposed about how cards interact, approved or rejected here before + * every account sees it in get_card and rules_lookup. + * + * Gated on knowledge.review alone, never admin.access: a contributor role can be given review, and a contributor + * never holds admin.access. So this component does not use useAdminQuery (which requires admin.access), and it is + * mounted twice: as an Administration tab for admins, and at /dev/knowledge for contributors. + * + * Every text field is the proposer's. React escapes it; the source link opens in a new tab with no referrer and is + * shown in full, so the reviewer sees where it goes before following it. + */ +const STATUSES: { key: KnowledgeNoteStatus; label: string }[] = [ + { key: 'pending', label: 'Waiting' }, + { key: 'approved', label: 'Approved' }, + { key: 'rejected', label: 'Rejected' }, +] + +function NoteBody({ note }: { note: KnowledgeNote }) { + return
+

{note.cards.join(' · ')}

+

{note.statement}

+
“{note.quote}”
+ {note.sourceUrl} + {note.flags.length > 0 &&
    {note.flags.map(flag => +
  • {flag}
    • )}
} + {note.reviewReason &&

Review note: {note.reviewReason}

} +
+} + +export function KnowledgeReview() { + const access = useAccess() + const [status, setStatus] = useState('pending') + const [before, setBefore] = useState(null) + const [deciding, setDeciding] = useState<{ note: KnowledgeNote; decision: 'approved' | 'rejected' } | null>(null) + const allowed = access.ready && access.permissions.includes('knowledge.review') + const params = new URLSearchParams({ status, limit: '50', ...(before ? { before: String(before) } : {}) }).toString() + // The key starts with 'admin' so ConfirmAction's save refreshes it after a decision. + const query = useQuery({ queryKey: ['admin', access.identity, access.revision, 'knowledge', params], queryFn: ({ signal }) => api.adminKnowledgeNotes(params, signal), enabled: allowed, retry: false, staleTime: 0 }) + if (!allowed) return + return
+

Knowledge notes

+

Findings agents proposed about how cards interact. An approved note appears for every account in card details and rules answers, under the note’s source. Check the quote against the source before approving; reject anything wrong, unsourced or personal.

+ { setStatus(key as KnowledgeNoteStatus); setBefore(null) }} items={STATUSES} /> + String(note.id)} getRowLabel={note => note.cards.join(', ')} + loading={query.isPending} refreshing={query.isFetching && !query.isPending} error={query.error?.message} onRetry={() => void query.refetch()} + empty={} + columns={[ + { id: 'note', header: 'Note', className: 'min-w-[320px] max-w-[640px]', cell: note => }, + { id: 'proposed', header: 'Proposed', className: 'min-w-[150px]', cell: note => }, + status === 'pending' + ? { id: 'decide', header: 'Decision', className: 'min-w-[200px]', cell: note =>
+ + +
} + : { id: 'reviewed', header: 'Decided', className: 'min-w-[150px]', cell: note => note.reviewedAt ? : '—' }, + ]} /> + {query.data?.next && } + {before && } + {deciding && api.adminReviewKnowledgeNote(deciding.note.id, { decision: deciding.decision, reason })} + close={() => setDeciding(null)} />} +
+} + +/** /dev/knowledge: the same screen for a contributor, who has Dev tools but not Administration. */ +export function DevKnowledgeReview() { + return
+} diff --git a/decisions/2026/2026-10-26-knowledge-notes-are-reviewed-in-administration-and-in-dev-tools.md b/decisions/2026/2026-10-26-knowledge-notes-are-reviewed-in-administration-and-in-dev-tools.md new file mode 100644 index 000000000..c7c502af0 --- /dev/null +++ b/decisions/2026/2026-10-26-knowledge-notes-are-reviewed-in-administration-and-in-dev-tools.md @@ -0,0 +1,33 @@ +--- +date: "2026-10-26" +title: "Knowledge notes are reviewed in Administration and in Dev tools" +decided_by: "Chey (via Claude)" +areas: ["web", "admin"] +supersedes: [] +--- +## 2026-10-26 — Knowledge notes are reviewed in Administration and in Dev tools +**Decided by:** Chey (via Claude) + +**Decision:** The review screen for knowledge notes (Phase 6, part two) is one component (`routes/admin/Knowledge.tsx`), mounted in two places: +- **Administration:** the "Knowledge notes" tab at `/admin/knowledge`, for admins. +- **Dev tools:** the "Knowledge notes" entry at `/dev/knowledge`, for anyone else holding `knowledge.review`. + +Both routes require `knowledge.review` and nothing else. The screen does not use `useAdminQuery`, which requires `admin.access`. + +On the screen, a reviewer: +- sees what is waiting, approved or rejected; +- reads each note's cards, statement, quote, full source link and automatic-check flags; +- approves (a reason is optional); +- or rejects (a reason is required). + +Each decision is final and audited (#358). + +**Why:** +- The owner asked for an administration surface that "users/admins of a certain reasonable tier" can reach. #358 lets a contributor role (tier 30) be given `knowledge.review`. +- `/admin` requires `admin.access`, which a contributor can never hold (the tier ceiling). A tab only under Administration would make that grant useless. +- Contributors already reach Dev tools. + +**Implications:** +- The source link opens in a new tab with `noopener noreferrer nofollow` and is printed in full, so the reviewer sees where it goes first. All proposer text renders as text. +- The browser suite covers the screen at 1280 px: text stays text, flags show, a rejection without a reason is not sent, and decided notes move to their tab. +- The chunk is precached like the other Administration screens. Only dev-only routes are excluded from the precache. diff --git a/tests/browser/admin.mjs b/tests/browser/admin.mjs index 32bd0a5bb..0af8a2bef 100644 --- a/tests/browser/admin.mjs +++ b/tests/browser/admin.mjs @@ -26,10 +26,15 @@ export function adminFixture(mount) { state.audit = Array.from({ length: 36 }, (_, i) => ({ id: 'audit-' + i, actorId: i % 2 ? USER : OWNER, actorName: i % 2 ? 'Future Contributor' : 'Owner', action: i % 3 ? 'role.update' : 'user.suspend', targetType: 'user', targetId: i % 2 ? USER : OWNER, before: { suspended: false }, after: { suspended: true, note: '' }, reason: 'Reviewed change ' + i, createdAt: new Date(Date.parse(now) - i * 60000).toISOString() })) state.orders = Array.from({ length: 38 }, (_, i) => ({ id: 'credit-order-' + i, userId: i % 2 ? USER : OWNER, username: i % 2 ? 'Future Contributor' : 'Owner', packName: 'Starter', credits: 500, priceCents: 500, currency: 'usd', status: i % 3 === 0 ? 'refunded' : 'paid', refundedCents: i % 3 === 0 ? 100 : 0, reversedCredits: i % 3 === 0 ? 100 : 0, disputeStatus: i === 0 ? 'closed' : null, createdAt: now, paidAt: now })) state.ledger = Array.from({ length: 32 }, (_, i) => ({ id: 'ledger-' + i, delta: i % 2 ? -5 : 20, debtDelta: i === 0 ? -10 : 0, kind: i % 2 ? 'usage' : 'adjustment', reason: 'Ledger review ' + i, createdAt: now, pricingRevision: 1 })) + // Knowledge notes waiting for review: one carries markup (it must stay text), one a reviewer flag. + state.knowledge = [ + { id: 3, cards: ['nighttime mine', 'dragapult ex'], statement: 'Nighttime Mine makes Phantom Dive cost one more Colorless. ', sourceUrl: 'https://www.pokemon.com/us/pokemon-tcg/rules', quote: 'Attacks used by each Tera Pokémon in play cost one more Colorless Energy.', status: 'pending', proposedAt: now, flags: [], reviewedAt: null, reviewReason: null }, + { id: 2, cards: ['boss\'s orders'], statement: 'Boss\'s Orders can bring up a Pokémon with no Retreat Cost.', sourceUrl: 'https://example.com/forum/thread', quote: 'Switch in 1 of your opponent\'s Benched Pokémon.', status: 'pending', proposedAt: now, flags: ['source is not a rulings site reviewers know'], reviewedAt: null, reviewReason: null }, + ] initFeedback(state, PERMISSIONS) const allowMutation = (pathname, method) => { const rel = pathname.slice(mount.length) - return ['POST','PUT','PATCH','DELETE'].includes(method) && ((state.chatProbe && ['/api/chat','/api/decke/history'].includes(rel)) || /^\/api\/(?:me\/(?:features\/[^/]+|decke-sharing|settings)|admin\/(?:features\/[^/]+|users\/[^/]+\/(?:role|ai-override)))$/.test(rel) || /^\/api\/admin\/(roles(?:\/[^/]+)?|settings|users\/[^/]+\/(roles|status|revoke-tokens)|credits\/(settings|packs(?:\/[^/]+)?|users\/[^/]+\/(adjustments|resolve-hold)))$/.test(rel) || rel === '/api/me/credits/checkout' || (method === 'POST' && rel === '/api/me/billing/visit')) + return ['POST','PUT','PATCH','DELETE'].includes(method) && ((state.chatProbe && ['/api/chat','/api/decke/history'].includes(rel)) || /^\/api\/(?:me\/(?:features\/[^/]+|decke-sharing|settings)|admin\/(?:features\/[^/]+|users\/[^/]+\/(?:role|ai-override)))$/.test(rel) || /^\/api\/admin\/(roles(?:\/[^/]+)?|settings|users\/[^/]+\/(roles|status|revoke-tokens)|credits\/(settings|packs(?:\/[^/]+)?|users\/[^/]+\/(adjustments|resolve-hold)))$/.test(rel) || rel === '/api/me/credits/checkout' || (method === 'POST' && rel === '/api/me/billing/visit') || (method === 'POST' && /^\/api\/admin\/knowledge\/\d+\/review$/.test(rel))) } const response = (rel, url, req = { method: 'GET' }) => { const { method, body } = req @@ -144,6 +149,18 @@ export function adminFixture(mount) { const orders = state.orders.filter(order => (!user || order.userId === user) && (!status || order.status === status)) return ok({orders:orders.slice(offset,offset+limit),total:orders.length,offset,limit}) } + if (rel === '/api/admin/knowledge') { + const status = url.searchParams.get('status') ?? 'pending' + return ok({ notes: state.knowledge.filter(n => n.status === status), next: null }) + } + if (/^\/api\/admin\/knowledge\/\d+\/review$/.test(rel)) { + assert.equal(method, 'POST') + assert.deepEqual(Object.keys(body).sort(), ['decision', 'reason']) + const note = state.knowledge.find(n => n.id === Number(rel.split('/')[4])) + if (!note || note.status !== 'pending') return { status: 404, body: { error: { message: 'That note is not waiting for review.' } } } + Object.assign(note, { status: body.decision, reviewedAt: now, reviewReason: body.reason || null }) + return ok({ note }) + } if (rel === '/api/admin/audit') { const actor = url.searchParams.get('actor'), action = url.searchParams.get('action'), target = url.searchParams.get('target'), limit = Number(url.searchParams.get('limit') ?? 25), offset = Number(url.searchParams.get('offset') ?? 0) const events = state.audit.filter(event => (!actor || event.actorId === actor) && (!action || event.action === action) && (!target || event.targetId === target)) @@ -514,6 +531,36 @@ async function checkAdminTables(browser, server, mount, label, out, fixture, wid assert.equal(await auditTable.locator('tbody > tr').count(),36) results.push({case:'admin-audit-table-exact-filters-safe-keyboard-details',label,width}) + if(width===1280){ + // Knowledge notes review: proposer text stays text, flags show, approve needs no reason, reject needs one. + state.permissions=[...PERMISSIONS,'knowledge.review'] + await go(page,'/admin/knowledge') + const notes=page.getByRole('table',{name:'Knowledge notes',exact:true}) + await notes.getByText('Boss\'s Orders can bring up a Pokémon with no Retreat Cost.',{exact:true}).waitFor() + assert.equal(await notes.locator('tbody > tr').count(),2) + assert.equal(await page.evaluate(()=>window.fixtureUnsafe),undefined,'A note\'s text must stay text') + await notes.getByText('source is not a rulings site reviewers know',{exact:true}).waitFor() + assert.equal(await notes.getByRole('link',{name:'https://www.pokemon.com/us/pokemon-tcg/rules',exact:true}).getAttribute('rel'),'noopener noreferrer nofollow') + await notes.getByRole('button',{name:'Reject',exact:true}).last().click() + const reject=page.getByRole('dialog',{name:'Reject note'}) + await reject.getByRole('button',{name:'Reject note',exact:true}).click() + assert.equal(state.knowledge[1].status,'pending','A rejection without a reason is not sent') + await reject.getByLabel('Reason').fill('Not a ruling; a forum guess.') + await reject.getByRole('button',{name:'Reject note',exact:true}).click() + await reject.waitFor({state:'detached'}) + assert.deepEqual([state.knowledge[1].status,state.knowledge[1].reviewReason],['rejected','Not a ruling; a forum guess.']) + await notes.getByRole('button',{name:'Approve',exact:true}).click() + const approve=page.getByRole('dialog',{name:'Approve note'}) + await approve.getByRole('button',{name:'Approve note',exact:true}).click() + await approve.waitFor({state:'detached'}) + assert.equal(state.knowledge[0].status,'approved') + await page.getByText('Nothing waiting',{exact:true}).waitFor() + await page.getByRole('button',{name:'Rejected',exact:true}).click() + await notes.getByText('Review note: Not a ruling; a forum guess.',{exact:true}).waitFor() + state.permissions=[...PERMISSIONS] + results.push({case:'admin-knowledge-review-approve-reject-text-safe',label,width}) + } + await go(page,'/admin/settings') const packs=page.getByRole('region',{name:'Credit packs',exact:true}), packTable=packs.getByRole('table') await packs.getByRole('status').filter({hasText:'1–25 of'}).waitFor();assert.equal(await packTable.locator('tbody > tr').count(),25) diff --git a/tests/browser/case-inventory.json b/tests/browser/case-inventory.json index 88684707e..1bf889497 100644 --- a/tests/browser/case-inventory.json +++ b/tests/browser/case-inventory.json @@ -35,6 +35,8 @@ "{\"case\":\"admin-audit-table-exact-filters-safe-keyboard-details\",\"label\":\"cloud\",\"width\":390}", "{\"case\":\"admin-audit-table-exact-filters-safe-keyboard-details\",\"label\":\"selfhost\",\"width\":1280}", "{\"case\":\"admin-audit-table-exact-filters-safe-keyboard-details\",\"label\":\"selfhost\",\"width\":390}", + "{\"case\":\"admin-knowledge-review-approve-reject-text-safe\",\"label\":\"cloud\",\"width\":1280}", + "{\"case\":\"admin-knowledge-review-approve-reject-text-safe\",\"label\":\"selfhost\",\"width\":1280}", "{\"case\":\"admin-ledger-table-fixed-pagination-debt\",\"label\":\"cloud\",\"width\":1280}", "{\"case\":\"admin-ledger-table-fixed-pagination-debt\",\"label\":\"cloud\",\"width\":390}", "{\"case\":\"admin-ledger-table-fixed-pagination-debt\",\"label\":\"selfhost\",\"width\":1280}", From a4b5141c85aa821b51a830a657124b1f17bc32cf Mon Sep 17 00:00:00 2001 From: cheyras Date: Mon, 26 Oct 2026 14:55:54 -0600 Subject: [PATCH 2/2] Approving a knowledge note offers an optional reason (Astra round 1) ConfirmAction gains reasonOptional: the Reason field is shown but not demanded. The browser case approves one note with a reason and one without and checks both bodies. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015oYErNsTyzvYNppwoipzdY --- apps/web/src/routes/admin/Knowledge.tsx | 1 + apps/web/src/routes/admin/shared.tsx | 6 ++++-- tests/browser/admin.mjs | 20 ++++++++++++++------ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/apps/web/src/routes/admin/Knowledge.tsx b/apps/web/src/routes/admin/Knowledge.tsx index 20b78273f..e269feb1e 100644 --- a/apps/web/src/routes/admin/Knowledge.tsx +++ b/apps/web/src/routes/admin/Knowledge.tsx @@ -69,6 +69,7 @@ export function KnowledgeReview() { ? `Every account will see this note about ${deciding.note.cards.join(', ')}. A decision is final and recorded in the audit log.` : 'The proposer will see it as rejected; nobody else ever sees it. Say why, briefly.'} reasonRequired={deciding.decision === 'rejected'} + reasonOptional action={reason => api.adminReviewKnowledgeNote(deciding.note.id, { decision: deciding.decision, reason })} close={() => setDeciding(null)} />} diff --git a/apps/web/src/routes/admin/shared.tsx b/apps/web/src/routes/admin/shared.tsx index eee426464..d9151a792 100644 --- a/apps/web/src/routes/admin/shared.tsx +++ b/apps/web/src/routes/admin/shared.tsx @@ -39,8 +39,9 @@ export function useAdminSave() { } return { busy, error, success, save } } -export function ConfirmAction({ title, description, action, close, reasonRequired = true }: { - title: string; description: string; action: (reason: string) => Promise; close: () => void; reasonRequired?: boolean +/** `reasonRequired` (default) demands a reason; `reasonOptional` shows the field without demanding it; neither hides it. */ +export function ConfirmAction({ title, description, action, close, reasonRequired = true, reasonOptional = false }: { + title: string; description: string; action: (reason: string) => Promise; close: () => void; reasonRequired?: boolean; reasonOptional?: boolean }) { const [reason, setReason] = useState('') const state = useAdminSave() @@ -48,6 +49,7 @@ export function ConfirmAction({ title, description, action, close, reasonRequire return { if (state.busy) return false; close() }}>

{description}

{reasonRequired && setReason(e.target.value)} hint="Recorded in the administrative audit log." />} + {!reasonRequired && reasonOptional && setReason(e.target.value)} hint="Recorded in the administrative audit log." />} {state.error && {state.error}}
diff --git a/tests/browser/admin.mjs b/tests/browser/admin.mjs index 0af8a2bef..451534ffe 100644 --- a/tests/browser/admin.mjs +++ b/tests/browser/admin.mjs @@ -28,6 +28,7 @@ export function adminFixture(mount) { state.ledger = Array.from({ length: 32 }, (_, i) => ({ id: 'ledger-' + i, delta: i % 2 ? -5 : 20, debtDelta: i === 0 ? -10 : 0, kind: i % 2 ? 'usage' : 'adjustment', reason: 'Ledger review ' + i, createdAt: now, pricingRevision: 1 })) // Knowledge notes waiting for review: one carries markup (it must stay text), one a reviewer flag. state.knowledge = [ + { id: 4, cards: ['jamming tower'], statement: 'Jamming Tower stops the effects of Pokémon Tools in play, both players\' alike.', sourceUrl: 'https://www.pokemon.com/us/pokemon-tcg/rules', quote: 'Pokémon Tools attached to each Pokémon (both yours and your opponent\'s) have no effect.', status: 'pending', proposedAt: now, flags: [], reviewedAt: null, reviewReason: null }, { id: 3, cards: ['nighttime mine', 'dragapult ex'], statement: 'Nighttime Mine makes Phantom Dive cost one more Colorless. ', sourceUrl: 'https://www.pokemon.com/us/pokemon-tcg/rules', quote: 'Attacks used by each Tera Pokémon in play cost one more Colorless Energy.', status: 'pending', proposedAt: now, flags: [], reviewedAt: null, reviewReason: null }, { id: 2, cards: ['boss\'s orders'], statement: 'Boss\'s Orders can bring up a Pokémon with no Retreat Cost.', sourceUrl: 'https://example.com/forum/thread', quote: 'Switch in 1 of your opponent\'s Benched Pokémon.', status: 'pending', proposedAt: now, flags: ['source is not a rulings site reviewers know'], reviewedAt: null, reviewReason: null }, ] @@ -537,23 +538,30 @@ async function checkAdminTables(browser, server, mount, label, out, fixture, wid await go(page,'/admin/knowledge') const notes=page.getByRole('table',{name:'Knowledge notes',exact:true}) await notes.getByText('Boss\'s Orders can bring up a Pokémon with no Retreat Cost.',{exact:true}).waitFor() - assert.equal(await notes.locator('tbody > tr').count(),2) + const kn=id=>state.knowledge.find(n=>n.id===id) + assert.equal(await notes.locator('tbody > tr').count(),3) assert.equal(await page.evaluate(()=>window.fixtureUnsafe),undefined,'A note\'s text must stay text') await notes.getByText('source is not a rulings site reviewers know',{exact:true}).waitFor() - assert.equal(await notes.getByRole('link',{name:'https://www.pokemon.com/us/pokemon-tcg/rules',exact:true}).getAttribute('rel'),'noopener noreferrer nofollow') + assert.equal(await notes.getByRole('link',{name:'https://www.pokemon.com/us/pokemon-tcg/rules',exact:true}).first().getAttribute('rel'),'noopener noreferrer nofollow') await notes.getByRole('button',{name:'Reject',exact:true}).last().click() const reject=page.getByRole('dialog',{name:'Reject note'}) await reject.getByRole('button',{name:'Reject note',exact:true}).click() - assert.equal(state.knowledge[1].status,'pending','A rejection without a reason is not sent') + assert.equal(kn(2).status,'pending','A rejection without a reason is not sent') await reject.getByLabel('Reason').fill('Not a ruling; a forum guess.') await reject.getByRole('button',{name:'Reject note',exact:true}).click() await reject.waitFor({state:'detached'}) - assert.deepEqual([state.knowledge[1].status,state.knowledge[1].reviewReason],['rejected','Not a ruling; a forum guess.']) - await notes.getByRole('button',{name:'Approve',exact:true}).click() + assert.deepEqual([kn(2).status,kn(2).reviewReason],['rejected','Not a ruling; a forum guess.']) + // Approve with a reason (the field is offered, not demanded), then approve another without one. + await notes.getByRole('button',{name:'Approve',exact:true}).first().click() const approve=page.getByRole('dialog',{name:'Approve note'}) + await approve.getByLabel('Reason (optional)').fill('Matches the rulebook.') + await approve.getByRole('button',{name:'Approve note',exact:true}).click() + await approve.waitFor({state:'detached'}) + assert.deepEqual([kn(4).status,kn(4).reviewReason],['approved','Matches the rulebook.']) + await notes.getByRole('button',{name:'Approve',exact:true}).click() await approve.getByRole('button',{name:'Approve note',exact:true}).click() await approve.waitFor({state:'detached'}) - assert.equal(state.knowledge[0].status,'approved') + assert.deepEqual([kn(3).status,kn(3).reviewReason],['approved',null]) await page.getByText('Nothing waiting',{exact:true}).waitFor() await page.getByRole('button',{name:'Rejected',exact:true}).click() await notes.getByText('Review note: Not a ruling; a forum guess.',{exact:true}).waitFor()