Skip to content

trim agent formatting guidance#17617

Open
titusfortner wants to merge 1 commit into
trunkfrom
agent_format
Open

trim agent formatting guidance#17617
titusfortner wants to merge 1 commit into
trunkfrom
agent_format

Conversation

@titusfortner
Copy link
Copy Markdown
Member

🔗 Related Issues

Pares back #17366

Every time my agent edits anything it runs a several minute format command completely unnecessarily (since I use a pre-push hook). I added to my .local/AGENTS.md file not to do that, but it wasn't listening to me, which makes sense when I see that the demand to do this is repeated in many places.

💥 What does this PR do?

  1. Removes duplicate context window text. Anything in AGENTS.md is in the context window. The purpose of the other files is to be able to provide scoped information that can be loaded conditionally.
  2. Clarifies that format is a pre-push concern not a post-edit concern
  3. Have the agent suggest adding a pre-push hook if one doesn't exist
  4. Recommends the format script which is scoped to changes instead of the full rake tasks that always does everything

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Cleanup (formatting, renaming)

@selenium-ci selenium-ci added C-py Python Bindings C-rb Ruby Bindings C-dotnet .NET Bindings C-java Java Bindings C-nodejs JavaScript Bindings B-build Includes scripting, bazel and CI integrations C-rust Rust code is mostly Selenium Manager B-manager Selenium Manager labels Jun 3, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Consolidate and clarify agent formatting guidance as pre-push concern

📝 Documentation

Grey Divider

Walkthroughs

Description
• Removes duplicate formatting guidance from agent instruction files
• Consolidates formatting context into main AGENTS.md with clarified scope
• Shifts formatting from post-edit to pre-push concern with hook recommendation
• Removes language-specific formatting details that duplicate main guidance
Diagram
flowchart LR
  A["Duplicate formatting<br/>guidance across files"] -->|consolidate| B["Single source in<br/>AGENTS.md"]
  C["Post-edit formatting<br/>instruction"] -->|reframe| D["Pre-push hook<br/>recommendation"]
  E["Language-specific<br/>formatting details"] -->|remove| F["Cleaner agent<br/>instructions"]
  B --> G["Reduced agent<br/>context bloat"]
  D --> G
  F --> G

Loading

Grey Divider

File Changes

1. .github/copilot-instructions.md 📝 Documentation +1/-2

Remove post-edit formatting instruction

.github/copilot-instructions.md


2. AGENTS.md 📝 Documentation +5/-16

Reframe formatting as pre-push concern with hook

AGENTS.md


3. CLAUDE.local.md 📝 Documentation +0/-2

Remove duplicate formatting guidance

CLAUDE.local.md


View more (7)
4. CLAUDE.md 📝 Documentation +0/-2

Remove duplicate formatting guidance

CLAUDE.md


5. dotnet/AGENTS.md 📝 Documentation +0/-11

Remove language-specific formatting details

dotnet/AGENTS.md


6. java/AGENTS.md 📝 Documentation +0/-10

Remove language-specific formatting details

java/AGENTS.md


7. javascript/selenium-webdriver/AGENTS.md 📝 Documentation +0/-13

Remove language-specific formatting details

javascript/selenium-webdriver/AGENTS.md


8. py/AGENTS.md 📝 Documentation +0/-14

Remove language-specific formatting details

py/AGENTS.md


9. rb/AGENTS.md 📝 Documentation +0/-10

Remove language-specific formatting details

rb/AGENTS.md


10. rust/AGENTS.md 📝 Documentation +0/-8

Remove language-specific formatting details

rust/AGENTS.md


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Jun 3, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0)

Grey Divider


Action required

1. Format guidance mismatches CI 🐞 Bug ⚙ Maintainability
Description
AGENTS.md now claims running ./scripts/format.sh before pushing avoids CI formatter failures, but
the CI format job still runs ./go format and fails on any diff produced by that task. Since `./go
format runs the full Rake formatting suite unconditionally while scripts/format.sh` is
change-scoped, contributors may follow the new guidance yet still fail CI or receive conflicting
instructions.
Code

AGENTS.md[R74-78]

Evidence
The PR changes AGENTS.md to recommend ./scripts/format.sh to avoid CI formatter failures, but CI
still runs ./go format and fails if it produces diffs; additionally, ./go format is the full
Rake :format task while scripts/format.sh is explicitly change-scoped, so they are not
guaranteed to behave identically in all cases.

AGENTS.md[69-78]
.github/workflows/ci-lint.yml[47-74]
Rakefile[128-144]
scripts/format.sh[1-8]
scripts/format.sh[35-137]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AGENTS.md` was updated to recommend `./scripts/format.sh` / `--pre-push` to prevent CI formatter failures, but CI currently runs `./go format` and reports failures based on that command. This creates inconsistent guidance and a real risk that contributors follow the documented workflow yet still hit CI failures.

## Issue Context
- `./go format` executes the Rake `:format` task, which formats across all languages unconditionally.
- `./scripts/format.sh` is change-scoped (runs some language formatters only when matching paths changed).
- CI currently enforces formatting by running `./go format` (and tells contributors to run it).

## Fix Focus Areas
Pick one clear source-of-truth and make both CI + docs consistent:
- Update CI workflows to run `./scripts/format.sh` (optionally `--pre-push`) and update error messages to reference it, OR
- Update `AGENTS.md` to explicitly state CI runs `./go format` and recommend `./go format` as the fallback/source-of-truth if CI fails.

References:
- AGENTS.md[69-78]
- .github/workflows/ci-lint.yml[47-74]
- Rakefile[128-144]
- scripts/format.sh[1-8], scripts/format.sh[35-137]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread AGENTS.md
Comment on lines +74 to +78
- Formatting is a pre-push concern, not a post-edit one. If `./scripts/format.sh --pre-push` is already in the user's pre-push hook, leave it alone — the hook handles it. If not, run or suggest `./scripts/format.sh` before pushing to avoid CI formatter failures, and recommend adding the hook (once):
```bash
#!/usr/bin/env bash
./scripts/format.sh --pre-push
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. Format guidance mismatches ci 🐞 Bug ⚙ Maintainability

AGENTS.md now claims running ./scripts/format.sh before pushing avoids CI formatter failures, but
the CI format job still runs ./go format and fails on any diff produced by that task. Since `./go
format runs the full Rake formatting suite unconditionally while scripts/format.sh` is
change-scoped, contributors may follow the new guidance yet still fail CI or receive conflicting
instructions.
Agent Prompt
## Issue description
`AGENTS.md` was updated to recommend `./scripts/format.sh` / `--pre-push` to prevent CI formatter failures, but CI currently runs `./go format` and reports failures based on that command. This creates inconsistent guidance and a real risk that contributors follow the documented workflow yet still hit CI failures.

## Issue Context
- `./go format` executes the Rake `:format` task, which formats across all languages unconditionally.
- `./scripts/format.sh` is change-scoped (runs some language formatters only when matching paths changed).
- CI currently enforces formatting by running `./go format` (and tells contributors to run it).

## Fix Focus Areas
Pick one clear source-of-truth and make both CI + docs consistent:
- Update CI workflows to run `./scripts/format.sh` (optionally `--pre-push`) and update error messages to reference it, OR
- Update `AGENTS.md` to explicitly state CI runs `./go format` and recommend `./go format` as the fallback/source-of-truth if CI fails.

References:
- AGENTS.md[69-78]
- .github/workflows/ci-lint.yml[47-74]
- Rakefile[128-144]
- scripts/format.sh[1-8], scripts/format.sh[35-137]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations B-manager Selenium Manager C-dotnet .NET Bindings C-java Java Bindings C-nodejs JavaScript Bindings C-py Python Bindings C-rb Ruby Bindings C-rust Rust code is mostly Selenium Manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants