Skip to content

feat(import): read the MR's review threads into the --from-mr extraction (#1187) - #1190

Merged
jeff-r2026 merged 3 commits into
Tencent:mainfrom
scpia:feat/import-mr-discussions
Oct 18, 2026
Merged

jeff-r2026 merged 3 commits into
Tencent:mainfrom
scpia:feat/import-mr-discussions

Conversation

@scpia

@scpia scpia commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Summary

teamai import --from-mr and teamai ci extract-mr now read the MR's review threads into the extraction, alongside the description, commits and diff, so a learning carries the decision a review reached instead of a summary of the diff. In the same prompt, language: en in teamai.yaml makes the learning English. Closes #1187.

What is fetched:

  • GitLab: GET /projects/:id/merge_requests/:iid/discussions, paginated via x-next-page.
  • GitHub: review comments (/pulls/:n/comments, grouped into threads by in_reply_to_id), review bodies (/pulls/:n/reviews, prefixed with the verdict, pending reviews skipped) and conversation comments (/issues/:n/comments), through gh api when gh served the PR and through the REST fallback otherwise.
  • TGit: the notes endpoint teamai ci already uses, grouped by discussion_id where the instance reports one.

System notes and teamai's own <!-- teamai: CI comments are left out, so the CI extraction does not feed its output back in.

The threads get their own section of the prompt with its own budget (6000 characters). Per thread: the file and line it is anchored to, whether it is resolved, and the notes in order, each marked [author <user>] or [reviewer <user>]. Threads with a reply come first, single unanswered remarks last; a thread over 1500 characters keeps its first and last notes, and once the budget is reached the remaining threads are counted rather than the section cut off mid-thread. The model is told that a decision in a resolved thread outweighs a restatement of the diff.

A failure to read the threads warns (Review threads not read (...)) and extracts without them. --no-discussions on import and on ci extract-mr keeps the previous behaviour.

language: en gives an English prompt with English headings (## Background / ## Solution / ## Lessons / ## Related Skills); zh or unset keeps the Chinese prompt byte for byte.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

  • npx tsc --noEmit passes
  • npm run lint passes
  • npx vitest run passes — 10836 passed; the only failures are the two submodule-checkout.test.ts cases (git config --get hook.teamai-post-checkout.command fails on this machine), unrelated to this change
  • Added/updated tests for the change

New unit tests: import-mr-threads.test.ts (prune, ordering, author marking, resolution labels, long-thread shortening, budget cut, prompt section, language variants), gitlab-mr-discussions.test.ts (pagination, system/teamai filtering, anchors, discussions: false, API error), github-mr-fetch.test.ts (threads from the three list endpoints, discussions: false, list error), tgit-mr-discussions.test.ts (grouping by discussion_id, discussions: false, notes error), import-mr.test.ts (--no-discussions plumbing, language: en reaches the prompt). The gitlab-provider.test.ts API-prefix cases now also expect the discussions URL.

Real-CLI run (after npm run build)

Against the merged PR #1056 (19 conversation comments, one approving review), with the real gh (2.94.0) and claude (2.1.268), a project-scope sandbox config with language: en, and --output so nothing is published.

$ teamai import --from-mr https://github.com/Tencent/teamai-cli/pull/1056 --output with-threads --all
✔ MR data fetched (19 review threads)
✔ AI analysis complete
ℹ ✅ Learning draft generated: No retro-prune: git history already holds leaked names
ℹ    Tags: typescript, security, best-practice, testing
ℹ Learning written: .../with-threads/learning.md
exit: 0

$ teamai import --from-mr https://github.com/Tencent/teamai-cli/pull/1056 --output without-threads --all --no-discussions
✔ MR data fetched
✔ AI analysis complete
ℹ ✅ Learning draft generated: Filter private data at the publish boundary, not at collection
ℹ Learning written: .../without-threads/learning.md
exit: 0

With the threads, the learning leads with the review's decision and its reason (from its ## Solution):

The PR shipped prune (strip previously-published non-shared names on the next rewrite) and the review overturned it. [...] A maintainer (hsuchifeng) decided against it: the names remain in git history either way, and rewriting that history is costly. Pruning the current file buys no real privacy while adding remediation work.

Without them, the same MR gives a summary of the diff, and the decision appears only as "flipped to conservative keep in commit 2 once reviewers weighed in", with no reason. Both learnings are in English, from language: en.

An earlier attempt with gh unauthenticated (sandbox HOME) exercised the REST fallback: ✔ MR data fetched (19 review threads) as well.

Not covered here: TGit (no access from this environment; its path follows the notes endpoint src/ci/mr-comment.ts already uses) and a live GitLab instance (the fetcher is covered by the unit tests above). Deferred to CI / reviewers with access.

Related Issues

Closes #1187

Notes for Reviewers

  • MRData.discussions is optional and absent when the fetch left threads out, so every provider without the feature (GitCode) and every existing caller is unchanged.
  • The GitHub REST API does not say whether a review thread is resolved, so resolved stays unset there; GitLab reports it.
  • src/providers/mr-threads.ts holds the one shared piece (dropping teamai's own comments); the prompt rendering is formatMrThreads in src/import-mr.ts.
  • The English prompt is a second template rather than a translation layer, so the Chinese prompt stays byte-identical for teams without language.

@scpia
scpia force-pushed the feat/import-mr-discussions branch from d9c0f40 to 6be73ef Compare October 16, 2026 17:01
…ion (Tencent#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 `<!-- teamai:` CI comments are left
out so the CI extraction does not feed its own output back in.

The threads get a section of the prompt with its own budget (6000
characters): per thread the file and line it is anchored to, whether it
is resolved, and its notes in order, each marked as the MR author's or a
reviewer's. Threads with a reply come first and single remarks last; a
thread over 1500 characters keeps its first and last notes, and once the
budget is reached the remaining threads are counted instead of the
section being cut off. The model is told that a decision in a resolved
thread outweighs a restatement of the diff. A failure to read the
threads warns and extracts without them. `--no-discussions` keeps the
previous behaviour.

In the same function, `language: en` in teamai.yaml now makes the
learning English with English headings; `zh` or unset keeps the Chinese
prompt byte for byte.
@scpia
scpia force-pushed the feat/import-mr-discussions branch from 6be73ef to 9a1c254 Compare October 16, 2026 17:26
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] src/providers/mr-threads.ts:14: Filtering only <!-- teamai: misses comments posted with the supported custom --comment-marker, such as <!-- learning-bot -->. Subsequent ci extract-mr runs then ingest their own previous learning as review input. Pass the configured marker through to filtering while retaining the default-prefix exclusion.

The description documents sufficient testing: post-build real-CLI runs cover extraction with and without discussions. Live GitLab/TGit coverage is explicitly deferred; no blocking testing finding.

…view threads (Tencent#1187)

`teamai ci extract-mr --comment-marker <marker>` posts its learning under
that marker instead of the default `<!-- teamai:` prefix, so the thread
filter missed those comments and a later run fed its own output back in.
The marker now travels from `ci extract-mr` through `importFromMR` and
`MRFetchOptions` into every provider's thread fetch, which drops it along
with the default prefix.
@github-actions

Copy link
Copy Markdown

No new findings in the specified diff.

  • Resolved: The earlier custom --comment-marker finding is fixed: CI passes the marker through extraction to all three providers, and filtering retains the default-prefix exclusion.
  • Testing: The description documents sufficient post-build real-CLI verification, including extraction with and without discussions and the REST fallback. Live GitLab/TGit coverage is explicitly deferred; no blocking testing finding.

Review was read-only; nothing from the PR was run, built, or installed.

…ussions

# Conflicts:
#	src/__tests__/github-mr-fetch.test.ts
@github-actions

Copy link
Copy Markdown

No new findings in the specified diff.

  • Resolved: The earlier custom --comment-marker finding is fixed. CI passes the marker through extraction to all three providers, while filtering retains the default <!-- teamai: exclusion.
  • Testing: The description documents sufficient post-build real-CLI verification, including extraction with and without discussions and the REST fallback. Live GitLab/TGit coverage is explicitly deferred; no blocking testing finding.

Review was read-only; nothing from the PR was run, built, or installed.

@jeff-r2026
jeff-r2026 merged commit 58706fe into Tencent:main Oct 18, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] import --from-mr: include the MR's review threads in the extraction

2 participants