diff --git a/HANDOFF.md b/HANDOFF.md index 4517bf5..8a0a9ae 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -178,6 +178,16 @@ git hooks (simple-git-hooks): pre-commit → lint-staged ; pre-push → typeche - ✅ acceptance G1에 devvit.json `$schema` + cron 5-field 체크 추가. `docs/new-mod-checklist.md` 신규. - ✅ **버그 수정**: 미사용 변수 2건(`SAFE_ACTIONS` import, `catch (err)`) — ESLint가 잡음. +**세션 3 (2026-05-12) — 외부 리뷰(gemini-code-assist / coderabbit / codex) 반영 (PR #3)**: + +- ✅ `r_shouting_title` starter rule이 **body** uppercase ratio를 보던 버그 → 새 fact `content.title.upperCaseRatio` 추가 (rule-schema FactPaths + fact-bag 계산 + system-prompt 자동 포함). 이제 ALL-CAPS **제목** 신호를 정확히 본다. (codex P2) +- ✅ `devvit-testkit.ts`: `zAdd` 중복 멤버 허용 → 실제 Redis ZADD처럼 멤버별 score 갱신; `zRange`가 `by:'score'`일 때 `reverse` 무시하던 것 수정 + 구조 정리. (gemini) +- ✅ `devvit-doctor.ts`: node 버전 비교가 major.minor만 보던 것 → major.minor.patch. (gemini) +- ✅ `system-prompt.test.ts`: clarification 예시의 빈 `suggestedAnswers []`가 통과하던 것 → `length > 0` 검증 추가. (coderabbit) +- ✅ `test/setup.ts` `beforeEach`: `vi.clearAllMocks()`만으로는 mock **구현**이 누수 → `getPostById`/`getCommentById` `mockReset()` + 모든 reddit 더블 기본 impl 재설정. (coderabbit) +- ✅ `replay-runner`: `redisHashes`/`redisZsets` fixture 필드 추가 (이전엔 string 키만 seed 가능) + replay용 generic thing stub. `fixtures/undo-action.json` 신규 (rollback 라운드트립 replay). +- 152 tests pass (1 skipped). lint/format/tsc clean. acceptance 4/4. + **아직 남음 (= 사용자 Devvit wizard 단계 + 이후)**: - `npm run dev` 실기 playtest로만 검증되는 gate (Compose 메뉴 렌더, OpenAI compile 라운드트립, undo 라운드트립) — acceptance/doctor 출력의 MANUAL/soft 섹션 참조 diff --git a/fixtures/undo-action.json b/fixtures/undo-action.json new file mode 100644 index 0000000..36ef6c8 --- /dev/null +++ b/fixtures/undo-action.json @@ -0,0 +1,21 @@ +{ + "route": "/internal/menu/undo-action", + "mod": true, + "body": { "location": "post", "targetId": "t3_target" }, + "redisZsets": { + "testsub:audit": [{ "member": "a_undo_demo", "score": 1730000000000 }] + }, + "redisHashes": { + "testsub:audit:a_undo_demo": { + "action": "remove", + "outcome": "applied", + "thingId": "t3_target", + "thingType": "post", + "authorName": "spammer", + "ruleSourceNL": "remove posts from accounts under 50 karma" + } + }, + "redis": { + "testsub:rollback:a_undo_demo": "{\"entry\":{\"actionId\":\"a_undo_demo\",\"ruleId\":\"r_low_karma\",\"ruleSourceNL\":\"remove posts from accounts under 50 karma\",\"thingId\":\"t3_target\",\"thingType\":\"post\",\"action\":\"remove\",\"params\":{},\"authorName\":\"spammer\",\"ts\":1730000000000,\"outcome\":\"applied\"},\"reverseParams\":{\"wasRemoved\":false,\"action\":\"remove\"}}" + } +} diff --git a/scripts/devvit-doctor.ts b/scripts/devvit-doctor.ts index 5527cd5..661f125 100644 --- a/scripts/devvit-doctor.ts +++ b/scripts/devvit-doctor.ts @@ -118,13 +118,18 @@ try { const have = process.versions.node; if (!want) warn('package.json has no engines.node'); else { - const min = want - .replace(/[^\d.]/g, '') - .split('.') - .map(Number); - const cur = have.split('.').map(Number); - const geq = cur[0] > min[0] || (cur[0] === min[0] && (cur[1] ?? 0) >= (min[1] ?? 0)); - if (geq) ok(`node ${have} satisfies engines.node "${want}"`); + // Compare major.minor.patch. Only handles a single ">=x.y.z" floor — fine + // for the engines fields we write; not a full semver-range parser. + const parts = (v: string) => + v + .replace(/[^\d.]/g, '') + .split('.') + .map((n) => Number(n) || 0); + const min = parts(want); + const cur = parts(have); + let cmp = 0; + for (let i = 0; i < 3 && cmp === 0; i++) cmp = (cur[i] ?? 0) - (min[i] ?? 0); + if (cmp >= 0) ok(`node ${have} satisfies engines.node "${want}"`); else fail(`node ${have} does NOT satisfy engines.node "${want}"`); } } catch { diff --git a/scripts/replay.ts b/scripts/replay.ts index bb900d7..9b14939 100644 --- a/scripts/replay.ts +++ b/scripts/replay.ts @@ -17,7 +17,9 @@ // "mod": true, // caller is a moderator of the playtest sub (default: true) // "settings":{ "openaiApiKey": "sk-x" },// values returned by settings.get(...) // "openai": { ...ruleJson... }, // canned OpenAI compile result (sets a fake fetch) -// "redis": { "testsub:rules:active": "{...json...}" } // pre-seed redis string keys +// "redis": { "testsub:rules:active": "{...json...}" }, // pre-seed string keys +// "redisHashes":{ "testsub:audit:": { "action": "remove", ... } }, // pre-seed hash keys +// "redisZsets": { "testsub:audit": [ { "member": "", "score": 1730000000000 } ] } // pre-seed zsets // } // // This shells out to vitest so it reuses test/setup.ts's mocks verbatim; the diff --git a/src/server/evaluator.test.ts b/src/server/evaluator.test.ts index 5018968..bfd803e 100644 --- a/src/server/evaluator.test.ts +++ b/src/server/evaluator.test.ts @@ -21,6 +21,7 @@ function facts(overrides: Partial> = { 'content.containsRegex': 'hello world join my discord.gg/abc', 'content.title.length': 12, 'content.title.contains': 'My Cool Title', + 'content.title.upperCaseRatio': 0.25, 'content.url': 'https://discord.gg/abc', 'content.urlDomain': 'discord.gg', 'sub.weeklyActiveUsers': 1000, diff --git a/src/server/fact-bag.test.ts b/src/server/fact-bag.test.ts index 66fc5df..77b1b66 100644 --- a/src/server/fact-bag.test.ts +++ b/src/server/fact-bag.test.ts @@ -36,6 +36,17 @@ describe('buildPostFactBag', () => { expect(bag['content.containsRegex']).toBe(POST.body); }); + it('computes content.title.upperCaseRatio from the title, independent of the body', async () => { + const shouty = await buildPostFactBag({ ...POST, title: 'BUY MY COURSE NOW', body: 'a perfectly calm body' }); + expect(shouty['content.title.upperCaseRatio']).toBe(1); + expect(shouty['content.upperCaseRatio']).toBeLessThan(0.2); // body ratio unaffected + const calm = await buildPostFactBag({ ...POST, title: 'a perfectly calm title', body: 'WHATEVER' }); + expect(calm['content.title.upperCaseRatio']).toBe(0); + // numeric-only title → 0, not NaN + const numeric = await buildPostFactBag({ ...POST, title: '2024 results' }); + expect(numeric['content.title.upperCaseRatio']).toBe(0); + }); + it('passes through subreddit context', async () => { const bag = await buildPostFactBag(POST); expect(bag['sub.weeklyActiveUsers']).toBe(1234); diff --git a/src/server/fact-bag.ts b/src/server/fact-bag.ts index b2fc08d..05eea48 100644 --- a/src/server/fact-bag.ts +++ b/src/server/fact-bag.ts @@ -29,12 +29,19 @@ interface CommentInput { sub?: { weeklyActiveUsers?: number; over18?: boolean }; } +// Fraction of A–Z letters in `s` that are uppercase. 0 when `s` has no letters +// (so a link post with an empty body / a numeric title scores 0, not NaN). +function upperCaseRatioOf(s: string): number { + const letters = s.replace(/[^A-Za-z]/g, ''); + return letters.length === 0 ? 0 : (letters.match(/[A-Z]/g)?.length ?? 0) / letters.length; +} + export async function buildPostFactBag(p: PostInput, reportsCount = 0): Promise { const a = await getAuthorFacts(p.authorId, p.authorName); const linkRegex = /https?:\/\/[^\s)]+/gi; const links = p.body?.match(linkRegex) ?? []; - const upper = (p.body ?? '').replace(/[^A-Za-z]/g, ''); - const upperCaseRatio = upper.length === 0 ? 0 : (upper.match(/[A-Z]/g)?.length ?? 0) / upper.length; + const upperCaseRatio = upperCaseRatioOf(p.body ?? ''); + const titleUpperCaseRatio = upperCaseRatioOf(p.title ?? ''); let urlDomain = ''; try { if (p.url) urlDomain = new URL(p.url).hostname; @@ -59,6 +66,7 @@ export async function buildPostFactBag(p: PostInput, reportsCount = 0): Promise< 'content.containsRegex': p.body ?? '', 'content.title.length': p.title?.length ?? 0, 'content.title.contains': p.title ?? '', + 'content.title.upperCaseRatio': titleUpperCaseRatio, 'content.url': p.url ?? '', 'content.urlDomain': urlDomain, @@ -74,8 +82,7 @@ export async function buildCommentFactBag(c: CommentInput, reportsCount = 0): Pr const a = await getAuthorFacts(c.authorId, c.authorName); const linkRegex = /https?:\/\/[^\s)]+/gi; const links = c.body.match(linkRegex) ?? []; - const upper = c.body.replace(/[^A-Za-z]/g, ''); - const upperCaseRatio = upper.length === 0 ? 0 : (upper.match(/[A-Z]/g)?.length ?? 0) / upper.length; + const upperCaseRatio = upperCaseRatioOf(c.body); return { 'author.accountAgeHours': a.accountAgeHours, @@ -93,6 +100,7 @@ export async function buildCommentFactBag(c: CommentInput, reportsCount = 0): Pr 'content.containsRegex': c.body, 'content.title.length': 0, 'content.title.contains': '', + 'content.title.upperCaseRatio': 0, 'content.url': '', 'content.urlDomain': '', diff --git a/src/shared/rule-schema.ts b/src/shared/rule-schema.ts index 9902cc7..1bb55cf 100644 --- a/src/shared/rule-schema.ts +++ b/src/shared/rule-schema.ts @@ -39,10 +39,11 @@ export const FactPaths = [ 'content.length', 'content.linkCount', 'content.imageCount', - 'content.upperCaseRatio', + 'content.upperCaseRatio', // body's A–Z uppercase ratio (0 for link posts / empty bodies) 'content.containsRegex', // requires .params.regex 'content.title.length', 'content.title.contains', // requires .params.needle + 'content.title.upperCaseRatio', // title's A–Z uppercase ratio — the "ALL CAPS TITLE" signal 'content.url', // full URL (post link) 'content.urlDomain', // hostname only diff --git a/src/shared/starter-rules.test.ts b/src/shared/starter-rules.test.ts index f40044b..f84b364 100644 --- a/src/shared/starter-rules.test.ts +++ b/src/shared/starter-rules.test.ts @@ -71,6 +71,7 @@ describe('starter rules behave as intended against representative fact bags', () 'content.containsRegex': 'a normal post', 'content.title.length': 20, 'content.title.contains': 'A Normal Title', + 'content.title.upperCaseRatio': 0.05, 'content.url': '', 'content.urlDomain': '', 'sub.weeklyActiveUsers': 500, @@ -98,15 +99,25 @@ describe('starter rules behave as intended against representative fact bags', () expect(out.map((r) => r.id)).toContain('r_low_karma_link_drop'); }); - it('an ALL-CAPS title matches the shouting-title rule', () => { + it('an ALL-CAPS title matches the shouting-title rule (title ratio, not body ratio)', () => { const out = selectMatchingRules(rules, 'onPostSubmit', { ...base, - 'content.upperCaseRatio': 0.95, + 'content.title.upperCaseRatio': 0.95, 'content.title.length': 30, }); expect(out.map((r) => r.id)).toContain('r_shouting_title'); }); + it('a normal-cased title with an ALL-CAPS body does NOT trigger the shouting-title rule', () => { + const out = selectMatchingRules(rules, 'onPostSubmit', { + ...base, + 'content.upperCaseRatio': 0.95, // body is shouty, title is not + 'content.title.upperCaseRatio': 0.05, + 'content.title.length': 30, + }); + expect(out.map((r) => r.id)).not.toContain('r_shouting_title'); + }); + it('a long all-caps comment matches the wall-of-caps rule', () => { const out = selectMatchingRules(rules, 'onCommentSubmit', { ...base, diff --git a/src/shared/starter-rules.ts b/src/shared/starter-rules.ts index 7fc2133..43da87d 100644 --- a/src/shared/starter-rules.ts +++ b/src/shared/starter-rules.ts @@ -48,7 +48,7 @@ const STARTER_RULE_SEEDS: readonly SeedRule[] = [ on: ['onPostSubmit'], when: { all: [ - { fact: 'content.upperCaseRatio', op: 'gt', value: 0.7 }, + { fact: 'content.title.upperCaseRatio', op: 'gt', value: 0.7 }, { fact: 'content.title.length', op: 'gte', value: 12 }, ], }, diff --git a/src/shared/system-prompt.test.ts b/src/shared/system-prompt.test.ts index bd2c2d0..ba46ac9 100644 --- a/src/shared/system-prompt.test.ts +++ b/src/shared/system-prompt.test.ts @@ -66,6 +66,7 @@ describe('FEW_SHOT_EXAMPLES', () => { if (!('needsClarification' in ex.assistant)) continue; expect((ex.assistant.question ?? '').length).toBeGreaterThan(0); expect(Array.isArray(ex.assistant.suggestedAnswers)).toBe(true); + expect((ex.assistant.suggestedAnswers ?? []).length).toBeGreaterThan(0); } }); }); diff --git a/test/devvit-testkit.ts b/test/devvit-testkit.ts index aa32f44..bc3e4bc 100644 --- a/test/devvit-testkit.ts +++ b/test/devvit-testkit.ts @@ -70,21 +70,22 @@ export function makeFakeRedis(): FakeRedis { }, hGetAll: async (k: string) => ({ ...(hashes.get(k) ?? {}) }), zAdd: async (k: string, entry: { member: string; score: number }) => { - const arr = zsets.get(k) ?? []; + // Real Redis ZADD updates the score of an existing member, not a duplicate. + const arr = (zsets.get(k) ?? []).filter((e) => e.member !== entry.member); arr.push(entry); zsets.set(k, arr); }, zRange: async (k: string, start: number, stop: number, opts?: { by?: 'rank' | 'score'; reverse?: boolean }) => { - let arr = [...(zsets.get(k) ?? [])].sort((a, b) => a.score - b.score); + const arr = [...(zsets.get(k) ?? [])].sort((a, b) => a.score - b.score); + if (opts?.reverse) arr.reverse(); if (opts?.by === 'score') { - arr = arr.filter((e) => e.score >= start && e.score <= stop); - } else { - if (opts?.reverse) arr.reverse(); - const end = stop < 0 ? arr.length : stop + 1; - arr = arr.slice(start, end); - return arr; + // start/stop are score bounds (low..high); honour them regardless of order. + const [lo, hi] = start <= stop ? [start, stop] : [stop, start]; + return arr.filter((e) => e.score >= lo && e.score <= hi); } - return arr; + // by rank (default): start/stop are indices into the (possibly reversed) array. + const end = stop < 0 ? arr.length : stop + 1; + return arr.slice(start, end); }, zCount: async (k: string, min: number, max: number | '+inf') => { const hi = max === '+inf' ? Infinity : max; diff --git a/test/replay-runner.test.ts b/test/replay-runner.test.ts index d107fce..6825256 100644 --- a/test/replay-runner.test.ts +++ b/test/replay-runner.test.ts @@ -27,7 +27,9 @@ type Fixture = { mod?: boolean; settings?: Record; openai?: unknown; - redis?: Record; + redis?: Record; // string keys, e.g. `${sub}:rules:active` + redisHashes?: Record>; // hash keys, e.g. `${sub}:audit:` + redisZsets?: Record>; // zset keys, e.g. `${sub}:audit` }; function snapshotRedisKeys() { @@ -48,11 +50,27 @@ describe.skipIf(!FIXTURE)('replay', () => { ); const body = 'body' in fx && fx.body !== undefined ? fx.body : fx; + // Generic Reddit "thing" stub so action/rollback flows complete in a replay + // (tests use precise mocks; here we just want to observe the control flow). + const stubThing = { + approve: async () => {}, + unlock: async () => {}, + lock: async () => {}, + remove: async () => {}, + removed: false, + flair: null as { text: string } | null, + }; + fakeReddit.getPostById.mockResolvedValue(stubThing); + fakeReddit.getCommentById.mockResolvedValue(stubThing); + // Apply fixture-driven mock state. if (fx.mod ?? true) fakeReddit.getModerators.mockResolvedValue(fakeListing([{ username: 'caller' }])); if (fx.settings) fakeSettings.get.mockImplementation(async (k: string) => fx.settings![k]); if (fx.openai !== undefined) fakeFetch.mockResolvedValue(openaiResponse(fx.openai)); if (fx.redis) for (const [k, v] of Object.entries(fx.redis)) await fakeRedis.set(k, v); + if (fx.redisHashes) for (const [k, h] of Object.entries(fx.redisHashes)) await fakeRedis.hSet(k, h); + if (fx.redisZsets) + for (const [k, members] of Object.entries(fx.redisZsets)) for (const m of members) await fakeRedis.zAdd(k, m); const before = snapshotRedisKeys(); const res = await app.fetch( diff --git a/test/setup.ts b/test/setup.ts index 05b2fcb..fb5aa7b 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -41,11 +41,21 @@ beforeEach(() => { fakeRedis.hashes.clear(); fakeRedis.zsets.clear(); vi.clearAllMocks(); + // clearAllMocks only wipes call history — re-establish every default *implementation* + // here so a `.mockResolvedValue(...)` / `.mockImplementation(...)` in one test can't + // leak into the next. (getPostById/getCommentById have no factory default → mockReset.) + fakeReddit.getPostById.mockReset(); + fakeReddit.getCommentById.mockReset(); fakeReddit.getCurrentSubreddit.mockResolvedValue({ id: 't5_testsub', name: 'testsub' }); fakeReddit.getCurrentUser.mockResolvedValue({ id: 't2_caller', username: 'caller' }); fakeReddit.getUserByUsername.mockResolvedValue(null); fakeReddit.getUserKarmaFromCurrentSubreddit.mockResolvedValue({ fromComments: 0, fromPosts: 0 }); fakeReddit.getModerators.mockResolvedValue({ all: async () => [] }); + fakeReddit.report.mockResolvedValue({}); + fakeReddit.setPostFlair.mockResolvedValue(undefined); + fakeReddit.banUser.mockResolvedValue(undefined); + fakeReddit.muteUser.mockResolvedValue(undefined); + fakeReddit.unbanUser.mockResolvedValue(undefined); fakeReddit.modMail.createModNotification.mockResolvedValue('modmail_conv_1'); fakeScheduler.runJob.mockResolvedValue('job_1'); fakeSettings.get.mockResolvedValue(undefined);