Skip to content

docs(agents): rewrite AGENTS.md around CI traps and reviewer feedback - #5177

Draft
kakkoyun wants to merge 2 commits into
mainfrom
chore/agents-md-rewrite
Draft

docs(agents): rewrite AGENTS.md around CI traps and reviewer feedback#5177
kakkoyun wants to merge 2 commits into
mainfrom
chore/agents-md-rewrite

Conversation

@kakkoyun

@kakkoyun kakkoyun commented Aug 10, 2026

Copy link
Copy Markdown
Member

WIP

What does this PR do?

Rewrites the root AGENTS.md from 29 lines of documentation policy into 146 lines describing what CI actually rejects and what reviewers actually send back.

Motivation

The old file said to read CONTRIBUTING.md, listed the five subdirectory AGENTS.md files, and gave criteria for updating the docs. All of that is still here. What was missing was any operational detail, so an agent had to learn the repository's constraints from CI failures.

Two sources fed the new content.

The multi-module dependency workflow, which is the largest source of avoidable churn. Bumping a version with go get and per-module go mod tidy leaves go.work.sum and indirect-only modules stale. go work sync goes the other way and rewrites unrelated dependencies across the workspace. Neither is a substitute for make fix-modules. The supported-configurations allowlist has a similar shape: configinverter add writes FIX_ME placeholders that have to be filled in by hand, the type field takes a fixed vocabulary, and re-serialising the JSON by any other means drops the top-level "version" field and reorders every entry.

Human review comments on the last 100 merged PRs (#4969#5147; 286 inline comments and 48 review bodies once bot reviewers are excluded). The themes that recur across several PRs and several reviewers:

  • assertions that pin a value rather than a shape, and tests that drive the public entry point instead of reaching through reflect
  • comments that still read correctly once the pull request is closed
  • restraint about new exported surface, which reviewers answered with "overkill" five times across four PRs
  • benchmarks for changes that add work to a hot path
  • fmt.Errorf unless a caller has to match the error with errors.Is/errors.As

Guidance with no support in that corpus was dropped rather than carried forward: conventional-commit PR titles (0 mentions in 334 comments), strings.Builder over fmt.Sprintf (1), cross-compilation (2).

Verification

Every command, script path and child AGENTS.md link in the file resolves against the tree. The type vocabulary was taken from the committed allowlist rather than from memory.

The file was also compared against the current AGENTS.md empirically: reconstructions of 13 already-merged PRs, each run twice in isolated single-branch clones at the PR's parent commit — once with each version of the file, nothing else different — and scored against the merged diff. The merged commit is absent from the clones, so it could not be consulted.

Across two independent PR sets: 5 wins, 0 losses, 7 ties, with no-op control tasks tying in every round. One further cell was discarded because the agent never opened AGENTS.md, which makes it a comparison of nothing against nothing. The differences that showed up more than once were dependency propagation, the allowlist type vocabulary, and use of configinverter instead of hand-editing the generated file.

Two limitations worth stating. The reviewer-derived rules about agent-authored comment scaffolding and reflect-based tests never triggered in any cell, so they are grounded in real review comments but unproven here. And make lint was never run by any cell in either arm, which suggests that if it needs enforcing it belongs in a hook rather than in prose.

kakkoyun and others added 2 commits August 10, 2026 15:19
The previous file was documentation policy: when to update CONTRIBUTING.md
and README.md, plus links to the subdirectory AGENTS.md files. It carried no
operational detail, so agents working here learned the repo's constraints by
tripping over them in CI.

This covers what the code cannot tell you. The multi-module dependency
workflow is the largest source of avoidable churn: walking modules by hand
leaves go.work.sum and indirect-only modules stale, while go work sync
rewrites unrelated dependencies. The supported-configurations allowlist has
the same shape, where the tool writes FIX_ME placeholders that must be filled
in by hand and the generated file must never be edited directly.

The remainder is distilled from human review comments on the last 100 merged
pull requests, covering the feedback reviewers repeat: assertions that pin a
value rather than a shape, comments that still read correctly once the pull
request is gone, and restraint about new exported surface. Guidance with no
support in that corpus was dropped rather than carried over.
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Aug 10, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 64.71% (+12.45%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 18708fc | Docs | Datadog PR Page | Give us feedback!

@pr-commenter

pr-commenter Bot commented Aug 10, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-08-10 13:58:29

Comparing candidate commit 18708fc in PR branch chore/agents-md-rewrite with baseline commit 60002e8 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 326 metrics, 0 unstable metrics, 1 flaky benchmarks without significant changes.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Known flaky benchmarks

These benchmarks are marked as flaky and will not trigger a failure. Modify FLAKY_BENCHMARKS_REGEX to control which benchmarks are marked as flaky.

Known flaky benchmarks without significant changes:

  • scenario:BenchmarkOTLPTraceWriterFlush

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.

1 participant