Skip to content

Add attribution test writing guidelines to AGENTS.md#1033

Closed
jwiegley wants to merge 1 commit into
johnw/review-integration-testsfrom
johnw/review-attribution-test-guidelines
Closed

Add attribution test writing guidelines to AGENTS.md#1033
jwiegley wants to merge 1 commit into
johnw/review-integration-testsfrom
johnw/review-attribution-test-guidelines

Conversation

@jwiegley

@jwiegley jwiegley commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Document patterns and conventions for writing integration tests
that exercise the attribution system, including test harness setup,
daemon interaction, and assertion helpers.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

jwiegley commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from ed02bb9 to 9e18ac5 Compare April 9, 2026 17:28
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch 2 times, most recently from 45f5332 to dda561b Compare April 9, 2026 20:13
@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from 9e18ac5 to 98654e8 Compare April 9, 2026 20:13
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch from dda561b to 644932e Compare April 15, 2026 19:52
@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from 98654e8 to 68438da Compare April 15, 2026 19:52
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch from 644932e to bd1f665 Compare April 15, 2026 20:15
@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from 68438da to 99815bc Compare April 15, 2026 20:15
@jwiegley jwiegley marked this pull request as ready for review April 15, 2026 21:46
@jwiegley jwiegley requested a review from svarlamov April 15, 2026 21:47

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from 99815bc to b1dbda4 Compare April 15, 2026 22:33
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch from bd1f665 to 570a9a0 Compare April 15, 2026 22:33
@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from b1dbda4 to c51217f Compare April 16, 2026 06:42
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch from 570a9a0 to 51f3b40 Compare April 16, 2026 06:43
@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from c51217f to a0ba1b3 Compare April 16, 2026 06:49
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch 2 times, most recently from 9d2b7d0 to f5e6530 Compare April 16, 2026 20:15
@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch 2 times, most recently from 6a82d28 to b99641f Compare April 22, 2026 18:07
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch 2 times, most recently from 7d62af7 to fd7b5db Compare April 23, 2026 05:39

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread AGENTS.md
Comment on lines +280 to +285
file.assert_lines_and_blame(lines![
"fn main() {".human(),
" // human code".human(),
" println!(\"AI\");".ai(),
"}".human(),
]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Documentation example uses .human() assertions for lines captured by an untracked (checkpoint --) checkpoint

The new code example at lines 270-285 uses checkpoint -- code.rs (which creates a legacy/untracked CheckpointKind::Human checkpoint per src/commands/git_ai_handlers.rs:1155) but then asserts .human() (known human) attribution for those lines. According to the conventions established earlier in this same file (AGENTS.md:52), checkpoint -- corresponds to untracked/unattributed changes, while .human() represents known-human attribution (which requires checkpoint mock_known_human). The correct assertions should use .unattributed_human() instead of .human(), or the checkpoint call should be changed to mock_known_human. The existing example at AGENTS.md:108-197 correctly uses checkpoint mock_known_human.human() and checkpoint human.unattributed_human(). This won't cause test failures since assert_lines_and_blame at tests/integration/repos/test_file.rs:316 treats Human and UnattributedHuman identically, but it will mislead agents/developers into writing tests with incorrect semantic expectations.

Prompt for agents
The documentation example at AGENTS.md lines 266-286 has an inconsistency between the checkpoint type used and the attribution assertions. The example uses `checkpoint -- code.rs` (which creates a legacy/untracked Human checkpoint) but then asserts `.human()` for those lines. There are two valid fixes:

1. Change the checkpoint in step 1 (line 271) from `checkpoint -- code.rs` to `checkpoint mock_known_human -- code.rs` so that lines are truly attributed as known human, matching the `.human()` assertions.

2. Alternatively, keep the `checkpoint --` call but change the assertions at lines 281-284 from `.human()` to `.unattributed_human()` for the non-AI lines.

Option 1 is preferable because the section title says "Write human content" and the intent is clearly to show known-human attribution. This would also be consistent with the existing example at AGENTS.md lines 108-197 which correctly uses `mock_known_human` for human-attributed content.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from 3d146e5 to dda323b Compare April 28, 2026 14:03
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch from fd7b5db to 9822853 Compare April 28, 2026 14:03
Document patterns and conventions for writing integration tests
that exercise the attribution system, including test harness setup,
daemon interaction, and assertion helpers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jwiegley jwiegley force-pushed the johnw/review-attribution-test-guidelines branch from 9822853 to f971342 Compare May 1, 2026 17:06
@jwiegley jwiegley force-pushed the johnw/review-integration-tests branch from dda323b to f7384f2 Compare May 1, 2026 17:06
@svarlamov

Copy link
Copy Markdown
Member

Sorry I'm so late to reviewing this. I ended up writing up a lot re tests in agentsmd that's on main already

@svarlamov svarlamov closed this May 5, 2026
@jwiegley jwiegley deleted the johnw/review-attribution-test-guidelines branch June 17, 2026 19:30
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.

2 participants