Add dd-apm-sdk-review skill and repo overrides, replace perf-review - #12364
Add dd-apm-sdk-review skill and repo overrides, replace perf-review#12364robertomonteromiguel wants to merge 7 commits 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.
🟢 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. |
* ci: migrate dd-apm-sdk-review evals to llm-validation gate Replace the promptfoo suite with .llm-validation cases, wire the reusable GitLab job, and document local Docker runs against the published llmval image.
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
There was a problem hiding this comment.
The review skill prints committed diffs before it scans them for secrets. A committed secret can enter the transcript and saved logs.
🤖 Datadog Autotest · Commit 02ec6ec · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02ec6ec22e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
LLM ValidationLLM Validation Gate — dd-apm-sdk-review✅ PASS
AnalysisChanged instruction file(s): 1 case(s) dipped but stayed within the pass bar.
Results
Cases
Per-dimension scores, token usage, latency, and estimated cost are in the CI job logs. |
…kill (#12428) * Tell no-harness reviewers to load the skill lenses GitHub Codex cannot invoke the skill; point it at the reviewer files and a temporary debug prefix so we can see which lens it used. Co-authored-by: Cursor <cursoragent@cursor.com> * Drop Codex debug prefix and bait class Keep the no-harness lens-loading fallback; remove the experiment-only review prefix and unused shouldSample helper. Co-authored-by: Cursor <cursoragent@cursor.com> * Move no-harness review spec out of AGENTS.md Keep the Codex fallback as a pointer; the lens list lives in the overrides file. Co-authored-by: Cursor <cursoragent@cursor.com> * Restore the inline no-harness review spec in AGENTS.md Keep the Codex fallback that already worked; drop the extracted overrides file. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
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/— 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
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.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]