Skip to content

Commit dc65bed

Browse files
committed
Fix github-comment tests to use actual code and document diff truncation
- Export buildCommentBody from github-comment.ts so tests can import it directly instead of reimplementing the function - Rewrite github-comment.test.ts to import and test the real function - Add "Diff truncation" section to README documenting the 8k/10k char limits in the change summarizer and script generator stages https://claude.ai/code/session_01PAVFqsnf36BamyXEiwS8DU
1 parent 369b694 commit dc65bed

File tree

3 files changed

+29
-94
lines changed

3 files changed

+29
-94
lines changed

README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,19 @@ See [CLAUDE.md](CLAUDE.md) for repo structure and contributor notes.
527527

528528
---
529529

530+
## Diff truncation
531+
532+
Large diffs are automatically truncated before being sent to the LLM to stay within reasonable token budgets:
533+
534+
| Stage | Character limit | Purpose |
535+
|---|---|---|
536+
| Change summarizer | 8 000 chars | Analyzing what changed and suggesting a demo flow |
537+
| Script generator | 10 000 chars | Generating the Playwright interaction script |
538+
539+
When a diff exceeds the limit, it is cut at the threshold and a `... (diff truncated)` marker is appended so the LLM knows the input is incomplete. In practice this means very large PRs may produce less accurate scripts — consider using `routeMap` or `app.hint` to give the LLM additional context when working with big diffs.
540+
541+
---
542+
530543
## Known limitations
531544

532545
- **Single entry point** — only one preview URL or start command per run is supported; multiple entry points are planned.

packages/core/src/publisher/github-comment.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ async function findExistingComment(
103103
return existing ? { id: existing.id } : null;
104104
}
105105

106-
function buildCommentBody(options: CommentOptions): string {
106+
export function buildCommentBody(options: CommentOptions): string {
107107
const { analysis, recordingUrl, screenshots, script, rerunUrl } = options;
108108

109109
const changedFilesList = analysis.changedFiles

tests/unit/github-comment.test.ts

Lines changed: 15 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1,99 +1,17 @@
1-
import { describe, it, expect, vi, beforeEach } from 'vitest';
1+
import { describe, it, expect } from 'vitest';
2+
import { buildCommentBody } from '../../packages/core/src/publisher/github-comment.js';
23
import type { ChangeAnalysis } from '../../packages/core/src/analyzer/change-summarizer.js';
34

4-
const mockCreateComment = vi.fn();
5-
const mockUpdateComment = vi.fn();
6-
const mockListComments = vi.fn();
7-
8-
// Mock the entire github-comment module's Octokit dependency
9-
// by mocking the source module itself and re-implementing with our stubs
10-
vi.mock('../../packages/core/src/publisher/github-comment.js', async (importOriginal) => {
11-
// We need to intercept the Octokit constructor that the original module uses.
12-
// Replace @octokit/rest in the module resolution before the original loads.
13-
vi.stubGlobal('__mockOctokit', {
14-
rest: {
15-
issues: {
16-
createComment: mockCreateComment,
17-
updateComment: mockUpdateComment,
18-
listComments: mockListComments,
19-
},
20-
},
21-
});
22-
23-
// Instead, let's manually implement the functions to test the comment body logic
24-
return importOriginal();
25-
});
26-
27-
// Since we can't easily mock nested deps, let's test comment body construction
28-
// by calling the exported functions with a properly mocked Octokit.
29-
// The real issue is @octokit/rest module resolution path — let's work around it.
30-
31-
describe('postPRComment comment body construction', () => {
32-
// Since mocking Octokit across pnpm workspace boundaries is unreliable,
33-
// we test the comment body logic by examining the function contract.
34-
// We replicate the internal buildCommentBody logic to verify its behavior.
35-
5+
describe('buildCommentBody', () => {
366
const ANALYSIS: ChangeAnalysis = {
377
changedFiles: ['app/routes/home.tsx', 'src/components/Button.tsx'],
388
affectedRoutes: [{ file: 'app/routes/home.tsx', route: '/', changeType: 'modified' }],
399
changeDescription: 'Added a virtual try-on button',
4010
suggestedDemoFlow: '1. Navigate to home page\n2. Click try-on button',
4111
};
4212

43-
// Replicate buildCommentBody logic for direct testing
44-
function buildCommentBody(options: {
45-
analysis: ChangeAnalysis;
46-
recordingUrl?: string;
47-
screenshots?: string[];
48-
script: string;
49-
rerunUrl?: string;
50-
}): string {
51-
const COMMENT_MARKER = '<!-- git-glimpse-demo -->';
52-
const { analysis, recordingUrl, screenshots, script, rerunUrl } = options;
53-
54-
const changedFilesList = analysis.changedFiles
55-
.slice(0, 5)
56-
.map((f) => `\`${f}\``)
57-
.join(', ');
58-
const moreFiles = analysis.changedFiles.length > 5
59-
? ` (+${analysis.changedFiles.length - 5} more)`
60-
: '';
61-
62-
const mediaSection = recordingUrl
63-
? `![Demo](${recordingUrl})\n\n[📱 Can't see the preview? Open it directly](${recordingUrl})`
64-
: screenshots && screenshots.length > 0
65-
? screenshots
66-
.map((s, i) => `![Screenshot ${i + 1}](${s})\n\n[📱 Can't see screenshot ${i + 1}? Open it directly](${s})`)
67-
.join('\n\n')
68-
: '_No recording available._';
69-
70-
const rerunSection = rerunUrl ? `\n\n[↺ Re-run demo](${rerunUrl})` : '';
71-
72-
return `${COMMENT_MARKER}
73-
## 🧐 UI Demo Preview
74-
75-
**Changes detected in**: ${changedFilesList}${moreFiles}
76-
77-
**What changed**: ${analysis.changeDescription}
78-
79-
${mediaSection}
80-
81-
<details>
82-
<summary>Demo script (auto-generated)</summary>
83-
84-
\`\`\`typescript
85-
${script}
86-
\`\`\`
87-
</details>
88-
89-
---
90-
*Generated by [git-glimpse](https://github.com/DeDuckProject/git-glimpse)${rerunSection}*
91-
92-
<img src="https://raw.githubusercontent.com/DeDuckProject/git-glimpse/main/assets/logo_square_small.png" width="90" height="90" alt="git-glimpse logo" />`;
93-
}
94-
9513
it('includes comment marker for idempotent updates', () => {
96-
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()' });
14+
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()', owner: 'o', repo: 'r', pullNumber: 1 });
9715
expect(body).toContain('<!-- git-glimpse-demo -->');
9816
});
9917

@@ -102,6 +20,7 @@ ${script}
10220
analysis: ANALYSIS,
10321
recordingUrl: 'https://example.com/demo.gif',
10422
script: 'demo()',
23+
owner: 'o', repo: 'r', pullNumber: 1,
10524
});
10625
expect(body).toContain('![Demo](https://example.com/demo.gif)');
10726
expect(body).toContain('Open it directly');
@@ -112,14 +31,15 @@ ${script}
11231
analysis: ANALYSIS,
11332
screenshots: ['https://example.com/s1.png', 'https://example.com/s2.png'],
11433
script: 'demo()',
34+
owner: 'o', repo: 'r', pullNumber: 1,
11535
});
11636
expect(body).toContain('Screenshot 1');
11737
expect(body).toContain('Screenshot 2');
11838
expect(body).not.toContain('No recording available');
11939
});
12040

12141
it('shows fallback when neither recording nor screenshots are present', () => {
122-
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()' });
42+
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()', owner: 'o', repo: 'r', pullNumber: 1 });
12343
expect(body).toContain('No recording available');
12444
});
12545

@@ -128,27 +48,27 @@ ${script}
12848
...ANALYSIS,
12949
changedFiles: ['a.tsx', 'b.tsx', 'c.tsx', 'd.tsx', 'e.tsx', 'f.tsx', 'g.tsx'],
13050
};
131-
const body = buildCommentBody({ analysis: manyFilesAnalysis, script: 'demo()' });
51+
const body = buildCommentBody({ analysis: manyFilesAnalysis, script: 'demo()', owner: 'o', repo: 'r', pullNumber: 1 });
13252
expect(body).toContain('`a.tsx`');
13353
expect(body).toContain('`e.tsx`');
13454
expect(body).not.toContain('`f.tsx`');
13555
expect(body).toContain('+2 more');
13656
});
13757

13858
it('shows all files when 5 or fewer', () => {
139-
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()' });
59+
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()', owner: 'o', repo: 'r', pullNumber: 1 });
14060
expect(body).toContain('`app/routes/home.tsx`');
14161
expect(body).toContain('`src/components/Button.tsx`');
14262
expect(body).not.toContain('more');
14363
});
14464

14565
it('includes the change description', () => {
146-
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()' });
66+
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()', owner: 'o', repo: 'r', pullNumber: 1 });
14767
expect(body).toContain('Added a virtual try-on button');
14868
});
14969

15070
it('wraps script in collapsible details with typescript code block', () => {
151-
const body = buildCommentBody({ analysis: ANALYSIS, script: 'export async function demo(page) {}' });
71+
const body = buildCommentBody({ analysis: ANALYSIS, script: 'export async function demo(page) {}', owner: 'o', repo: 'r', pullNumber: 1 });
15272
expect(body).toContain('<details>');
15373
expect(body).toContain('Demo script (auto-generated)');
15474
expect(body).toContain('```typescript');
@@ -160,18 +80,19 @@ ${script}
16080
analysis: ANALYSIS,
16181
script: 'demo()',
16282
rerunUrl: 'https://github.com/owner/repo/actions/runs/123',
83+
owner: 'o', repo: 'r', pullNumber: 1,
16384
});
16485
expect(body).toContain('Re-run demo');
16586
expect(body).toContain('https://github.com/owner/repo/actions/runs/123');
16687
});
16788

16889
it('omits rerun link when not provided', () => {
169-
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()' });
90+
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()', owner: 'o', repo: 'r', pullNumber: 1 });
17091
expect(body).not.toContain('Re-run demo');
17192
});
17293

17394
it('includes git-glimpse branding', () => {
174-
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()' });
95+
const body = buildCommentBody({ analysis: ANALYSIS, script: 'demo()', owner: 'o', repo: 'r', pullNumber: 1 });
17596
expect(body).toContain('git-glimpse');
17697
expect(body).toContain('logo_square_small.png');
17798
});
@@ -182,6 +103,7 @@ ${script}
182103
recordingUrl: 'https://example.com/demo.gif',
183104
screenshots: ['https://example.com/s1.png'],
184105
script: 'demo()',
106+
owner: 'o', repo: 'r', pullNumber: 1,
185107
});
186108
expect(body).toContain('![Demo]');
187109
expect(body).not.toContain('Screenshot 1');

0 commit comments

Comments
 (0)