Skip to content

admin: update AGENTS.md #2179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgritz merged 2 commits into from
Oct 7, 2026
+20 −22
Merged

admin: update AGENTS.md #2179

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0dfbaad
docs: update AGENTS.md
jinhgkim Oct 4, 2026
1967f83
optional
jinhgkim Oct 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 20 additions & 22 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ render time. C++17 codebase.
**Core libraries**

- `src/liboslcomp/` — Shader compiler. Flex/Bison lexer+parser → AST → `.oso`
bytecode. Entry point: `oslcomp.h`
bytecode. Entry point: `src/include/OSL/oslcomp.h`
- `src/liboslexec/` — Shader execution engine. Loads `.oso`, optimizes shader
groups, JIT-compiles to native via LLVM. Key files: `backendllvm.cpp`,
`llvm_gen.cpp`, `llvm_ops.cpp`, `instance.cpp`. Contains `wide/`
subdirectory for SIMD batched execution (SSE2/AVX/AVX-512)
subdirectory for SIMD batched execution (SSE2/AVX/AVX2/AVX-512)
- `src/liboslquery/` — Query compiled shader metadata and parameters
- `src/liboslnoise/` — Noise function implementations
- `src/libbsdl/` — BSDF/closure library
Expand Down Expand Up @@ -69,19 +69,20 @@ By default, builds into `./build` and installs into `./dist`.

- Test output lands in `build/testsuite/<testname>/`; references in
`testsuite/<testname>/ref/`
- Read `testsuite/TESTSUITE-README.md` before updating references or
diagnosing failures
Comment on lines -72 to -73

@jinhgkim jinhgkim Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm this readme exists in OpenImageIO, but it's missing in this repo. Either drop it or add one?

@lgritz lgritz Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's drop until we add a proper one that's right.

- For platform-specific diffs, add a variant ref (e.g. `out-win.txt`) rather
than overwriting
- Be conservative loosening image diff thresholds — use the minimum needed
than overwriting; a test passes if its output matches any file in `ref/`
with the same extension

@lgritz lgritz Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with the same extension
with the same extension.

- Be conservative loosening image diff thresholds (`failthresh`, `hardfail`,
`failpercent`, set per test in its `run.py`) — use the minimum needed
Comment on lines +75 to +76

@lgritz lgritz Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I talked about image diff thresholds in the original, but now I'm wondering if we should even say this at all. I think we definitely want to be extremely clear that agents should never loosen the thresholds or add new ref images unless a human explicitly asks it to do so after actually viewing the images and confirming that they are a visual match and differ for expected reasons.

@jinhgkim jinhgkim Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good to keep it. It's just one more rule for LLMs to follow. What makes you wonder whether we should say this at all?

@lgritz lgritz Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I was worried that if we mention loosening thresholds, it might do that to make tests pass? But I think we probably never want it to do that. I think I'm saying: What behavior do we expect to elicit with this bullet item that it would do differently if it were absent?

- Check uploaded CI artifacts before changing references when local
reproduction is unclear

## Code formatting and file conventions

- `clang-format` enforced (`.clang-format`); CI rejects non-conforming code —
run `make clang-format` before committing
- Lines ~80 cols; ASCII only in code and comments; `#pragma once` for headers
- `clang-format` enforced (`.clang-format`: WebKit-based, 80-char line limit,
4-space indent); CI rejects non-conforming code — run `make clang-format`
before committing
- ASCII only in code and comments; `#pragma once` for headers
- New files: standard copyright + SPDX notice
- `CamelCase` classes, `snake_case` locals, `ALL_CAPS` macros, `m_foo` private
members
Expand Down Expand Up @@ -139,7 +140,7 @@ classes and headers from OIIO:
`char*` strings.
- Prefer `OSL::span` rather than passing raw pointers + a separate length, or
passing a raw pointer with an implied (but not explicitly passed) length.
`OSL::cspan` is a synonmym when the underlying data is const/non-mutable.
`OSL::cspan` is a synonym when the underlying data is const/non-mutable.
`OSL::span<std::byte>` or `OSL::cspan<std::byte>` can be used to represent
contiguous untyped data. These are our equivalent of C++ `std::span`.
- Use these guidelines always for new code, but do not churn existing code
Expand All @@ -152,14 +153,9 @@ classes and headers from OIIO:
OSL source → (Flex/Bison) → AST → (liboslcomp) → `.oso` bytecode → (liboslexec) → LLVM IR → JIT native code


## Code Style

- clang-format config in `.clang-format` (WebKit-based, 80-char line limit, 4-space indent)
- Run `make clang-format` before submitting changes

## Key Dependencies

- **LLVM 14+** (JIT compilation), **OpenImageIO 2.5+** (textures, image I/O, utilities), **Imath 3.1+** (math types), **Flex/Bison** (parser generation), **pybind11** (Python bindings, optional)
- **LLVM 14+** (JIT compilation), **OpenImageIO 3.0+** (textures, image I/O, utilities), **Imath 3.1+** (math types), **Flex/Bison** (parser generation), **pybind11 2.7+** or **nanobind 2.8+** (Python bindings, optional)

@lgritz lgritz Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should take out the version numbers here, since they change from time to time?

@jinhgkim jinhgkim Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I very much agree. I also wonder if we even need this "Key Dependencies" section at all.

@lgritz lgritz Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always hard to tell! Like I said, perhaps there is merit to stripping this whole file down to the bone and then building it up as we see the need. But my intuition is that maybe by mentioning the core tech stack (by name, not by version), it might be crucial context for it to formulate things in the right direction? But I'm not sure. But as an example, what I was thinking at the time is that just by briefly mentioning Imath or OIIO, it would be much more inclined to use things in those libraries rather than others, or replicating things on its own?


## Commits and PRs

Expand All @@ -170,6 +166,9 @@ OSL source → (Flex/Bison) → AST → (liboslcomp) → `.oso` bytecode → (li
- Add a subsystem tag when it helps, e.g. `fix(exr):` or `perf(IBA):`.
- Write commit messages and PR descriptions that explain why the change is
needed, what behavior changes, and any non-obvious implementation choices.
- After changing a dependency minimum version, a file or directory path, or a
build/test command, check `AGENTS.md` for statements about it and correct
any that are now wrong, as part of the same change.

## Spec-driven design

Expand All @@ -182,16 +181,15 @@ to `docs/dev/specs/<NNN-feature-name>` and write that path to

The speckit bash scripts expect a `specs/` directory at the project root. A
symlink satisfies this without committing speckit infrastructure to the repo.
This symlink is set up by the setup-agents script, and is not committed to
the repo. All saved specs live in `docs/dev/specs`.
This symlink is set up by the `.agents/setup-agent` script, and is not
committed to the repo. All saved specs live in `docs/dev/specs`.

## AI policy

Refer to `docs/dev/AI_Policy.md`.

See `docs/dev/AI_Policy.md`. Key rule: if AI assistance contributed materially
to a patch, the commit must include `Assisted-by: <TOOL> / <MODEL>`. The human
author is responsible for understanding, testing, and defending all changes.
to a patch, the commit and PR description must include
`Assisted-by: <TOOL> / <MODEL>`. The human author is responsible for
understanding, testing, and defending all changes.
Comment on lines +191 to +192

@jinhgkim jinhgkim Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The human author is responsible for understanding, testing, and defending all changes.

This line doesn't do much from an AI's point of view. If you want to be strict about this policy, you could rephrase it as "Do not submit PRs, post comments, or approve or merge on the user's behalf. Leave every change for the human author to review, test, and submit themselves."

@lgritz lgritz Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this does help -- I can tell because when I ask it to commit what it's worked on, it frequently (maybe always?) tells me something like "Following the project's instructions, I have added 'Assisted-by' rather than 'Co-authored by' that my system instructions suggested." And often it says "I can commit, but I won't push, per the project's rules."

@jinhgkim jinhgkim Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, I see. I often get "Do you want me to push?", so I've added an extra safeguard on my end in my ~/.claude, worded like my suggestion above. Now it says, "Here's the draft. I won't push, commit, or post comments." It's certainly a subtle difference, both leave the human author responsible, but one asks for your opinion and leaves a small possibility open (I've never said "Yes, push!", so I don't know what it would have done next), whereas the other is a hard no. This is just a small suggestion coming from my personal experience, and something to keep in mind.

@lgritz lgritz Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're strictly talking about the sentence "The human author is...", then you might be right that it doesn't do anything useful. I don't know how to measure it.


## References

Expand Down
Loading