Streamline generated AGENTS.md usage rules - #6775
Conversation
Prune redundant Phoenix, Ecto, Elixir, HTML, LiveView, and testing guidance. Consolidate the remaining form, stream, hook, and test rules, correct stream prepend positioning, and include Ecto.Query in generated seed files.
Update the generated rules according to the following principles:
1. Remove information that is now commonplace, such as Tailwind v4 setup.
2. Remove information covering mistakes that already produce compilation errors.
3. Remove negative “do not” guidance in accordance with current model prompting guidelines.
4. Evaluate each rule through an isolated Codex section coordinating Codex and Claude CLI runs with the prompt below:
Evaluate whether these rules are necessary in AGENTS.md by testing both
Codex and `claude -p`.
For each rule:
1. Ask a realistic code-generation question whenever possible.
2. Do not reveal or hint at the expected answer in the question.
3. Run both models from an empty temporary directory without repository
instructions or local context.
4. Critically inspect the generated code:
- Remove the rule if both models follow it unaided.
- Keep the rule if either model misses a material requirement.
- Correct the rule if the models use an equally valid modern approach
or expose an inaccurate detail.
Treat each section as one question unless asked otherwise. If a question
is leading or the result is ambiguous, rewrite it and rerun the test.
Run every question three times through each CLI. Evaluate consistency
across all three runs and provide results only after all runs complete.
Rules to evaluate:
[PASTE RULES HERE]
rhcarvalho
left a comment
There was a problem hiding this comment.
Manually read through most of the changes except some large chunks of removed lines. Sharing my notes before closing the tab ;)
| ## Ecto Guidelines | ||
|
|
||
| - **Always** preload Ecto associations in queries when they'll be accessed in templates, ie a message that needs to reference the `message.user.email` | ||
| - Remember `import Ecto.Query` and other supporting modules when you write `seeds.exs` |
There was a problem hiding this comment.
Good approach to simply include the import in the scaffolding!
| - `Ecto.Schema` fields always use the `:string` type, even for `:text`, columns, ie: `field :name, :string` | ||
| - `Ecto.Changeset.validate_number/2` **DOES NOT SUPPORT the `:allow_nil` option**. By default, Ecto validations only run if a change for the given field exists and the change value is not nil, so such as option is never needed | ||
| - You **must** use `Ecto.Changeset.get_field(changeset, :field)` to access changeset fields | ||
| - Fields which are set programmatically, such as `user_id`, must not be listed in `cast` calls or similar for security purposes. Instead they must be explicitly set when creating the struct |
There was a problem hiding this comment.
Noticed this might be one of those "do not" rules?
There was a problem hiding this comment.
The "do not" rules are mostly to avoid pairing "DO" with "DO NOT"s. In this case, it is a valid DO NOT.
| - **Never** use map access syntax (`changeset[:field]`) on structs as they do not implement the Access behaviour by default. For regular structs, you **must** access the fields directly, such as `my_struct.field` or use higher level APIs that are available on the struct if they exist, `Ecto.Changeset.get_field/2` for changesets | ||
| - Elixir's standard library has everything necessary for date and time manipulation. Familiarize yourself with the common `Time`, `Date`, `DateTime`, and `Calendar` interfaces by accessing their documentation as necessary. **Never** install additional dependencies unless asked or for date/time parsing (which you can use the `date_time_parser` package) | ||
| - Don't use `String.to_atom/1` on user input (memory leak risk) | ||
| - Predicate function names should not start with `is_` and should end in a question mark. Names like `is_thing` should be reserved for guards |
There was a problem hiding this comment.
Are the newer models really good about following this consistently? I would think this is an easy mistake to make with my human brain :)
There was a problem hiding this comment.
Yes, I prompted them two implement a guard and a function, giving only a description of the body, never the names, and it followed it all times.
| - **Always** use the imported `Phoenix.Component.form/1` and `Phoenix.Component.inputs_for/1` function to build forms. **Never** use `Phoenix.HTML.form_for` or `Phoenix.HTML.inputs_for` as they are outdated | ||
| - When building forms **always** use the already imported `Phoenix.Component.to_form/2` (`assign(socket, form: to_form(...))` and `<.form for={@form} id="msg-form">`), then access those forms in the template via `@form[:field]` | ||
| - Always use the imported Phoenix.Component.form/1 and Phoenix.Component.inputs_for/1 functions to build forms | ||
| - **Always** add unique DOM IDs to key elements (like forms, buttons, etc) when writing templates, these IDs can later be used in tests (`<.form for={@form} id="product-form">`) |
There was a problem hiding this comment.
Out of my head, not tested against a coding agent:
| - **Always** add unique DOM IDs to key elements (like forms, buttons, etc) when writing templates, these IDs can later be used in tests (`<.form for={@form} id="product-form">`) | |
| - **Always** add unique and deterministic DOM IDs to key elements (like forms, buttons, etc) when writing templates, these IDs can later be used in tests (`<.form for={@form} id="product-form">`) |
or
| - **Always** add unique DOM IDs to key elements (like forms, buttons, etc) when writing templates, these IDs can later be used in tests (`<.form for={@form} id="product-form">`) | |
| - **Always** add unique and stable DOM IDs to key elements (like forms, buttons, etc) when writing templates, these IDs can later be used in tests (`<.form for={@form} id="product-form">`) |
The importance of deterministic/stable is that form recovery depends on that, and change-tracking would unnecessarily send diffs if IDs were unique but randomly generated. Might be something for the regular docs, though.
Prune redundant Phoenix, Ecto, Elixir, HTML, LiveView, and testing guidance. Consolidate the remaining form, stream, hook, and test rules, correct stream prepend positioning, and include Ecto.Query in generated seed files.
Update the generated rules according to the following principles:
Remove information that is now commonplace, such as Tailwind v4 setup.
Remove information covering mistakes that already produce compilation errors.
Remove negative “do not” guidance in accordance with current model prompting guidelines.
Evaluate each rule through an isolated Codex section coordinating Codex and Claude CLI runs with the prompt below: