RFC: add AGENTS.md - #21924
Conversation
andriiryzhkov
added
the
scope: codebase
|
Good idea and good content already, thanks for that.
I think claude code is the only tool which insists to not support the https://agents.md/ convention, it seems Google, Github Copilot, Windsurf, OpenAI Codex have implemented it. There is even documentation around this proprietary behavior: https://code.claude.com/docs/en/memory#agents-md Hence I suggest to also create a CLAUDE.md file with the only content
Sounds fine, even if those lines will be part of every prompt context.
I don't have a strong opinion on that.
I didn't find an obvious violation of our (unwritten) project pratices, hence it's such a good idea to finally agree on common accepted practices ;) |
3d1c4b3 to
55fb335
Compare
Added. |
|
Thanks @andriiryzhkov, LGTM. To allow for customization, maybe you could add |
@masterpiga - How will agents behave if this file is mentioned but does not exist? Will they process it correctly and not consider it an error? If everything is fine, I wouldn't add an empty file to the repo because it just looks a bit sloppy. |
|
The last line in |
|
@masterpiga : The core idea is that Is similar multi-layer approach something you can adopt? |
|
Sure, I am adaptable. I thought that it would be nice to have a way to make it easier to integrate specific developer instructions, but if it's controversial just forget about it 👍 |
I don't think it is controversial and it is a right place to discuss it. But I never saw such extensions so far and majority of AI platforms have support for instructions on different levels - project, user, etc. I probably need to check better instructions layering on different platforms and I will come back with more educated opinion and maybe suggestions on that topic. |
As I mentioned before, I don't feel strongly about it, and I wouldn't want you to waste your time on it. It's just that AGENTS.md is the only almost-standard way to instruct agents on a specific project. If I want to use different agents for the same project with a different/additional set of instructions (but the same instructions for all), I have to create N separate instruction files (or N symlinks), one for each agent/platform, and then add them one by one to |
|
@masterpiga Several agents do support per-project personal instructions – Claude Code reads No
@victoryforce – no placeholder needed, the wording is conditional on the file existing. One thing I didn't carry over: you asked for "override, amend or complement", and I've written it as amends, with project conventions still governing – otherwise a local file can switch off the commit rules and a reviewer pays for it. If you have a case where overriding matters, please say so. |
AI-assisted contributions arrive with a recurring set of problems the existing docs do not speak to: commit series where only the final commit compiles, which leaves git bisect useless, and patches written by inferring an API instead of reading the page that documents it. The conventions are not new. They are collected from the Developer's guide, dev-doc/ and the existing history into the file name that most coding assistants read by default. Two wiki corrections are folded in: .clang-format and tools/beautify_style.sh no longer exist, having been removed in 46b054c and b734b01. Related: darktable-org#21881
Claude Code reads CLAUDE.md and does not pick up AGENTS.md on its own. The `@AGENTS.md` import keeps the conventions in one file rather than duplicating them, and a one-line pointer works where a symlink would not, such as checkouts with core.symlinks disabled.
Maintainers now hold a PR until RELEASE_NOTES.md carries an entry for it. Record which changes qualify and which do not, so an agent settles the question while writing the PR rather than at review time.
|
Thanks a lot, @andriiryzhkov, LGTM. The only instruction that I override is how to build and run. I have my own branch-aware local scripts for that, but it's no biggie. |
45a9b50 to
0c37494
Compare
We can add in build section to look to developer's instructions for building and running specific to his setup. Updated |
Per-project personal instructions have no cross-platform home. Agents offer a global user layer and a committed project layer, but nothing scoped to a single checkout, so carrying the same personal rules across several agents means one file per agent, each excluded by hand. AGENTS.md now points at an optional AGENTS.local.md, written as prose rather than an import directive so that agents which do not expand `@` still act on it, and .gitignore carries the file so nobody has to edit their own exclude list.
0c37494 to
3ad39de
Compare
|
@andriiryzhkov A couple of additions that I have in my AGENTS.md and that are not currently covered in this one (wording should be improved so that it is more aligned with the current style):
And in the "Comments" section:
Both are based on actual review comments from previous PRs. Actually, it could make sense to collect a rubric of "do"s and "don't"s by asking an agent to go through a bunch of recent PRs and distill general principles from reviewer comments. I am not requesting any change, I am just thinking out loud :) Thanks a lot for your efforts to tidy up agentic work in the repo 👍 |
|
Right now I'm reading an article that concludes: "Codebase overviews are useless" in AGENTS.md. Also: "Less content, better results". I'll try to understand more and post a link to the research here soon. |
Ok, I'll wait for your feedback, I was about to merge :) |
|
I'm definitely not an expert on this topic (quite the opposite, a newbie), but in trying to figure out how to instruct agents I came across several interesting articles and studies:
I'll quote a few important conclusions from what I read right here: The findings challenged the assumption that more context is always better:
|
|
Reduced instructions size. It should look better now. I'd propose to move on and merge this. I am sure this is not the final version, but we need more testing and feedback. Especially with different AI platforms. |
Remove unused quality setting dropdown borders: fix memory corruption on non-positive coordinate deltas Commit 257e632 widened the loop index variable in set_pixels() and copy_pixels() from int to size_t. In dt_iop_copy_image_with_border(), these functions are called with slice lengths computed from coordinate differences such as fl_right - image_right. Under boundary conditions (e.g. scaled thumbnail generation or tiling), the difference can be negative. With a signed int counter, 0 < npixels evaluated to false and safely skipped the loop. With size_t, signed-to-unsigned conversion caused the loop to iterate 18446744073709551615 times, corrupting heap memory and causing a SIGSEGV. Revert the loop variables back to int, and clamp image_right and image_bottom to roi_out bounds in dt_iop_setup_binfo. Keep a path's feather when the shape is resized Scroll-resizing a path morphs a cached baseline of its nodes and swaps the result in whole, borders included. The feather branch of the same scroll handler edits every node's border in place without dropping that cache, so the next resize restored the borders the baseline was captured with and the feather collapsed. Invalidate the resize state there, as _path_modify_property already does for DT_MASKS_PROPERTY_FEATHER. Reproduced by resizing a path with the wheel, raising the feather with shift+scroll, then resizing again; circles and ellipses are unaffected as they have no baseline to go stale. Fixes darktable-org#22052 RELEASE_NOTES.md: path feather lost on scroll resize Add protection against memory allocation failure remove mention of Intel Macs from README (darktable-org#22114) * remove mention of Intel Macs from README * update README for the nightly builds RELEASE_NOTES.md: clean-up the changed dependencies. borders: disable frame line controls when border geometry prevents it The frame line width is proportional to the minimum border width across all four sides. When border size is 0 or when horizontal/vertical offsets are set to extreme values (0.0 or 1.0), the minimum border width collapses to 0, making the frame line invisible. Disable frame line width, offset, and color controls when geometry prevents the frame line from rendering, and update their tooltips to explain the required adjustment. borders: fix asymmetric frame line centering and rounding precision Frame line boundary coordinates previously subtracted 1 to represent inclusive pixel indices, but were assigned directly to half-open interval endpoints (fl_right, border_right, fl_bot, border_bot). This caused the inner border on the right and bottom sides to be 1 pixel narrower and the outer border 1 pixel wider, noticeably shifting the frame line off-center at smaller border sizes and downscaled preview resolutions. Define the bottom-right frame coordinates as exclusive upper bounds, matching CPU slice lengths and OpenCL bounding box expectations, and use roundf() on fractional offset and size products. src/tests/integration: Update sub-module. Update expected output after the border precision fixes. RFC: add AGENTS.md (darktable-org#21924) * Add AGENTS.md with conventions for AI coding agents AI-assisted contributions arrive with a recurring set of problems the existing docs do not speak to: commit series where only the final commit compiles, which leaves git bisect useless, and patches written by inferring an API instead of reading the page that documents it. The conventions are not new. They are collected from the Developer's guide, dev-doc/ and the existing history into the file name that most coding assistants read by default. Two wiki corrections are folded in: .clang-format and tools/beautify_style.sh no longer exist, having been removed in 46b054c and b734b01. Related: darktable-org#21881 * Add CLAUDE.md importing AGENTS.md Claude Code reads CLAUDE.md and does not pick up AGENTS.md on its own. The `@AGENTS.md` import keeps the conventions in one file rather than duplicating them, and a one-line pointer works where a symlink would not, such as checkouts with core.symlinks disabled. * AGENTS.md: require a release notes entry with each PR Maintainers now hold a PR until RELEASE_NOTES.md carries an entry for it. Record which changes qualify and which do not, so an agent settles the question while writing the PR rather than at review time. * Support personal agent instructions in AGENTS.local.md Per-project personal instructions have no cross-platform home. Agents offer a global user layer and a committed project layer, but nothing scoped to a single checkout, so carrying the same personal rules across several agents means one file per agent, each excluded by hand. AGENTS.md now points at an optional AGENTS.local.md, written as prose rather than an import directive so that agents which do not expand `@` still act on it, and .gitignore carries the file so nobody has to edit their own exclude list. gui: prevent resize handles from starting graph drags Resizable drawing areas share their widget with the resize wrapper. A press on the resize handle could therefore start the graph gesture as well and change interactive graph parameters while resizing.\n\nRun the resize click and motion controllers in the capture phase. Handle presses are claimed before graph gestures, while presses outside the handle are denied so normal graph interaction remains available.\n\nFixes darktable-org#22106 gui: refresh resize indicators after child motion preview_data: guard the pipe-nodes walk in dt_preview_data_is_fresh with busy_mutex dt_preview_data_is_fresh() (introduced in 8a26f35 / darktable-org#21397, the shared preview-data service for interactive editing) walks dev->preview_pipe->nodes and hashes the matching piece from the GUI thread while holding only the module's own gui_lock. pipe->nodes is actually protected by pipe->busy_mutex: dt_dev_pixelpipe_cleanup_nodes() frees every piece under that lock whenever a history change rebuilds the pipe topology (DT_DEV_PIPE_REMOVE in dt_dev_pixelpipe_change), and that can run concurrently on the pipe-processing thread. A caller that walks pipe->nodes off the GUI thread without busy_mutex can therefore dereference a piece that gets freed out from under it -- reproduced as a segfault dereferencing a freed piece->module in _dev_pixelpipe_cache_basichash(), from a module's preview-pipe-finished GUI callback calling dt_preview_data_is_fresh(). colorequal.c's own interactive hue-editing mode (also from darktable-org#21397) calls dt_preview_data_is_fresh() from mouse-hover/scroll handlers and is exposed to the same race in principle, just from a narrower window that makes it harder to trigger in practice. Take pipe->busy_mutex around the walk. It must be a trylock, not a blocking lock: a module's process() runs under busy_mutex for the whole pipe run (dt_dev_pixelpipe_process) and can itself call dt_preview_data_store() (colorequal.c does, twice), which takes this same module's gui_lock -- the opposite order -- so a blocking lock here would AB-BA deadlock against it. A pipe that's currently busy can't have fresh data for us anyway, so treating "busy" as "not fresh" costs nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WpGKm49b4w1L51YETXyGpU RELEASE_NOTES.md: ColorEQ interactive editing mode. Introduce `dt_history_hash_unset_mipmap()` Sets the image mipmap hash to zero, used to invalidate that mipmap. Invalidate watermark cachelines for tag/metadata change As reported a metadata or tag change leaves an invalid cacheline, the suggested fix is good and the performance penalty is neglectable as those changes are rare and the module has a high iop_order. Also ensure recalculation of lighttable mipmaps. While being in lighttable view we don't have the iop_order ready so we simply clear the darkroom pipe caches for the next run. Fixes darktable-org#22069 Bump libheif version to the latest for AppImage build
Remove unused quality setting dropdown borders: fix memory corruption on non-positive coordinate deltas Commit 257e632 widened the loop index variable in set_pixels() and copy_pixels() from int to size_t. In dt_iop_copy_image_with_border(), these functions are called with slice lengths computed from coordinate differences such as fl_right - image_right. Under boundary conditions (e.g. scaled thumbnail generation or tiling), the difference can be negative. With a signed int counter, 0 < npixels evaluated to false and safely skipped the loop. With size_t, signed-to-unsigned conversion caused the loop to iterate 18446744073709551615 times, corrupting heap memory and causing a SIGSEGV. Revert the loop variables back to int, and clamp image_right and image_bottom to roi_out bounds in dt_iop_setup_binfo. Keep a path's feather when the shape is resized Scroll-resizing a path morphs a cached baseline of its nodes and swaps the result in whole, borders included. The feather branch of the same scroll handler edits every node's border in place without dropping that cache, so the next resize restored the borders the baseline was captured with and the feather collapsed. Invalidate the resize state there, as _path_modify_property already does for DT_MASKS_PROPERTY_FEATHER. Reproduced by resizing a path with the wheel, raising the feather with shift+scroll, then resizing again; circles and ellipses are unaffected as they have no baseline to go stale. Fixes darktable-org#22052 RELEASE_NOTES.md: path feather lost on scroll resize Add protection against memory allocation failure remove mention of Intel Macs from README (darktable-org#22114) * remove mention of Intel Macs from README * update README for the nightly builds RELEASE_NOTES.md: clean-up the changed dependencies. borders: disable frame line controls when border geometry prevents it The frame line width is proportional to the minimum border width across all four sides. When border size is 0 or when horizontal/vertical offsets are set to extreme values (0.0 or 1.0), the minimum border width collapses to 0, making the frame line invisible. Disable frame line width, offset, and color controls when geometry prevents the frame line from rendering, and update their tooltips to explain the required adjustment. borders: fix asymmetric frame line centering and rounding precision Frame line boundary coordinates previously subtracted 1 to represent inclusive pixel indices, but were assigned directly to half-open interval endpoints (fl_right, border_right, fl_bot, border_bot). This caused the inner border on the right and bottom sides to be 1 pixel narrower and the outer border 1 pixel wider, noticeably shifting the frame line off-center at smaller border sizes and downscaled preview resolutions. Define the bottom-right frame coordinates as exclusive upper bounds, matching CPU slice lengths and OpenCL bounding box expectations, and use roundf() on fractional offset and size products. src/tests/integration: Update sub-module. Update expected output after the border precision fixes. RFC: add AGENTS.md (darktable-org#21924) * Add AGENTS.md with conventions for AI coding agents AI-assisted contributions arrive with a recurring set of problems the existing docs do not speak to: commit series where only the final commit compiles, which leaves git bisect useless, and patches written by inferring an API instead of reading the page that documents it. The conventions are not new. They are collected from the Developer's guide, dev-doc/ and the existing history into the file name that most coding assistants read by default. Two wiki corrections are folded in: .clang-format and tools/beautify_style.sh no longer exist, having been removed in 46b054c and b734b01. Related: darktable-org#21881 * Add CLAUDE.md importing AGENTS.md Claude Code reads CLAUDE.md and does not pick up AGENTS.md on its own. The `@AGENTS.md` import keeps the conventions in one file rather than duplicating them, and a one-line pointer works where a symlink would not, such as checkouts with core.symlinks disabled. * AGENTS.md: require a release notes entry with each PR Maintainers now hold a PR until RELEASE_NOTES.md carries an entry for it. Record which changes qualify and which do not, so an agent settles the question while writing the PR rather than at review time. * Support personal agent instructions in AGENTS.local.md Per-project personal instructions have no cross-platform home. Agents offer a global user layer and a committed project layer, but nothing scoped to a single checkout, so carrying the same personal rules across several agents means one file per agent, each excluded by hand. AGENTS.md now points at an optional AGENTS.local.md, written as prose rather than an import directive so that agents which do not expand `@` still act on it, and .gitignore carries the file so nobody has to edit their own exclude list. gui: prevent resize handles from starting graph drags Resizable drawing areas share their widget with the resize wrapper. A press on the resize handle could therefore start the graph gesture as well and change interactive graph parameters while resizing.\n\nRun the resize click and motion controllers in the capture phase. Handle presses are claimed before graph gestures, while presses outside the handle are denied so normal graph interaction remains available.\n\nFixes darktable-org#22106 gui: refresh resize indicators after child motion preview_data: guard the pipe-nodes walk in dt_preview_data_is_fresh with busy_mutex dt_preview_data_is_fresh() (introduced in 8a26f35 / darktable-org#21397, the shared preview-data service for interactive editing) walks dev->preview_pipe->nodes and hashes the matching piece from the GUI thread while holding only the module's own gui_lock. pipe->nodes is actually protected by pipe->busy_mutex: dt_dev_pixelpipe_cleanup_nodes() frees every piece under that lock whenever a history change rebuilds the pipe topology (DT_DEV_PIPE_REMOVE in dt_dev_pixelpipe_change), and that can run concurrently on the pipe-processing thread. A caller that walks pipe->nodes off the GUI thread without busy_mutex can therefore dereference a piece that gets freed out from under it -- reproduced as a segfault dereferencing a freed piece->module in _dev_pixelpipe_cache_basichash(), from a module's preview-pipe-finished GUI callback calling dt_preview_data_is_fresh(). colorequal.c's own interactive hue-editing mode (also from darktable-org#21397) calls dt_preview_data_is_fresh() from mouse-hover/scroll handlers and is exposed to the same race in principle, just from a narrower window that makes it harder to trigger in practice. Take pipe->busy_mutex around the walk. It must be a trylock, not a blocking lock: a module's process() runs under busy_mutex for the whole pipe run (dt_dev_pixelpipe_process) and can itself call dt_preview_data_store() (colorequal.c does, twice), which takes this same module's gui_lock -- the opposite order -- so a blocking lock here would AB-BA deadlock against it. A pipe that's currently busy can't have fresh data for us anyway, so treating "busy" as "not fresh" costs nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WpGKm49b4w1L51YETXyGpU RELEASE_NOTES.md: ColorEQ interactive editing mode. Introduce `dt_history_hash_unset_mipmap()` Sets the image mipmap hash to zero, used to invalidate that mipmap. Invalidate watermark cachelines for tag/metadata change As reported a metadata or tag change leaves an invalid cacheline, the suggested fix is good and the performance penalty is neglectable as those changes are rare and the module has a high iop_order. Also ensure recalculation of lighttable mipmaps. While being in lighttable view we don't have the iop_order ready so we simply clear the darkroom pipe caches for the next run. Fixes darktable-org#22069 Bump libheif version to the latest for AppImage build
* Add AGENTS.md with conventions for AI coding agents AI-assisted contributions arrive with a recurring set of problems the existing docs do not speak to: commit series where only the final commit compiles, which leaves git bisect useless, and patches written by inferring an API instead of reading the page that documents it. The conventions are not new. They are collected from the Developer's guide, dev-doc/ and the existing history into the file name that most coding assistants read by default. Two wiki corrections are folded in: .clang-format and tools/beautify_style.sh no longer exist, having been removed in 46b054c and b734b01. Related: darktable-org#21881 * Add CLAUDE.md importing AGENTS.md Claude Code reads CLAUDE.md and does not pick up AGENTS.md on its own. The `@AGENTS.md` import keeps the conventions in one file rather than duplicating them, and a one-line pointer works where a symlink would not, such as checkouts with core.symlinks disabled. * AGENTS.md: require a release notes entry with each PR Maintainers now hold a PR until RELEASE_NOTES.md carries an entry for it. Record which changes qualify and which do not, so an agent settles the question while writing the PR rather than at review time. * Support personal agent instructions in AGENTS.local.md Per-project personal instructions have no cross-platform home. Agents offer a global user layer and a committed project layer, but nothing scoped to a single checkout, so carrying the same personal rules across several agents means one file per agent, each excluded by hand. AGENTS.md now points at an optional AGENTS.local.md, written as prose rather than an import directive so that agents which do not expand `@` still act on it, and .gitignore carries the file so nobody has to edit their own exclude list.
5 participants
Related: #21881
This is a first pass at the
AGENTS.mdidea from the bisect RFC. It is meant as a starting point for discussion, not a finished policy – if the consensus is that we don't want this file at all, that's a fine outcome too.Why
We're getting more AI-assisted patches, and they go wrong in the same few ways. The one that hurts most is the bisect problem from #21881: a series where only the last commit builds. The others are familiar – an API inferred from its name instead of read, a claim that something works when it was never run, a diff with half the file reformatted around the actual change.
None of that is addressed anywhere an agent will look. The Developer's guide and
dev-doc/are excellent, but nothing points an assistant at them, andAGENTS.mdin the repo root is the filename most coding tools now read by default. So this is mostly a signpost with the sharp edges called out.What's in it
The rules aren't new – they're collected from the Developer's guide,
dev-doc/and what the history already does. Where I couldn't find a written rule I measured the tree instead of inventing one, which is how the commit-message and comment sections ended up the way they did.Two things in it contradict the wiki, and I think the wiki is wrong:
.clang-formatandtools/beautify_style.shdon't exist any more (removed in 46b054c and b734b01), but the guide still points people at the script.What I'd like feedback on
AGENTS.mdthe right name? Some tools want their own filename; a symlink or a short pointer file could cover those.Fixes #Nin commit messages because the history does it, but if maintainers would rather close issues from the PR only, that should change.Written with AI assistance. Every rule in it was checked against the tree or the history before it went in, but that checking was done by the same assistant, so a second pair of eyes on the specifics would be welcome.