Added AGENTS.md instructions and SKILLS.md#1921
Conversation
A global general instruction and one per project JAVA-6143
rozza
changed the title
|
Followed general guidelines from: https://wiki.corp.mongodb.com/spaces/MMS/pages/499158370/Making+a+Repo+Agent+Ready AGENTS.md: The ConstitutionAGENTS.md is a markdown file at the root of the repository. Every AI agent — Augment, Claude Code, Cursor — reads it automatically at the start of every session. This means every token in AGENTS.md is loaded into the agent's context window every single time. The context window has a budget. Frontier models reliably follow ~150-200 instructions, and tool system prompts consume a significant share before your AGENTS.md even loads. Overstuffing AGENTS.md degrades the quality of all instructions — not selectively. Brevity is load-bearing. What belongs in AGENTS.mdCore development principles (style, testing, safety rules) What does NOT belong in AGENTS.mdDetailed workflows for specific tasks (use a skill) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 53 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
rozza
requested review from
katcharov and
strogiyotec
and removed request for
strogiyotec
|
We had a bit of discussion about this in the AI/ML-sync meeting yesterday. My approach has been to use very minimal agents files (for Junie, Augie, and Claude) because I don't think it is clear what is:
I think we've all seen harmful changes--for example, telling it about tests and then having the agent get caught up attempting to run them all the time. Things that are not harmful but also not very useful (the benign category) can end up being accidentally harmful by taking up context space and thereby forcing something important out. We can discuss these things on a case-by-case basis, but I think that means starting small and understanding the impacts of the changes made. Another approach is for each platform team to do something different and then we all compare notes later. |
katcharov
left a comment
katcharov
left a comment
There was a problem hiding this comment.
Partial review. Agree with @ajcvickers points above. There is a fair bit of content here, and I am not sure how we can validate it.
There was a problem hiding this comment.
Could you run Claude to review this PR, asking something like "Confirm that this PR follows best practices for creating CLAUDE.md files. Evaluate it personally, confirming that various items are generally necessary. Also evaluate it based on official and authoritative sources (official Claude documentation, online posts that are corroborated AND experimentally verified, and posts from recognized experts; but not: blog posts, social media speculations, and other uncorroborated sources. Is anything unnecessary, and easily discoverable? Is anything crucial missing?"
Independently, have it doublecheck correctness until it stops finding issues.
Please also include info about how these files were generated, and AI-reviewed, including results of the above. (This is just the typical AI usage, effectiveness comment in the description).
There was a problem hiding this comment.
| - **Information hiding:** Bury complexity behind simple interfaces. | ||
| - **Pull complexity downward:** Make the implementer work harder so callers work less. | ||
| - **General-purpose over special-case:** Fewer flexible methods over many specialized ones. | ||
| - **Define errors out of existence:** Design APIs so errors cannot happen rather than detecting and handling them. |
There was a problem hiding this comment.
I am not sure how to interpret this or check if a model has actually applied these principles. For example, the first one, "deep modules", seems to be a matter of intuition. In practice, I'm not sure how we would choose one approach vs another, or what would make a given approach have a "powerful implementation".
| ## API Stability Annotations | ||
|
|
||
| - `@Alpha` — Early development, may be removed. | ||
| Not for production use. | ||
| - `@Beta` — Subject to change or removal. | ||
| Libraries should not depend on these. | ||
| - `@Evolving` — May add abstract methods in future releases. | ||
| Safe to use, but implementing/extending bears upgrade risk. | ||
| - `@Sealed` — Must not be extended or implemented by consumers. | ||
| Safe to use, but not to subclass. | ||
| - `@Deprecated` — Supported until next major release but should be migrated away from. |
There was a problem hiding this comment.
I don't think a model should ever be applying some of these. I also don't know what "Libraries should not depend on these" means? Our users can depend on Beta, but we can change it.
There was a problem hiding this comment.
You can ask the model to apply / create / update methods to add / remove these etc.. these. This is just a skill regarding their usage.
There was a problem hiding this comment.
I slightly modified the text for clarity.
.agents/skills/api-design/SKILL.md
Outdated
| ## Public API Rules | ||
|
|
||
| - Breaking changes require a major version bump - ALWAYS warn if breaking binary compatibility | ||
| - All `com.mongodb.internal.*` / `org.bson.internal.*` is private API — never expose in public APIs |
There was a problem hiding this comment.
I don't understand how a model would expose these as public APIs?
| @@ -0,0 +1,45 @@ | |||
| --- | |||
| name: api-design | |||
There was a problem hiding this comment.
I ran only this skill through Claude, and I got this:
What's already known to models:
- The design principles section is essentially a summary of Ousterhout's A
Philosophy of Software Design. Any capable model already knows these
concepts and would apply them naturally. This section adds little value.
- "Search before implementing" is generic good practice, not
project-specific guidance.
What's genuinely useful (project-specific):
- The stability annotations (@Alpha, @Beta, @Evolving, @Sealed) — these are
driver-specific and not inferable without reading the source.
- The com.mongodb.internal.* / org.bson.internal.* rule.
- The concrete pattern examples (Filters.eq(),
MongoClientSettings.builder()).
- The package-info.java requirement.
What's missing that would actually help:
- How to choose which annotation for new API — when should something be
@Alpha vs @Beta vs @Evolving? What's the promotion path?
- Sync/async API mirroring — does every sync method need an async
counterpart? How are they kept in sync?
- Nullability conventions — does the driver use @Nullable/@NonNull? What's
the stance on null parameters?
- The module structure — what goes in driver-core vs driver-sync vs
driver-reactive-streams? Where should new API land?
- Default methods on interfaces — the strategy for evolving @Evolving
interfaces without breaking implementors.
- Concrete examples of "wrong" API decisions — models learn more from
"don't do X because Y happened" than from abstract principles.
Bottom line: The skill is ~30% useful as-is. The design principles section
could be dropped entirely without loss. The project-specific rules are
valuable but thin. The biggest improvement would be replacing the generic
principles with concrete, driver-specific guidance on the missing items
above — things a model can't infer from first principles.
I am uncertain that what is marked "useful" above will be in practice (see other comments).
bom/CLAUDE.md
Outdated
| @@ -0,0 +1 @@ | |||
| @AGENTS.md | |||
There was a problem hiding this comment.
What is this pattern based on?
I tried to look up how it works, and info is scarce. As of 9 months ago, this convention did not imply that the file would be embedded in the other file. See comments in this post, someone experimented and confirmed that Claude sometimes only conditionally reads these files. I believe this means that:
- We don't actually have a guarantee that a referenced file will be read, and the conditions under which it is read are unclear
- Updating the referenced file while a session is in-flight will not cause an immediate context update, which makes tuning difficult
- There will be an additional step (time, tokens) for reading this file
I don't think we should use this pattern if the above is true.
There was a problem hiding this comment.
The internal guidelines as linked in the ticket description: https://wiki.corp.mongodb.com/spaces/MMS/pages/499158370/Making+a+Repo+Agent+Ready
The cloud team have used this pattern to great effect and it has allowed them to be model agnostic.
There was a problem hiding this comment.
Turns out both sources are out of date. AGENTS.md is all thats needed for claude now and will automatically imported. CLAUDE.md can contain claude specific logic above AGENTS.md.
Confirmed with Claude and read about it here: https://medium.com/data-science-collective/the-complete-guide-to-ai-agent-memory-files-claude-md-agents-md-and-beyond-49ea0df5c5a9 .
Turns out you need the @ pattern: https://code.claude.com/docs/en/memory#agents-md but can add overrides in your local CLAUDE.md files after the import.
|
@ajcvickers thanks for the feedback. Out of interest were the MMS team in the AI/ML-sync meeting? The formating of this PR and use of skills over a large Claude.md file came via Glean indiciating the Cloud teams approach : https://wiki.corp.mongodb.com/spaces/MMS/pages/499158370/Making+a+Repo+Agent+Ready They seem to be a few months ahead with their adoption and usage of these tools. @katcharov know anyone on their team that could review this. |
|
So I think before going further, an ideological approach has to be agreed upon. Do we add skills at all initially? We could .gitignore the skills directory and then manually add them on a one by one basis in other PR's? That approach is handy and would allow users to have test their own skills without risk of accidently adding to the repo. Note the request for local skills anthropics/claude-code#31155 has been closed so there is no local repo skills pattern other than via .gitignore. We can start small - just the AGENTS.md files explaining the source code and how to compile and run the tests. This is useful to stop claude trying to use sbt instead of gradle everytime it tries to do anything in scala.
|
|
I made some changes after iterating with Claude. Full report of the final outcomes: https://gist.github.com/rozza/1a474291735bac0a28e399c59a977db2 |
.agents/skills/api-design/SKILL.md
Outdated
| - **General-purpose over special-case:** Fewer flexible methods over many specialized ones. | ||
| - **Define errors out of existence:** Design APIs so errors cannot happen rather than detecting and handling them. | ||
|
|
||
| ## Search Before Implementing |
There was a problem hiding this comment.
This is covered by the AGENTS.md so was considered a duplication.
.agents/skills/api-design/SKILL.md
Outdated
| - Fluent builders: `MongoClientSettings.builder()` is the primary entry point | ||
| - Abstract core with pluggable transports | ||
|
|
||
| ## Public API Rules |
There was a problem hiding this comment.
Another duplication from the top level AGENTS.md
|
|
||
| - **Static analysis:** CodeNarc | ||
|
|
||
| ## Javadoc / KDoc / Scaladoc |
There was a problem hiding this comment.
Claude identified these as missing
| description: Sync AGENTS.md files and skills after buildSrc or convention changes. Use after modifying build plugins, formatting rules, testing conventions, or task names to keep documentation consistent. | ||
| disable-model-invocation: true | ||
| --- | ||
| # Sync Documentation After Build Changes |
There was a problem hiding this comment.
Claude identified the buildSrc/AGENTS.md was not the correct place for this as it changes code. It also recommended this not be automatically run by claude.
| @@ -0,0 +1,88 @@ | |||
| # Test Skeletons | |||
There was a problem hiding this comment.
Claude recommended having official test skeleton examples to use for new test cases.
bom/AGENTS.md
Outdated
There was a problem hiding this comment.
Claude recommended deleting a few AGENTS.md files as they didnt add value or were for niche projects within the codebase.
bson-kotlin/AGENTS.md
Outdated
| ./gradlew :bson-kotlin:check | ||
| ``` | ||
|
|
||
| For global rules see [root AGENTS.md](../AGENTS.md). |
There was a problem hiding this comment.
These comments were considered unnecessary because the root agent is already in scope.
|
@rozza I think you should continue with this approach and on the C# driver we will start more minimal, and then let's revisit how things are working in a few weeks. |
Initial implementation of AGENTS.md and SKILLS.md for the MongoDB Java Driver Repo
JAVA-6143