Add dd-apm-sdk-review skill and repo overrides, replace perf-review - #12364
Draft
robertomonteromiguel wants to merge 1 commit into
Draft
Add dd-apm-sdk-review skill and repo overrides, replace perf-review#12364robertomonteromiguel wants to merge 1 commit into
robertomonteromiguel wants to merge 1 commit into
Conversation
Introduces .agents/skills/dd-apm-sdk-review as a multi-perspective PR review skill (SKILL.md plus reviewer lenses: coherence, conventions, correctness, cross-sdk, design, maintainability, performance, security, and a shared report template), replacing the previous perf-review skill. Adds dd-trace-java-specific overrides under .agents/skills/dd-apm-sdk-review-overrides (repo-context.md and per-lens overrides for conventions, design, maintainability, performance, security). Wires up validation: .llm-validation/config.yaml and suites/dd-apm-sdk-review.yaml, plus .promptfoo/dd-apm-sdk-review config and test cases, to exercise the skill's review output. Updates CODEOWNERS, .gitignore, AGENTS.md, and the .claude/skills symlink accordingly.
This comment has been minimized.
This comment has been minimized.
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Adds the
dd-apm-sdk-reviewskill to this repo: an automated, multi-lens code review that runs before a developer pushes, checking a diff for correctness, security, performance, design, maintainability, conventions, coherence, and cross-SDK consistency issues.Concretely:
.agents/skills/dd-apm-sdk-review/— the skill itself (SKILL.md+ one rule file per lens). This is a verbatim copy fromdd-apm-sdk-review-core, the shared source of truth for all tracer repos..agents/dd-apm-sdk-review-overrides/— the dd-trace-java-specific layer: repo context and per-lens overrides that adapt the generic rules to this codebase..llm-validation/and.promptfoo/— validation scaffolding to test the skill's review output against known cases.perf-reviewskill, and updatesCODEOWNERS,.gitignore, andAGENTS.mdaccordingly.This lands as a single, squashed commit for ease of review. The branch history (with intermediate fixes from earlier automated review passes) is preserved at
backup/phase1-agentic-review-rolloutif needed.How To Review This
dd-apm-sdk-review-corePR #1 — that's where the skill's design, rules, and rationale are reviewed in depth..agents/skills/dd-apm-sdk-review/reviewers/*.mdhere — those are an exact copy of the core repo's rules, not authored in this PR..agents/dd-apm-sdk-review-overrides/— this is the part specific to dd-trace-java and worth a real read..llm-validation/and.promptfoo/to confirm the test cases match what you'd expect the skill to catch (see Additional Notes below).Motivation
Today, review feedback on things like resource leaks, TOCTOU security gaps, or serialization bugs only surfaces once a human reviewer catches it — after the PR is already open. This skill runs the same categories of checks locally, before push, so common and recurring issues get caught earlier and human review time is spent on judgment calls rather than patterns we already know how to detect.
Additional Notes
promptfooside by side to decide which one drives validation going forward. Nothing here should be read as a final choice.promptfootest cases from real, historical dd-trace-java bugs. This is a temporary home for those cases — once the testing tool is decided, they'll move to wherever that tool expects them.CI is not wired up yet. There's no job today that automatically runs these tests when
AGENTS.mdor any file under.agents/dd-apm-sdk-review-overrides/changes. That's a follow-up once the testing tool above is decided.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]