Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ and the versioning follows [Semantic Versioning](https://semver.org/).

### 🔧 Fixed

- A generated PR description no longer ends with a blank card — an invisible marker the review engine now stamps into its output was being treated as a section of its own.
- Diagrams in a generated PR description are laid out left-to-right again, instead of turning tall and top-down once they get past a few steps and pushing the rest of the result off screen.
- The commit divider in the review timeline now stays where the commit boundary actually is, instead of sliding further down every time a new message appears below it.
- The file list in a generated PR description now shows the real number of added and removed lines per file, instead of `+-1/--1`.
- Links in that file list now open the file instead of pointing at a non-existent line, so clicking through works.
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.zh-CN.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

### 🔧 修复

- 生成的 PR 描述末尾不再出现空白卡片——评审引擎新加入输出的一个不可见标记此前被当成了独立分块。
- 生成的 PR 描述中的图表恢复为从左至右布局,不再在步骤稍多时变成纵向长图、把其余内容挤出屏幕。
- 评审时间线中的提交分割线现在固定在提交边界的真实位置,不再因为下方出现新消息而一路下移。
- 生成的 PR 描述中,文件清单现在显示每个文件真实的增删行数,不再是 `+-1/--1`。
- 该清单中的链接现在指向文件本身,而非一个不存在的行号,点击可正常跳转。
Expand Down
2 changes: 1 addition & 1 deletion docs/arch/02-agent/05-pragent-runtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Current patches (pinned pr-agent **0.45.0**):

**Retired patches** (kept as a record, because "why is this no longer patched" is the question an upgrade raises): *binary-safe diff* and the *single-line-hunk phantom line* were both **fixed upstream in 0.45.0** — `get_diff_files` now skips a file that fails to decode, and `extract_hunk_headers` defaults an omitted hunk size to 1 rather than 0. Both were verified against the installed runtime before deletion, not assumed from release notes.

**Upstream defaults that must stay pinned**: 0.45.0 turned on three features that append to the output this app *parses* — `pr_reviewer.persistent_finding_state` (a "resolved findings" section carrying upstream's own cross-run state, which the app already owns via drafts / finding closures / re-review verdicts), and two coverage footers. They are forced off in `buildPragentEnv` rather than left at their defaults. **Re-check this list on every upgrade**: a new default that adds a section is not a free improvement here — it lands as a bogus finding.
**Upstream defaults that must stay pinned** (in `buildPragentEnv`): 0.45.0 turned on three features that append to the output this app *parses* — `pr_reviewer.persistent_finding_state` (a "resolved findings" section carrying upstream's own cross-run state, which the app already owns via drafts / finding closures / re-review verdicts) and two coverage footers — plus `pr_description.pr_diagram_direction='adaptive'`, which turns a longer diagram top-down. **Re-check this list on every upgrade**, and note the two distinct ways a default can be wrong here: one *adds output* (landing as a bogus finding), the other *changes presentation* for a surface upstream does not know about — pr-agent assumes a full-width page, while this diagram is read in a narrow chat column. Neither shows up as an error.

### Real token usage

Expand Down
4 changes: 4 additions & 0 deletions packages/poller/src/parse-output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ function trimNoise(body: string): string {
const trimmed = l.trim();
if (trimmed === '') return true;
if (/^(?:[-*_]\s*){3,}$/.test(trimmed)) return true; // markdown HR
// A line that is nothing but an HTML comment: invisible once rendered, so it is not content. pr-agent 0.45.0
// stamps `<!-- pr-agent-generated -->` at the top of its output, and counting that as content produced a section
// with a body that renders to nothing — an empty card in the run result.
if (/^<!--[\s\S]*-->$/.test(trimmed)) return true;
if (INTERNAL_BRANCH_RE.test(trimmed) && trimmed.length < 40) return true; // short line + contains branch name
return false;
};
Expand Down
27 changes: 27 additions & 0 deletions packages/poller/tests/parse-output.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,3 +772,30 @@ describe('splitMarkdownSections minLevel', () => {
expect(findings.some((f) => /package-lock/i.test(f.title ?? ''))).toBe(false);
});
});

describe('invisible-only sections', () => {
// Regression: pr-agent 0.45.0 stamps `<!-- pr-agent-generated -->` at the top of its output. That landed in the
// leading (title-less) section, whose body was therefore "not empty" — but an HTML comment renders to nothing, so
// the run result grew a card that looked blank.
it('drops a section whose body is only an HTML comment', () => {
const md = [
'pr-7c82edaec417/head',
'<!-- pr-agent-generated -->',
'### **PR Type**',
'Enhancement',
].join('\n');
const { findings } = parseReviewOutput(md, 'describe');
expect(findings.map((f) => f.title)).toEqual(['PR Type']);
});

it('keeps a section that has real content alongside the comment', () => {
const md = [
'<!-- pr-agent-generated -->',
'actual prose',
'### **PR Type**',
'Enhancement',
].join('\n');
const { findings } = parseReviewOutput(md, 'describe');
expect(findings.some((f) => (f.body ?? '').includes('actual prose'))).toBe(true);
});
});
5 changes: 5 additions & 0 deletions packages/pr-agent-bridge/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ function normalizeModel(provider: LlmProfile['provider'], model: string): string
* — so upstream's version would both duplicate it and parse as extra findings.
* - `PR_REVIEWER__ENABLE_REVIEW_COVERAGE_FOOTER=false` / `PR_CODE_SUGGESTIONS__ENABLE_SUGGESTIONS_COVERAGE_FOOTER=false`
* (new in 0.45, both default **true**): a footer appended to the body, which the parser would read as content.
* - `PR_DESCRIPTION__PR_DIAGRAM_DIRECTION=LR` (new in 0.45, defaults `adaptive`): `adaptive` turns any chain longer
* than a few nodes top-down, which is the wrong trade for where this diagram is read — a narrow chat column, where
* a tall diagram pushes everything else off screen while a wide one scrolls sideways and can be opened in the
* preview view. Pinned left-to-right rather than left to a heuristic tuned for a full-width page.
*/
export function buildPragentEnv(profile: LlmProfile, maxModelTokens?: number): Record<string, string> {
const env: Record<string, string> = {};
Expand All @@ -103,6 +107,7 @@ export function buildPragentEnv(profile: LlmProfile, maxModelTokens?: number): R
env['PR_REVIEWER__PERSISTENT_FINDING_STATE'] = 'false';
env['PR_REVIEWER__ENABLE_REVIEW_COVERAGE_FOOTER'] = 'false';
env['PR_CODE_SUGGESTIONS__ENABLE_SUGGESTIONS_COVERAGE_FOOTER'] = 'false';
env['PR_DESCRIPTION__PR_DIAGRAM_DIRECTION'] = 'LR';
// On import litellm fetches the remote model price table over the network (raw.githubusercontent.com); on an intranet/weak network
// the SSL timeout slows startup and floods warnings. We only take the real token count (from API response.usage),
// don't need the price table → force using only the in-package local backup, no network at all. See sitecustomize's usage callback.
Expand Down
Loading