From 9a1c254111b0387e3e431ee8e9750496eff3b91e Mon Sep 17 00:00:00 2001 From: Nils Schiller Date: Fri, 16 Oct 2026 18:24:02 +0200 Subject: [PATCH 1/2] feat(import): read the MR's review threads into the --from-mr extraction (#1187) `teamai import --from-mr` and `teamai ci extract-mr` now fetch the MR's review threads alongside the description, commits and diff: GitLab `/discussions` (paginated), GitHub review comments, reviews and conversation comments (through `gh api` or the REST fallback), and TGit notes. System notes and teamai's own `\n## TeamAI 知识提炼', user: { login: 'ci-bot' } }, + { body: 'Merging after the split.', user: { login: 'dev' } }, +]; + function mockResponses(failedPath?: string, status = 200, body = ''): string[] { const seen: string[] = []; const request = (options: https.RequestOptions, callback: (res: IncomingMessage) => void) => { @@ -22,6 +42,9 @@ function mockResponses(failedPath?: string, status = 200, body = ''): string[] { const failed = key === failedPath; const response = failed ? body : diff ? 'diff --git a/a b/a\n+fixed\n' : url === PR ? JSON.stringify(METADATA) + : url.startsWith(`${PR}/comments`) ? JSON.stringify(REVIEW_COMMENTS) + : url.startsWith(`${PR}/reviews`) ? JSON.stringify(REVIEWS) + : url.startsWith('/repos/team/repo/issues/42/comments') ? JSON.stringify(CONVERSATION) : JSON.stringify([{ sha: 'abc123', commit: { message: 'Fix retries\n\nDetails' } }]); const req = Object.assign(new EventEmitter(), { setTimeout: vi.fn(), @@ -75,3 +98,34 @@ describe('GitHub PR REST fallback', () => { }); }); }); + +describe('GitHub PR review threads (#1187)', () => { + it('groups diff comments into threads and adds review verdicts and conversation, without teamai comments', async () => { + mockResponses(); + const mr = await fetchGitHubPR('https://github.com/team/repo/pull/42'); + expect(mr.discussions).toEqual([ + { file: 'src/up.ts', line: 42, notes: [{ author: 'bob', body: 'Three retries hide a dead bucket.' }, { author: 'dev', body: 'Agreed, fail fast on 403.' }] }, + { file: 'src/x.ts', line: 7, notes: [{ author: 'carol', body: 'Unrelated nit' }] }, + { notes: [{ author: 'carol', body: 'changes requested: Please split the retry policy out.' }] }, + { notes: [{ author: 'dev', body: 'Merging after the split.' }] }, + ]); + }); + + it('requests none of the list endpoints with discussions: false', async () => { + const seen = mockResponses(); + const mr = await fetchGitHubPR('https://github.com/team/repo/pull/42', { discussions: false }); + expect(mr).not.toHaveProperty('discussions'); + expect(seen.filter((u) => /comments|reviews/.test(u))).toEqual([]); + }); + + it('keeps the extraction going, with a warning, when a list endpoint fails', async () => { + vi.mocked(log.warn).mockClear(); + mockResponses(`${PR}/reviews?per_page=100&page=1`, 403, '{"message":"Forbidden"}'); + const mr = await fetchGitHubPR('https://github.com/team/repo/pull/42'); + expect(mr.title).toBe('Fix retries'); + expect(mr).not.toHaveProperty('discussions'); + expect(vi.mocked(log.warn).mock.calls.map(([m]) => String(m))).toEqual([ + expect.stringMatching(/Review threads not read \(GitHub API error 403/), + ]); + }); +}); diff --git a/src/__tests__/gitlab-mr-discussions.test.ts b/src/__tests__/gitlab-mr-discussions.test.ts new file mode 100644 index 000000000..3b76bc877 --- /dev/null +++ b/src/__tests__/gitlab-mr-discussions.test.ts @@ -0,0 +1,111 @@ +/** + * `fetchGitLabMR` reads the MR's review threads from `/discussions` (#1187): + * every page, without system notes or teamai's own CI comments, with the file + * and line a diff thread is anchored to and whether it is resolved. An API + * error there keeps the extraction going without threads, and + * `discussions: false` does not call the endpoint at all. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../utils/logger.js', () => ({ + log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), dim: vi.fn() }, +})); + +import { fetchGitLabMR } from '../providers/gitlab/mr-fetch.js'; +import { log } from '../utils/logger.js'; + +const URL = 'https://gitlab.com/group/repo/-/merge_requests/7'; +const API = 'https://gitlab.com/api/v4/projects/group%2Frepo/merge_requests/7'; + +const MR = { title: 'Retry uploads', description: 'd', author: { username: 'alice' }, merged_at: '2026-09-20T10:00:00Z' }; + +const note = (author: string, body: string, extra: Record = {}) => ({ body, author: { username: author }, ...extra }); + +const PAGE_1 = [ + { notes: [note('gitlab', 'changed the description', { system: true })] }, + { + notes: [ + note('bob', 'Fail fast on 403.', { resolvable: true, resolved: true, position: { new_path: 'src/up.ts', old_path: 'src/up.ts', new_line: 42, old_line: null } }), + note('alice', 'Done.', { resolvable: true, resolved: true }), + ], + }, + { notes: [note('ci', '\n## TeamAI 知识提炼')] }, +]; +const PAGE_2 = [ + { notes: [note('carol', 'Deleted line was load-bearing.', { resolvable: true, resolved: false, position: { new_path: null, old_path: 'src/old.ts', new_line: null, old_line: 3 } })] }, + { notes: [note('dave', 'LGTM')] }, +]; + +type Handler = (url: string) => { status?: number; body: unknown; nextPage?: string }; + +function stubFetch(handler: Handler): string[] { + const seen: string[] = []; + vi.stubGlobal('fetch', vi.fn(async (input: string) => { + seen.push(input); + const { status = 200, body, nextPage } = handler(input); + return { + ok: status >= 200 && status < 300, + status, + headers: new Headers(nextPage ? { 'x-next-page': nextPage } : {}), + json: async () => body, + text: async () => (typeof body === 'string' ? body : JSON.stringify(body)), + } as unknown as Response; + })); + return seen; +} + +const base: Handler = (url) => { + if (url === `${API}`) return { body: MR }; + if (url.startsWith(`${API}/commits`)) return { body: [{ id: 'abc', title: 'retry' }] }; + if (url.startsWith(`${API}/changes`)) return { body: { changes: [{ diff: '+retry' }] } }; + if (url.endsWith('/discussions?per_page=100&page=1')) return { body: PAGE_1, nextPage: '2' }; + if (url.endsWith('/discussions?per_page=100&page=2')) return { body: PAGE_2 }; + throw new Error(`unexpected ${url}`); +}; + +beforeEach(() => { + vi.stubEnv('GITLAB_TOKEN', 'secret'); + vi.mocked(log.warn).mockClear(); +}); +afterEach(() => { + vi.unstubAllGlobals(); + vi.unstubAllEnvs(); +}); + +describe('fetchGitLabMR review threads (#1187)', () => { + it('reads every page of discussions, mapping anchors and resolution, without system or teamai notes', async () => { + const seen = stubFetch(base); + const mr = await fetchGitLabMR(URL); + expect(seen.filter((u) => u.includes('/discussions'))).toHaveLength(2); + expect(mr.discussions).toEqual([ + { file: 'src/up.ts', line: 42, resolved: true, notes: [{ author: 'bob', body: 'Fail fast on 403.' }, { author: 'alice', body: 'Done.' }] }, + { file: 'src/old.ts', line: 3, resolved: false, notes: [{ author: 'carol', body: 'Deleted line was load-bearing.' }] }, + { file: undefined, line: undefined, resolved: undefined, notes: [{ author: 'dave', body: 'LGTM' }] }, + ]); + expect(mr.title).toBe('Retry uploads'); + expect(mr.diff).toBe('+retry'); + }); + + it('does not call the endpoint with discussions: false', async () => { + const seen = stubFetch(base); + const mr = await fetchGitLabMR(URL, { discussions: false }); + expect(seen.some((u) => u.includes('/discussions'))).toBe(false); + expect(mr).not.toHaveProperty('discussions'); + }); + + it('keeps the extraction going, with a warning, when the discussions call fails', async () => { + stubFetch((url) => (url.includes('/discussions') ? { status: 403, body: { message: 'Forbidden' } } : base(url))); + const mr = await fetchGitLabMR(URL); + expect(mr).not.toHaveProperty('discussions'); + expect(mr.title).toBe('Retry uploads'); + expect(vi.mocked(log.warn).mock.calls.map(([m]) => String(m))).toEqual([ + expect.stringMatching(/Review threads not read \(GitLab API error 403\)/), + ]); + }); + + it('returns an empty list for an MR without review', async () => { + stubFetch((url) => (url.includes('/discussions') ? { body: [] } : base(url))); + const mr = await fetchGitLabMR(URL); + expect(mr.discussions).toEqual([]); + }); +}); diff --git a/src/__tests__/gitlab-provider.test.ts b/src/__tests__/gitlab-provider.test.ts index 6a4d06b6c..588357eb6 100644 --- a/src/__tests__/gitlab-provider.test.ts +++ b/src/__tests__/gitlab-provider.test.ts @@ -652,12 +652,16 @@ describe('fetchGitLabMR', () => { if (String(url) === `${base}/changes`) { return new Response(JSON.stringify({ changes: [{ diff: '+fixed' }] }), { status: 200 }); } + if (String(url).startsWith(`${base}/discussions`)) { + return new Response(JSON.stringify([]), { status: 200 }); + } return new Response(JSON.stringify({ title: 'T', description: null, author: { username: 'a' } }), { status: 200 }); }) as never; const mr = await fetchGitLabMR('https://gitlab.com/gitlab/g/sub/repo/-/merge_requests/7'); - expect(seen).toEqual([base, `${base}/commits?per_page=50`, `${base}/changes`]); + // The review threads (#1187) come from the same instance root. + expect(seen).toEqual([base, `${base}/commits?per_page=50`, `${base}/changes`, `${base}/discussions?per_page=100&page=1`]); expect(mr.commits).toEqual([{ hash: 'abc123', message: 'Fix' }]); expect(mr.diff).toBe('+fixed'); }); diff --git a/src/__tests__/import-mr-threads.test.ts b/src/__tests__/import-mr-threads.test.ts new file mode 100644 index 000000000..8ad7a6b17 --- /dev/null +++ b/src/__tests__/import-mr-threads.test.ts @@ -0,0 +1,116 @@ +/** + * `teamai import --from-mr` reads the MR's review threads (#1187): the prompt + * gets them in a section of their own, with the MR author and reviewers + * marked, replied threads first, a long thread shortened rather than the + * section cut off, and the model told that a resolved thread's decision + * outweighs the diff. With `language: en` the learning is written in English. + */ +import { describe, expect, it } from 'vitest'; + +import { extractMRLearningPrompt, formatMrThreads } from '../import-mr.js'; +import { pruneThreads } from '../providers/mr-threads.js'; +import type { MRData, MRThread } from '../types.js'; + +const mr: MRData = { + title: 'Retry uploads', + description: 'Retries S3 uploads.', + author: 'alice', + commits: [{ hash: 'abc12345', message: 'retry upload' }], + diff: 'diff --git a/up.ts b/up.ts\n+retry(3)\n', + url: 'https://gitlab.example.com/g/r/-/merge_requests/7', +}; + +const replied: MRThread = { + file: 'src/up.ts', + line: 42, + resolved: true, + notes: [ + { author: 'bob', body: 'Three retries hide a dead bucket. Fail fast on 403.' }, + { author: 'alice', body: 'Agreed, 403 now raises at once; retries only on 5xx.' }, + ], +}; +const remark: MRThread = { notes: [{ author: 'carol', body: 'LGTM' }] }; + +describe('pruneThreads', () => { + it('drops teamai CI comments and blank notes, and a thread left empty', () => { + const threads = pruneThreads([ + { notes: [{ author: 'ci', body: '\n## TeamAI 知识提炼' }] }, + { notes: [{ author: 'ci', body: '' }, { author: 'bob', body: ' ' }] }, + remark, + ]); + expect(threads).toEqual([remark]); + }); +}); + +describe('formatMrThreads', () => { + it('puts replied threads before single remarks and marks the MR author', () => { + const text = formatMrThreads([remark, replied], 'alice'); + expect(text.indexOf('src/up.ts:42')).toBeLessThan(text.indexOf('LGTM')); + expect(text).toContain('src/up.ts:42 (resolved)'); + expect(text).toContain('[reviewer bob] Three retries hide a dead bucket.'); + expect(text).toContain('[author alice] Agreed, 403 now raises at once;'); + expect(text).toContain('[reviewer carol] LGTM'); + }); + + it('labels an unresolved thread and one without a resolution state', () => { + const text = formatMrThreads([{ ...replied, resolved: false }, { ...replied, resolved: undefined, file: undefined }], 'alice'); + expect(text).toContain('src/up.ts:42 (unresolved)'); + expect(text).toMatch(/^### Thread 2\n/m); + }); + + it('shortens a long thread to its first and last notes instead of cutting the section', () => { + const long: MRThread = { + notes: Array.from({ length: 12 }, (_, i) => ({ author: i % 2 ? 'alice' : 'bob', body: `note ${i} ${'x'.repeat(300)}` })), + }; + const text = formatMrThreads([long], 'alice', 6000); + expect(text).toContain('note 0'); + expect(text).toContain('note 11'); + expect(text).not.toContain('note 5'); + expect(text).toContain('(10 notes omitted)'); + expect(text.length).toBeLessThan(2000); + }); + + it('stops at the budget and says how many threads it left out', () => { + const many = Array.from({ length: 8 }, (_, i) => ({ ...replied, line: i })); + const text = formatMrThreads(many, 'alice', 500); + expect(text).toContain('### Thread 1'); + expect(text).not.toContain('### Thread 8'); + expect(text).toMatch(/\d+ more threads? omitted/); + expect(text.length).toBeLessThanOrEqual(600); + }); +}); + +describe('extractMRLearningPrompt', () => { + it('has no thread section when the MR has no threads', () => { + const prompt = extractMRLearningPrompt({ ...mr, discussions: [] }); + expect(prompt).not.toContain('Review threads'); + expect(prompt).toContain('用中文撰写'); + }); + + it('adds the threads with the instruction that a resolved decision outweighs the diff', () => { + const prompt = extractMRLearningPrompt({ ...mr, discussions: [remark, replied] }); + expect(prompt).toContain('Review threads'); + expect(prompt).toContain('[author alice]'); + expect(prompt).toMatch(/resolved 的 thread 里的决定比复述 diff 更重要/); + // The threads do not push the diff out. + expect(prompt).toContain('+retry(3)'); + expect(extractMRLearningPrompt({ ...mr, discussions: [replied] }, 'en')) + .toMatch(/decision reached in a resolved thread outweighs a restatement of the diff/); + }); + + it('writes the learning in English when the team language is en, with English headings', () => { + const prompt = extractMRLearningPrompt({ ...mr, discussions: [replied] }, 'en'); + expect(prompt).toContain('Write in English'); + expect(prompt).toContain('## Background'); + expect(prompt).not.toContain('用中文撰写'); + expect(prompt).toContain(`source_mr: "${mr.url}"`); + }); + + it('keeps the Chinese prompt for zh and when the language is not set', () => { + for (const language of ['zh', undefined] as const) { + const prompt = extractMRLearningPrompt(mr, language); + expect(prompt).toContain('用中文撰写'); + expect(prompt).toContain('## 背景'); + } + }); +}); diff --git a/src/__tests__/import-mr.test.ts b/src/__tests__/import-mr.test.ts index ee32da35a..ffed3cae6 100644 --- a/src/__tests__/import-mr.test.ts +++ b/src/__tests__/import-mr.test.ts @@ -18,6 +18,8 @@ const DRAFT_BODY = 'gateway retry budget exhausted under burst traffic backoff j /** The team repo `teamai import` runs against, set per test. */ let teamRepo = ''; +/** The team's teamai.yaml `language`, set per test (#1187). */ +let teamLanguage: 'en' | 'zh' | undefined; vi.mock('../config.js', async (importOriginal) => ({ ...(await importOriginal()), autoDetectInit: vi.fn(async () => ({ @@ -28,7 +30,7 @@ vi.mock('../config.js', async (importOriginal) => ({ additionalRoles: [], projects: ['alpha'], }, - teamConfig: { team: 't', repo: 'r' }, + teamConfig: { team: 't', repo: 'r', ...(teamLanguage ? { language: teamLanguage } : {}) }, })), })); @@ -249,6 +251,32 @@ describe('teamai import --from-mr (#1104)', () => { expect(infos()).toContainEqual(`[2/2] ${urls[1]}`); }); + it('reads the review threads by default and leaves them out with --no-discussions (#1187)', async () => { + const write = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + try { + await importCmd({ fromMr: [urls[0]], dryRun: true, all: true }); + await importCmd({ fromMr: [urls[0]], dryRun: true, all: true, discussions: false }); + } finally { + write.mockRestore(); + } + + expect(vi.mocked(fetchGitHubPR).mock.calls.map(([, opts]) => opts)) + .toEqual([{ discussions: true }, { discussions: false }]); + }); + + it('writes the learning in English when teamai.yaml sets language: en (#1187)', async () => { + const write = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + teamLanguage = 'en'; + try { + await importCmd({ fromMr: [urls[0]], dryRun: true, all: true }); + } finally { + teamLanguage = undefined; + write.mockRestore(); + } + + expect(vi.mocked(callClaude).mock.calls[0][0]).toContain('Write in English'); + }); + it('keeps going when one MR fails, reports it and exits non-zero', async () => { vi.mocked(fetchGitHubPR).mockRejectedValueOnce(new Error('MR not found')); const write = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); diff --git a/src/__tests__/tgit-mr-discussions.test.ts b/src/__tests__/tgit-mr-discussions.test.ts new file mode 100644 index 000000000..a3f18604c --- /dev/null +++ b/src/__tests__/tgit-mr-discussions.test.ts @@ -0,0 +1,86 @@ +/** + * `fetchTGitMR` reads the MR's notes through the endpoint `teamai ci` uses and + * groups them into threads by `discussion_id` (#1187), without system notes + * or teamai's own CI comments. A failure there keeps the extraction going. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../providers/tgit/gf-cli.js', () => ({ gfGetOAuthToken: vi.fn(() => null) })); +vi.mock('../utils/logger.js', () => ({ + log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), dim: vi.fn() }, +})); + +import { fetchTGitMR } from '../providers/tgit/mr-fetch.js'; +import { log } from '../utils/logger.js'; + +const URL = 'https://git.woa.com/group/repo/merge_requests/5'; +const API = 'https://git.woa.com/api/v3/projects/group%2Frepo'; + +const NOTES = [ + { body: 'approved this merge request', author: { username: 'bot' }, system: true }, + { body: 'Fail fast on 403.', author: { username: 'bob' }, discussion_id: 'd1' }, + { body: 'LGTM', author: { username: 'dave' } }, + { body: 'Done.', author: { username: 'alice' }, discussion_id: 'd1' }, + { body: '\n## TeamAI 知识提炼', author: { username: 'ci' } }, +]; + +type Handler = (url: string) => { status?: number; body: unknown }; + +function stubFetch(handler: Handler): string[] { + const seen: string[] = []; + vi.stubGlobal('fetch', vi.fn(async (input: string) => { + seen.push(input); + const { status = 200, body } = handler(input); + return { + ok: status >= 200 && status < 300, + status, + json: async () => body, + text: async () => JSON.stringify(body), + } as unknown as Response; + })); + return seen; +} + +const base: Handler = (url) => { + if (url === `${API}/merge_requests?iid=5`) return { body: [{ id: 99, iid: 5, title: 'Retry uploads', description: 'd', author: { username: 'alice' } }] }; + if (url === `${API}/merge_request/99/changes`) return { body: { files: [{ diff: '+retry' }] } }; + if (url === `${API}/merge_requests/99/notes?per_page=100&page=1`) return { body: NOTES }; + throw new Error(`unexpected ${url}`); +}; + +beforeEach(() => { + vi.stubEnv('TGIT_TOKEN', 'secret'); + vi.mocked(log.warn).mockClear(); +}); +afterEach(() => { + vi.unstubAllGlobals(); + vi.unstubAllEnvs(); +}); + +describe('fetchTGitMR review threads (#1187)', () => { + it('groups the notes by discussion, without system or teamai notes', async () => { + stubFetch(base); + const mr = await fetchTGitMR(URL); + expect(mr.discussions).toEqual([ + { notes: [{ author: 'bob', body: 'Fail fast on 403.' }, { author: 'alice', body: 'Done.' }] }, + { notes: [{ author: 'dave', body: 'LGTM' }] }, + ]); + expect(mr.diff).toBe('+retry'); + }); + + it('does not read the notes with discussions: false', async () => { + const seen = stubFetch(base); + const mr = await fetchTGitMR(URL, { discussions: false }); + expect(seen.some((u) => u.includes('/notes'))).toBe(false); + expect(mr).not.toHaveProperty('discussions'); + }); + + it('keeps the extraction going, with a warning, when the notes call fails', async () => { + stubFetch((url) => (url.includes('/notes') ? { status: 500, body: {} } : base(url))); + const mr = await fetchTGitMR(URL); + expect(mr).not.toHaveProperty('discussions'); + expect(vi.mocked(log.warn).mock.calls.map(([m]) => String(m))).toEqual([ + expect.stringMatching(/Review threads not read \(TGit API error 500\)/), + ]); + }); +}); diff --git a/src/ci/extract-mr.ts b/src/ci/extract-mr.ts index 260690d4d..f0d6c4340 100644 --- a/src/ci/extract-mr.ts +++ b/src/ci/extract-mr.ts @@ -32,6 +32,8 @@ export interface CiExtractMrOptions { output?: string; dryRun?: boolean; individualComments?: boolean; + /** Read the MR's review threads into the extraction; `--no-discussions` sets false (#1187) */ + discussions?: boolean; } // ─── Git User 配置 ─────────────────────────────────────── @@ -239,12 +241,18 @@ export async function ciExtractMr(opts: CiExtractMrOptions): Promise { // AI 提炼（复用 importFromMR） let learning: LearningDraft | undefined; + // The team repo's `language: en` makes the learning English (#1187). + const language = opts.teamRepo + ? (await import('../config.js').then(({ loadTeamConfig }) => loadTeamConfig(opts.teamRepo!)))?.language + : undefined; const result = await importFromMR({ url: opts.url, all: true, // learnings-root ok: CI runs against a bare checkout, with no LocalConfig learningsDirs: opts.teamRepo ? [path.join(opts.teamRepo, 'learnings')] : undefined, dryRun: true, // 不让 importFromMR 自己写文件，我们自己控制写入 + discussions: opts.discussions, + language, }); learning = result.learning; diff --git a/src/import-mr.ts b/src/import-mr.ts index 3d726df5b..dfaf8fbf6 100644 --- a/src/import-mr.ts +++ b/src/import-mr.ts @@ -7,7 +7,7 @@ import matter from 'gray-matter'; import { fetchGitHubPR } from './providers/github/mr-fetch.js'; import { fetchGitLabMR } from './providers/gitlab/mr-fetch.js'; import { fetchTGitMR } from './providers/tgit/mr-fetch.js'; -import type { MRData, LearningDraft } from './types.js'; +import type { MRData, MRFetchOptions, MRThread, LearningDraft } from './types.js'; import { callClaude } from './utils/ai-client.js'; import { extractKeywords, findOverlappingLearnings } from './utils/dedup.js'; import { log, spinner } from './utils/logger.js'; @@ -19,51 +19,155 @@ const DEFAULT_LEARNINGS_DIR = path.join(getUserHome(), '.teamai', 'learnings'); /** * Auto-detects the provider from the URL and fetches MR data. * - * @param url Full URL of the MR / PR - * @returns Normalized MRData object - * @throws Error when the URL does not belong to a known provider + * @param url Full URL of the MR / PR + * @param opts What to read besides the MR, its commits and its diff + * @returns Normalized MRData object + * @throws Error when the URL does not belong to a known provider */ -async function fetchMR(url: string): Promise { +async function fetchMR(url: string, opts: MRFetchOptions): Promise { if (url.includes('github.com')) { - return fetchGitHubPR(url); + return fetchGitHubPR(url, opts); } if (url.includes('git.woa.com')) { - return fetchTGitMR(url); + return fetchTGitMR(url, opts); } // GitLab (incl. self-hosted): the `/-/merge_requests/` route is unique to // GitLab, so it identifies the platform on any host. if (/\/-\/merge_requests\/\d+/.test(url)) { - return fetchGitLabMR(url); + return fetchGitLabMR(url, opts); } throw new Error(`Unsupported MR URL: ${url}. Only GitHub, TGit and GitLab are supported`); } +/** Characters of the prompt the review threads may take (#1187). */ +const THREADS_BUDGET = 6000; +/** Characters one thread may take before it is shortened to its first and last notes. */ +const THREAD_MAX = 1500; +/** Characters one note may take. */ +const NOTE_MAX = 500; + +function clip(text: string, max: number): string { + const trimmed = text.trim(); + return trimmed.length <= max ? trimmed : `${trimmed.slice(0, max - 1)}…`; +} + +/** + * Renders the review threads for the extraction prompt (#1187). + * + * Threads with a reply come first and single unanswered remarks last, so a + * budget cut drops the remarks. Each thread names the file and line it is + * anchored to and whether it is resolved, then its notes in order, each marked + * as the MR author's or a reviewer's. A thread over {@link THREAD_MAX} keeps + * its first and last notes; once the budget is reached the rest of the + * threads are counted, not cut off mid-thread. + * + * @param threads The MR's review threads + * @param mrAuthor The MR author's username, to mark their notes + * @param budget Characters the section may take + */ +export function formatMrThreads(threads: MRThread[], mrAuthor: string | undefined, budget = THREADS_BUDGET): string { + const ordered = [...threads.filter((t) => t.notes.length > 1), ...threads.filter((t) => t.notes.length <= 1)]; + const sections: string[] = []; + let used = 0; + for (const [index, thread] of ordered.entries()) { + const anchor = thread.file ? ` — ${thread.file}${thread.line !== undefined ? `:${thread.line}` : ''}` : ''; + const state = thread.resolved === undefined ? '' : thread.resolved ? ' (resolved)' : ' (unresolved)'; + const header = `### Thread ${index + 1}${anchor}${state}`; + const line = (note: MRThread['notes'][number]): string => { + const role = note.author !== undefined && note.author === mrAuthor ? 'author' : 'reviewer'; + const who = note.author ? `${role} ${note.author}` : role; + return `- [${who}] ${clip(note.body, NOTE_MAX).replace(/\n+/g, '\n ')}`; + }; + let lines = thread.notes.map(line); + if (lines.join('\n').length > THREAD_MAX && thread.notes.length > 2) { + lines = [lines[0], `- (${thread.notes.length - 2} notes omitted)`, lines[lines.length - 1]]; + } + const section = `${header}\n${lines.join('\n')}`; + if (used + section.length + 2 > budget) { + const left = ordered.length - index; + sections.push(`(${left} more thread${left === 1 ? '' : 's'} omitted)`); + break; + } + sections.push(section); + used += section.length + 2; + } + return sections.join('\n\n'); +} + /** * Builds the learning extraction prompt. * - * @param mr MR data object - * @returns Full prompt string for callClaude + * The MR's review threads, when the fetch read any, get a section of their + * own after the diff, and the model is told that a decision reached in a + * resolved thread outweighs a restatement of the diff (#1187). `language` + * `en` asks for an English learning with English headings; `zh` or unset + * keeps the Chinese one. + * + * @param mr MR data object + * @param language The team's `language` from teamai.yaml + * @returns Full prompt string for callClaude */ -function extractMRLearningPrompt(mr: MRData): string { +export function extractMRLearningPrompt(mr: MRData, language?: 'en' | 'zh'): string { const commitsFormatted = mr.commits .map((c) => `- ${c.hash.slice(0, 8)}: ${c.message}`) .join('\n'); const diff3000 = mr.diff.slice(0, 3000); const author = mr.author ?? 'unknown'; const date = mr.mergedAt ? mr.mergedAt.slice(0, 10) : new Date().toISOString().slice(0, 10); - - return `你是团队知识库管理员。从以下 MR 信息提炼一条有价值的团队 learning。 -【必须】用中文撰写，输出完整 Markdown 文档（含 YAML frontmatter）。 - -frontmatter 字段（严格按此格式，不要加其他字段）： ---- -title: "<简短标题，描述核心问题或发现，<60字符>" + const threads = mr.discussions && mr.discussions.length > 0 ? formatMrThreads(mr.discussions, mr.author) : ''; + const frontmatter = (titleHint: string): string => `--- +title: "${titleHint}" author: ${author} date: ${date} tags: [tag1, tag2, tag3] confidence: 0.85 source_mr: "${mr.url}" +---`; + + if (language === 'en') { + return `You are the team's knowledge base curator. Distill one valuable team learning from the MR below. +Write in English. Output a complete Markdown document with YAML frontmatter. +${threads ? '\nThe review threads carry the decisions: a decision reached in a resolved thread outweighs a restatement of the diff. Lead with what the review decided and why.\n' : ''} +Frontmatter fields (exactly this format, no other fields): +${frontmatter('')} + +Body structure (every section is required): +## Background +What was being done? What problem came up? + +## Solution +How was it solved? What were the key steps? + +## Lessons +- lesson 1 +- lesson 2 + +## Related Skills +- skill-name (or "none") + +Pick 2-5 tags from these groups: +stack: python, typescript, go, k8s, docker, sglang, cuda +problem: troubleshooting, performance, deployment, config, api +pattern: workflow, pattern, tool-usage, best-practice +scene: debugging, testing, monitoring, security + --- +MR title: ${mr.title} +MR description: +${mr.description} + +Commits: +${commitsFormatted} + +Key diff (first 3000 characters): +${diff3000}${threads ? `\n\nReview threads (${mr.discussions!.length}):\n${threads}` : ''}`; + } + + return `你是团队知识库管理员。从以下 MR 信息提炼一条有价值的团队 learning。 +【必须】用中文撰写，输出完整 Markdown 文档（含 YAML frontmatter）。 +${threads ? '\nReview threads 一节记录了评审达成的决定：已 resolved 的 thread 里的决定比复述 diff 更重要，优先写评审决定了什么、为什么。\n' : ''} +frontmatter 字段（严格按此格式，不要加其他字段）： +${frontmatter('<简短标题，描述核心问题或发现，<60字符>')} body 结构（以下各节必须包含）： ## 背景 @@ -94,7 +198,7 @@ ${mr.description} ${commitsFormatted} 关键 diff（前 3000 字）： -${diff3000}`; +${diff3000}${threads ? `\n\nReview threads（${mr.discussions!.length}）：\n${threads}` : ''}`; } /** @@ -190,6 +294,10 @@ export async function importFromMR(opts: { outputFileName?: string; queueLearning?: (filename: string, content: string) => Promise; dryRun?: boolean; + /** Read the MR's review threads into the extraction (default true; `--no-discussions` turns it off). */ + discussions?: boolean; + /** The team's `language` from teamai.yaml: `en` writes the learning in English. */ + language?: 'en' | 'zh'; }): Promise<{ learning?: LearningDraft; repoUrl: string; learningFile?: string }> { const learningsDirs = opts.learningsDirs ?? [DEFAULT_LEARNINGS_DIR]; @@ -199,8 +307,10 @@ export async function importFromMR(opts: { let mr: MRData; try { - mr = await fetchMR(opts.url); - fetchSpinner.succeed('MR data fetched'); + mr = await fetchMR(opts.url, { discussions: opts.discussions ?? true }); + fetchSpinner.succeed(mr.discussions?.length + ? `MR data fetched (${mr.discussions.length} review thread${mr.discussions.length === 1 ? '' : 's'})` + : 'MR data fetched'); } catch (err: unknown) { fetchSpinner.fail('MR data fetch failed'); throw err; @@ -212,7 +322,7 @@ export async function importFromMR(opts: { let learningContent: string; try { - learningContent = await callClaude(extractMRLearningPrompt(mr)); + learningContent = await callClaude(extractMRLearningPrompt(mr, opts.language)); aiSpinner.succeed('AI analysis complete'); } catch (err: unknown) { aiSpinner.fail('AI analysis failed'); diff --git a/src/import.ts b/src/import.ts index e29b91990..0fa07a679 100644 --- a/src/import.ts +++ b/src/import.ts @@ -26,6 +26,8 @@ interface ImportOptions extends GlobalOptions { fromClaude?: boolean; /** Extract knowledge from one or more merged MR/PR URLs */ fromMr?: string | string[]; + /** Read the MR's review threads into the extraction; `--no-discussions` sets false (#1187) */ + discussions?: boolean; /** iWiki Space ID or page URL for bulk importing iWiki documents */ fromIwiki?: string; /** Whether to resume an interrupted import session */ @@ -250,6 +252,9 @@ export async function importCmd(opts: ImportOptions): Promise { return queued.path; }, dryRun: opts.dryRun, + discussions: opts.discussions, + // `language: en` in teamai.yaml makes the learning English (#1187). + language: teamConfig?.language, }); extractedAll.push({ url, ...extracted }); } catch (e: unknown) { diff --git a/src/index.ts b/src/index.ts index 61e2a46f9..249c18489 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1235,6 +1235,7 @@ program .option('--dir ', 'Extract code knowledge from a local directory (same as --from-repo but no clone)') .addOption(new Option('--from-claude', 'Scan Claude/Cursor rule directories (the Claude root\'s rules/ — ~/.claude or the recorded toolRoots.claude — and ~/.cursor/rules)').hideHelp()) .option('--from-mr ', 'Extract learning from one or more merged MRs/PRs and trigger incremental teamwiki update') + .option('--no-discussions', 'Leave the MR\'s review threads out of the extraction (with --from-mr)') .option('--from-iwiki ', 'Import documents from iWiki Space ID or page URL (requires TAI_PAT_TOKEN)') .addOption(new Option('--resume', 'Resume an interrupted import session').hideHelp()) .option('--all', 'Accept all suggestions without interactive confirmation') @@ -1349,6 +1350,7 @@ ciCmd .option('--write-mode ', 'Write strategy: direct | pending-review', 'direct') .option('--output ', 'Write artifacts to directory') .option('--individual-comments', 'Post each suggestion as separate comment with reaction/resolve support') + .option('--no-discussions', 'Leave the MR\'s review threads out of the extraction') .action(async (cmdOpts) => { const globalOpts = program.opts() as GlobalOptions; const { ciExtractMr } = await import('./ci/extract-mr.js'); diff --git a/src/providers/github/mr-fetch.ts b/src/providers/github/mr-fetch.ts index 31eae425d..1d06d43eb 100644 --- a/src/providers/github/mr-fetch.ts +++ b/src/providers/github/mr-fetch.ts @@ -1,8 +1,9 @@ -import { execSync } from 'node:child_process'; +import { execFileSync, execSync } from 'node:child_process'; import https from 'node:https'; -import { type MRData } from '../../types.js'; +import { type MRData, type MRFetchOptions, type MRThread } from '../../types.js'; import { log } from '../../utils/logger.js'; +import { pruneThreads } from '../mr-threads.js'; /** GitHub PR URL 解析结果 */ interface ParsedGitHubPR { @@ -93,6 +94,93 @@ async function githubApiGet(path: string): Promise { }); } +/** Reads one API path as JSON: through `gh api` or the REST client. */ +type ApiGet = (path: string) => Promise; + +/** `gh api `: the same paths as the REST client, with gh's credentials. */ +async function ghApiGet(path: string): Promise { + const out = execFileSync('gh', ['api', path.replace(/^\//, '')], { maxBuffer: 10 * 1024 * 1024, encoding: 'utf8' }); + return JSON.parse(out); +} + +/** A `/pulls/:n/comments` entry (the fields read). */ +interface GitHubReviewComment { + id: number; + body?: string | null; + path?: string; + line?: number | null; + original_line?: number | null; + /** The thread's first comment, on a reply. */ + in_reply_to_id?: number; + user?: { login: string }; +} + +/** A `/pulls/:n/reviews` entry (the fields read). */ +interface GitHubReview { + body?: string | null; + state?: string; + user?: { login: string }; +} + +/** A `/issues/:n/comments` entry (the fields read). */ +interface GitHubIssueComment { + body?: string | null; + user?: { login: string }; +} + +/** Pages of 100 read at most per list endpoint. */ +const MAX_LIST_PAGES = 10; + +/** Reads every page of a list endpoint. */ +async function readPages(get: ApiGet, path: string): Promise { + const all: T[] = []; + for (let page = 1; page <= MAX_LIST_PAGES; page++) { + const items = await get(`${path}?per_page=100&page=${page}`); + if (!Array.isArray(items)) { + throw new Error(`GitHub API returned no list for ${path}`); + } + all.push(...(items as T[])); + if (items.length < 100) break; + } + return all; +} + +/** + * Reads the PR's review threads (#1187): the diff comments grouped into their + * threads, each review's summary, and the conversation comments, without + * teamai's own CI comments. The REST API does not say whether a thread is + * resolved, so `resolved` stays unset. + * + * @throws Error on an API error; the caller keeps the extraction going without threads + */ +async function fetchGitHubThreads(get: ApiGet, owner: string, repo: string, number: string): Promise { + const base = `/repos/${owner}/${repo}`; + const comments = await readPages(get, `${base}/pulls/${number}/comments`); + const reviews = await readPages(get, `${base}/pulls/${number}/reviews`); + const conversation = await readPages(get, `${base}/issues/${number}/comments`); + + const byRoot = new Map(); + for (const c of comments) { + const note = { author: c.user?.login, body: c.body ?? '' }; + const thread = byRoot.get(c.in_reply_to_id ?? c.id); + if (thread) { + thread.notes.push(note); + } else { + byRoot.set(c.id, { file: c.path, line: c.line ?? c.original_line ?? undefined, notes: [note] }); + } + } + const threads = [...byRoot.values()]; + for (const r of reviews) { + if (!r.body?.trim() || r.state === 'PENDING') continue; + const verdict = r.state ? `${r.state.toLowerCase().replace(/_/g, ' ')}: ` : ''; + threads.push({ notes: [{ author: r.user?.login, body: `${verdict}${r.body}` }] }); + } + for (const c of conversation) { + threads.push({ notes: [{ author: c.user?.login, body: c.body ?? '' }] }); + } + return pruneThreads(threads); +} + /** * 通过 GitHub REST API 获取 PR 数据（gh CLI 不可用时的回退路径）。 */ @@ -157,16 +245,22 @@ async function fetchGitHubPRViaApi(owner: string, repo: string, number: string): * * gh CLI 不可用时自动回退到 GitHub REST API（支持公开仓库，无需 token）。 * + * 评审 threads（diff 评论、review 结论、会话评论）随后通过同一条路径读取， + * 失败非致命（#1187）。 + * * @param url - GitHub PR 完整 web URL，例如 https://github.com/owner/repo/pull/123 + * @param opts - 是否读取评审 threads（默认读取） * @returns 包含标题、描述、提交列表、diff 的 MRData 对象 * @throws Error 当 URL 格式不合法或两种方式均失败时 */ -export async function fetchGitHubPR(url: string): Promise { +export async function fetchGitHubPR(url: string, opts: MRFetchOptions = {}): Promise { const { owner, repo, number } = parseGitHubPRUrl(url); const repoArg = `${owner}/${repo}`; log.debug(`fetchGitHubPR: ${repoArg}#${number}`); + let mr: MRData; + let get: ApiGet; // ── 优先尝试 gh CLI ────────────────────────────────────── try { const viewOutput = execSync( @@ -180,7 +274,7 @@ export async function fetchGitHubPR(url: string): Promise { { maxBuffer: 50 * 1024 * 1024, encoding: 'utf8' }, ); - return { + mr = { title: prView.title, description: prView.body ?? '', author: prView.author?.login, @@ -192,13 +286,24 @@ export async function fetchGitHubPR(url: string): Promise { diff: rawDiff.slice(0, 50000), url, }; + get = ghApiGet; } catch { // gh CLI 不可用或失败，回退到 REST API log.debug('gh CLI unavailable, falling back to GitHub REST API'); + // ── 回退：GitHub REST API ──────────────────────────────── + mr = await fetchGitHubPRViaApi(owner, repo, number); + get = githubApiGet; } - // ── 回退：GitHub REST API ──────────────────────────────── - return fetchGitHubPRViaApi(owner, repo, number); + // ── 评审 threads（失败非致命） ─────────────────────────── + if (opts.discussions !== false) { + try { + mr.discussions = await fetchGitHubThreads(get, owner, repo, number); + } catch (err) { + log.warn(`Review threads not read (${(err as Error).message}); the learning is extracted without them`); + } + } + return mr; } diff --git a/src/providers/gitlab/mr-fetch.ts b/src/providers/gitlab/mr-fetch.ts index 37ff19af9..d3d903042 100644 --- a/src/providers/gitlab/mr-fetch.ts +++ b/src/providers/gitlab/mr-fetch.ts @@ -1,5 +1,6 @@ -import { type MRData } from '../../types.js'; +import { type MRData, type MRFetchOptions, type MRThread } from '../../types.js'; import { log } from '../../utils/logger.js'; +import { pruneThreads } from '../mr-threads.js'; import { getGitLabToken, gitlabBaseUrl } from './gitlab-api.js'; import { GITLAB_HOST } from './repo-url.js'; @@ -97,6 +98,72 @@ function authHeaders(token: string): Record { return { 'PRIVATE-TOKEN': token, 'Accept': 'application/json' }; } +/** A note of a `/discussions` entry (the fields read). */ +interface GitLabDiscussionNote { + body: string; + author?: { username: string }; + /** A system note ("changed the description", "approved"), not a review remark. */ + system?: boolean; + resolvable?: boolean; + resolved?: boolean; + /** Set for a diff comment: the file and line the thread is anchored to. */ + position?: { + new_path?: string | null; + old_path?: string | null; + new_line?: number | null; + old_line?: number | null; + }; +} + +/** One entry of `GET /projects/:id/merge_requests/:iid/discussions`. */ +interface GitLabDiscussion { + notes?: GitLabDiscussionNote[]; +} + +/** Pages of 100 discussions read at most; an MR with more review is unusual. */ +const MAX_DISCUSSION_PAGES = 20; + +/** Maps a GitLab discussion to a thread; system notes are left out. */ +function toThread(discussion: GitLabDiscussion): MRThread { + const notes = (discussion.notes ?? []).filter((n) => !n.system); + const position = notes[0]?.position; + const resolvable = notes.find((n) => n.resolvable); + return { + file: position?.new_path ?? position?.old_path ?? undefined, + line: position?.new_line ?? position?.old_line ?? undefined, + resolved: resolvable ? resolvable.resolved === true : undefined, + notes: notes.map((n) => ({ author: n.author?.username, body: n.body })), + }; +} + +/** + * Reads the MR's review threads, page by page (`x-next-page`), without + * system notes or teamai's own CI comments (#1187). + * + * @throws Error on an API error; the caller keeps the extraction going without threads + */ +async function fetchGitLabDiscussions( + apiBase: string, + enc: string, + mrIid: string, + headers: Record, +): Promise { + const threads: MRThread[] = []; + for (let page = 1; page <= MAX_DISCUSSION_PAGES; page++) { + const resp = await fetch( + `${apiBase}/projects/${enc}/merge_requests/${mrIid}/discussions?per_page=100&page=${page}`, + { headers, redirect: 'manual' }, + ); + if (!resp.ok) { + throw new Error(`GitLab API error ${resp.status}`); + } + const raw = (await resp.json()) as GitLabDiscussion[]; + threads.push(...raw.map(toThread)); + if (!resp.headers.get('x-next-page')) break; + } + return pruneThreads(threads); +} + /** * 通过 GitLab REST API（/api/v4）获取 MR 的完整数据。 * @@ -104,13 +171,16 @@ function authHeaders(token: string): Record { * 2. GET /projects/{enc}/merge_requests/{iid}/commits 获取提交列表 * 3. GET /projects/{enc}/merge_requests/{iid}/changes 获取 diff * （截断至 50KB，失败非致命） + * 4. GET /projects/{enc}/merge_requests/{iid}/discussions 获取评审 threads + * （opts.discussions !== false 时；失败非致命，#1187） * * @param url - GitLab MR 完整 web URL，例如 * https://gitlab.com/group/repo/-/merge_requests/456 + * @param opts - 是否读取评审 threads（默认读取） * @returns 包含标题、描述、提交列表、diff 的 MRData 对象 * @throws Error 当 URL 格式不合法、未配置 token 或 API 调用失败时 */ -export async function fetchGitLabMR(url: string): Promise { +export async function fetchGitLabMR(url: string, opts: MRFetchOptions = {}): Promise { const { apiBase, projectPath, mrIid } = parseGitLabMRUrl(url); const token = getGitLabToken(); if (!token) { @@ -163,6 +233,16 @@ export async function fetchGitLabMR(url: string): Promise { log.debug(`GitLab MR diff 获取异常，diff 将为空：${(err as Error).message}`); } + // ── 4. 获取评审 threads（失败非致命） ───────────────────── + let discussions: MRThread[] | undefined; + if (opts.discussions !== false) { + try { + discussions = await fetchGitLabDiscussions(apiBase, enc, mrIid, headers); + } catch (err) { + log.warn(`Review threads not read (${(err as Error).message}); the learning is extracted without them`); + } + } + return { title: mr.title, description: mr.description ?? '', @@ -171,5 +251,6 @@ export async function fetchGitLabMR(url: string): Promise { commits, diff, url, + ...(discussions ? { discussions } : {}), }; } diff --git a/src/providers/mr-threads.ts b/src/providers/mr-threads.ts new file mode 100644 index 000000000..1f2c7168f --- /dev/null +++ b/src/providers/mr-threads.ts @@ -0,0 +1,27 @@ +import type { MRThread } from '../types.js'; + +/** + * Every comment `teamai ci extract-mr` posts carries an HTML marker with this + * prefix (``, ``, the + * codebase-graph marker). A fetch of the review threads leaves them out so the + * CI extraction does not feed its own output back in (#1187). + */ +const TEAMAI_COMMENT_PREFIX = '', + }); + + expect(mockImportFromMR).toHaveBeenCalledWith(expect.objectContaining({ + commentMarker: '', + })); + }); + it('write 模式需要 --team-repo 参数', async () => { await expect(ciExtractMr({ url: 'https://github.com/org/repo/pull/1', diff --git a/src/__tests__/import-mr-threads.test.ts b/src/__tests__/import-mr-threads.test.ts index 8ad7a6b17..a300187b1 100644 --- a/src/__tests__/import-mr-threads.test.ts +++ b/src/__tests__/import-mr-threads.test.ts @@ -40,6 +40,17 @@ describe('pruneThreads', () => { ]); expect(threads).toEqual([remark]); }); + + it('drops comments posted with a custom --comment-marker as well', () => { + const threads = pruneThreads([ + { notes: [{ author: 'ci', body: '\n## TeamAI 知识提炼' }] }, + { notes: [{ author: 'ci', body: '' }] }, + remark, + ], ''); + expect(threads).toEqual([remark]); + // Without the marker the custom comment is an ordinary note. + expect(pruneThreads([{ notes: [{ author: 'ci', body: '' }] }])).toHaveLength(1); + }); }); describe('formatMrThreads', () => { diff --git a/src/ci/extract-mr.ts b/src/ci/extract-mr.ts index f0d6c4340..c4906257c 100644 --- a/src/ci/extract-mr.ts +++ b/src/ci/extract-mr.ts @@ -252,6 +252,8 @@ export async function ciExtractMr(opts: CiExtractMrOptions): Promise { learningsDirs: opts.teamRepo ? [path.join(opts.teamRepo, 'learnings')] : undefined, dryRun: true, // 不让 importFromMR 自己写文件，我们自己控制写入 discussions: opts.discussions, + // A custom marker's comments are teamai's own too: keep them out of the threads. + commentMarker: opts.commentMarker, language, }); learning = result.learning; diff --git a/src/import-mr.ts b/src/import-mr.ts index dfaf8fbf6..ba9b75c4d 100644 --- a/src/import-mr.ts +++ b/src/import-mr.ts @@ -296,6 +296,8 @@ export async function importFromMR(opts: { dryRun?: boolean; /** Read the MR's review threads into the extraction (default true; `--no-discussions` turns it off). */ discussions?: boolean; + /** The marker `ci extract-mr --comment-marker` posts with, so its own comments stay out of the threads. */ + commentMarker?: string; /** The team's `language` from teamai.yaml: `en` writes the learning in English. */ language?: 'en' | 'zh'; }): Promise<{ learning?: LearningDraft; repoUrl: string; learningFile?: string }> { @@ -307,7 +309,7 @@ export async function importFromMR(opts: { let mr: MRData; try { - mr = await fetchMR(opts.url, { discussions: opts.discussions ?? true }); + mr = await fetchMR(opts.url, { discussions: opts.discussions ?? true, commentMarker: opts.commentMarker }); fetchSpinner.succeed(mr.discussions?.length ? `MR data fetched (${mr.discussions.length} review thread${mr.discussions.length === 1 ? '' : 's'})` : 'MR data fetched'); diff --git a/src/providers/github/mr-fetch.ts b/src/providers/github/mr-fetch.ts index 1d06d43eb..e925b749d 100644 --- a/src/providers/github/mr-fetch.ts +++ b/src/providers/github/mr-fetch.ts @@ -153,7 +153,13 @@ async function readPages(get: ApiGet, path: string): Promise { * * @throws Error on an API error; the caller keeps the extraction going without threads */ -async function fetchGitHubThreads(get: ApiGet, owner: string, repo: string, number: string): Promise { +async function fetchGitHubThreads( + get: ApiGet, + owner: string, + repo: string, + number: string, + commentMarker?: string, +): Promise { const base = `/repos/${owner}/${repo}`; const comments = await readPages(get, `${base}/pulls/${number}/comments`); const reviews = await readPages(get, `${base}/pulls/${number}/reviews`); @@ -178,7 +184,7 @@ async function fetchGitHubThreads(get: ApiGet, owner: string, repo: string, numb for (const c of conversation) { threads.push({ notes: [{ author: c.user?.login, body: c.body ?? '' }] }); } - return pruneThreads(threads); + return pruneThreads(threads, commentMarker); } /** @@ -298,7 +304,7 @@ export async function fetchGitHubPR(url: string, opts: MRFetchOptions = {}): Pro // ── 评审 threads（失败非致命） ─────────────────────────── if (opts.discussions !== false) { try { - mr.discussions = await fetchGitHubThreads(get, owner, repo, number); + mr.discussions = await fetchGitHubThreads(get, owner, repo, number, opts.commentMarker); } catch (err) { log.warn(`Review threads not read (${(err as Error).message}); the learning is extracted without them`); } diff --git a/src/providers/gitlab/mr-fetch.ts b/src/providers/gitlab/mr-fetch.ts index d3d903042..4b84c30d1 100644 --- a/src/providers/gitlab/mr-fetch.ts +++ b/src/providers/gitlab/mr-fetch.ts @@ -147,6 +147,7 @@ async function fetchGitLabDiscussions( enc: string, mrIid: string, headers: Record, + commentMarker?: string, ): Promise { const threads: MRThread[] = []; for (let page = 1; page <= MAX_DISCUSSION_PAGES; page++) { @@ -161,7 +162,7 @@ async function fetchGitLabDiscussions( threads.push(...raw.map(toThread)); if (!resp.headers.get('x-next-page')) break; } - return pruneThreads(threads); + return pruneThreads(threads, commentMarker); } /** @@ -237,7 +238,7 @@ export async function fetchGitLabMR(url: string, opts: MRFetchOptions = {}): Pro let discussions: MRThread[] | undefined; if (opts.discussions !== false) { try { - discussions = await fetchGitLabDiscussions(apiBase, enc, mrIid, headers); + discussions = await fetchGitLabDiscussions(apiBase, enc, mrIid, headers, opts.commentMarker); } catch (err) { log.warn(`Review threads not read (${(err as Error).message}); the learning is extracted without them`); } diff --git a/src/providers/mr-threads.ts b/src/providers/mr-threads.ts index 1f2c7168f..70a2a6795 100644 --- a/src/providers/mr-threads.ts +++ b/src/providers/mr-threads.ts @@ -8,20 +8,25 @@ import type { MRThread } from '../types.js'; */ const TEAMAI_COMMENT_PREFIX = '