Files
hermes-webui/CONTRIBUTING.md
ebf64b8a79 docs: fold in Rod's PR-review feedback into GUIDELINES + CONTRIBUTING (#6211)
* docs: fold in Rod's PR-review feedback into GUIDELINES + CONTRIBUTING

Based on rodboev's feedback distilled from 50+ recent PRs. Four accepted
points plus one clause, folded into existing rules rather than adding new ones:

- Rule 6: load the reporter's shipped reproduction, don't rebuild a fixture
  from your reading of it (the one genuine hole — a fix and test from the same
  wrong model agree with each other and certify a no-op).
- Rule 2: confirm a value is authoritative (declared at the point of intent),
  not inferred from id prefix / content shape / emptiness / DOM state.
- Rule 4: read prior PRs and review threads to find a subsystem's real variants
  instead of inventing axes from the single case handed to you.
- Rule 1: the chokepoint is the smallest boundary that contains the fault, not
  the widest you can reach (don't disable a whole pipeline to suppress one output).
- "Show your work": name who owns the truth for any claim the repo doesn't own.

CONTRIBUTING.md carries the two contributor-facing points (repro-loading,
proof-ownership) in the PR-description section, deferring detail to GUIDELINES.md.

Co-authored-by: rodboev <rodboev@users.noreply.github.com>

* docs: tighten Rule 6 escape hatch per Rod — gate on shape under-specified, not file absent

Rod's review: the #5749 no-op came from a fully prose-specified repro (fenced JSON
+ field-level conditions + steps), not a missing file. The old hatch ('why the issue
gave you nothing to load') reads as 'no downloadable attachment', letting someone walk
past a binding JSON block. Gate the hatch on the SHAPE being under-specified instead:
a fenced JSON structure / field conditions / step list pin the shape as bindingly as a
file; if pinned, satisfy every condition and don't add a property the shape never had
to make a guard fire; only say 'constructed, assumed X' when the shape is truly unpinned.

Co-authored-by: rodboev <rodboev@users.noreply.github.com>

* docs: distill Rule 6 repro-shape guidance into a principle (Rod style note)

Rod: 'distill into durable principles, don't enumerate lists; strip negative
conditions that read like narration.' Reworked the addition to lead with the
principle ('a reproduction is whatever pins the bug's shape'), collapse the
capture/JSON/conditions/steps enumeration into flowing prose, and convert the
'don't add a property...' negative into a positive imperative ('bind your fixture
to that shape: satisfy every condition... instead of granting...'). Same for the
CONTRIBUTING bullet.

---------

Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
2026-07-17 20:40:55 -07:00

230 lines
9.2 KiB
Markdown

# Contributing to Hermes WebUI
Thanks for contributing.
Hermes WebUI is intentionally simple to work on: Python on the server, vanilla JS in the browser, no build step, no bundler, no frontend framework. The best pull requests preserve that simplicity while solving a real problem cleanly.
## Before You Start
- Read [`AGENTS.md`](AGENTS.md) if an AI assistant is doing or helping with the
change.
- Read [`docs/GUIDELINES.md`](docs/GUIDELINES.md) — the change guidelines that get a PR
merged in one review round instead of several (what "complete and verified" means here).
- Read [`docs/CONTRACTS.md`](docs/CONTRACTS.md) and any linked contract/RFC for
the subsystem you will touch.
- For UI or UX work, read [`docs/UIUX-GUIDE.md`](docs/UIUX-GUIDE.md)
and [`DESIGN.md`](DESIGN.md).
- For runtime, streaming, recovery, replay, compression, context reconstruction,
or session metadata work, start with [`docs/rfcs/README.md`](docs/rfcs/README.md)
and the relevant RFC listed there.
Use those documents as review guardrails: keep the change scoped, preserve the
no-build-step architecture, update docs when behavior changes, put
release-note-worthy details in the PR body, include UI evidence for UI changes,
and add tests for behavior changes where practical.
### Contract-affecting PRs
A contract-affecting PR is any change that updates a public contract document,
an RFC, a contributor guide, a product-semantics test, or behavior that those
documents already describe. These PRs need an explicit `Contract Routing` section
in the PR body that names the touched contract family and the evidence used.
See [`docs/CONTRACTS.md#contract-routing`](docs/CONTRACTS.md#contract-routing)
for the short routing shape and [`docs/CONTRACTS.md#contract-changes`](docs/CONTRACTS.md#contract-changes)
for intentional contract changes.
If the PR intentionally changes an existing contract, add a `Contract Change`
section that states the old rule, the new rule, and why the change is justified.
Do not silently redefine product behavior by changing tests alone; update the
corresponding docs in the same PR.
A release batch should call out included contract-affecting PRs separately
from ordinary fixes, even when the code diff is small and CI is green.
## Two Paths to a Strong Pull Request
### Path 1: Small, Focused Changes
This is the fastest path to review and merge.
- Fix one clear bug or add one tightly scoped improvement
- Touch the fewest files you can
- Avoid drive-by refactors mixed into functional changes
- Run the relevant tests locally before opening the PR
- Keep the PR description concise and specific
These are the changes that are easiest to review and safest to merge quickly.
### Path 2: Bigger Changes
If you want to change architecture, reshape a workflow, add a substantial UI feature, or alter core behavior, align on direction first.
- Open an issue, start a discussion, or open a draft PR early
- Explain the problem you are solving, not just the implementation you want
- Call out tradeoffs, migration risk, and any alternatives you considered
- Keep the final PR easy to review by separating unrelated work
Large changes are welcome, but surprise rewrites are hard to review well.
## What We Expect in Every PR
### 1. One Logical Change Per PR
Keep each PR focused. A small related group of fixes is fine. A bug fix plus a CSS cleanup plus a refactor plus a docs rewrite is not.
### 2. Local Verification
Run the test suite locally through the repo runner. It creates/uses a supported
Python 3.11-3.13 `.venv` and installs missing dev test dependencies, avoiding
unsupported system interpreters during collection:
```bash
./scripts/test.sh
```
CI also runs this suite on Python `3.11`, `3.12`, and `3.13`.
If your change affects browser behavior, also run the relevant manual checks from [TESTING.md](TESTING.md).
### 3. Clear PR Description
There is currently no PR template in this repo, so include the important sections yourself:
- Thinking Path
- What Changed
- Why It Matters
- Verification
- Risks / Follow-ups
- Model Used
Two things reviewers look for specifically in Verification, both detailed in
[`docs/GUIDELINES.md`](docs/GUIDELINES.md):
- When the issue pins the bug's shape — a session capture, a script, exact steps, or
a fenced JSON block / field-level trigger conditions — bind your test to *that* shape
rather than a fixture rebuilt from your reading of it, satisfying every condition it
names; otherwise the test can pass while the reported bug goes untouched. Reconstruct
the shape yourself only when the issue leaves it unpinned, and then say what you assumed.
- For any claim the repo doesn't own (browser behavior, a provider's API, a registry,
an OS convention), name who owns the truth and confirm your proof is one they'd accept.
In-page automation proves page behavior, not browser behavior; a mock proves your
intent, not the provider's contract.
If the change is user-visible, include screenshots or a short video.
If the change is release-note-worthy, include concise release-note wording in
the PR body. Do not edit `CHANGELOG.md` directly in ordinary contributor PRs;
the release workflow maintains it through release commits.
For UI or UX changes, before/after images are required. PRs that change the interface or interaction flow without before/after images may not receive meaningful review until that evidence is added.
### 4. AI Usage Disclosure
If AI helped produce the change, say so in the PR description.
Include:
- Provider
- Exact model name or ID
- Any notable mode or tool use that mattered
If no AI was used, write: `None — human-authored`.
### 5. Keep the Docs Honest
If your change alters behavior, architecture, testing, setup, or user-facing workflows, update the relevant docs in the same PR.
Common files:
- [README.md](README.md) for setup, usage, and contributor-facing commands
- [ROADMAP.md](ROADMAP.md) for shipped features and sprint history
- [ARCHITECTURE.md](ARCHITECTURE.md) for implementation details and design constraints
- [TESTING.md](TESTING.md) for manual and automated verification guidance
- [CHANGELOG.md](CHANGELOG.md) for release history context. Do not edit it in
ordinary contributor PRs; include release-note-ready wording in the PR body
so maintainers can carry it into the release workflow.
## Project-Specific Guidelines
### Preserve the Design Constraints
Hermes WebUI is deliberately:
- No build step
- No bundler
- No frontend framework
- Easy to modify from a terminal
Do not introduce new infrastructure or dependencies unless the gain is clear and the tradeoff is justified.
### Match the Existing Shape of the Codebase
- Server logic belongs in `api/` with `server.py` staying thin
- Frontend behavior belongs in the existing `static/*.js` modules
- Prefer extending current patterns over introducing parallel abstractions
- Keep changes legible to future contributors working directly from the repo in a terminal
### Be Careful With User-Facing Changes
This project is heavily UI-driven. If you change interaction flows, session behavior, workspace browsing, onboarding, or mobile layouts:
- test the happy path
- test reload behavior where relevant
- test narrow/mobile layouts where relevant
- include before/after images in the PR
### Security and Safety Matter
This app can expose workspace contents, run agent actions, and optionally sit behind a reverse proxy or Docker deployment. Treat auth, path handling, uploads, streaming, and environment handling as high-risk areas.
If your PR touches security-sensitive behavior, say so explicitly in the PR description and explain how you verified it.
## Writing a Good PR Message
Start with a short Thinking Path that explains the chain from project goal to the specific fix.
Example:
> - Hermes WebUI aims for near 1:1 parity with the Hermes CLI in a browser
> - Long-running chat turns rely on SSE streaming and session recovery
> - Reloading during an in-flight turn can leave the UI in an inconsistent state
> - The bug was that recovered sessions restored messages but not the live stream state
> - This PR fixes the recovery path so in-flight turns reconnect cleanly after reload
> - The benefit is that users can refresh or reconnect without losing visibility into active work
Another example:
> - Hermes WebUI is intentionally a simple Python + vanilla JS application
> - The right panel is used for workspace browsing and previews
> - On mobile, panel state changes need to be obvious and touch-friendly
> - The existing close affordance was inconsistent with the bottom-nav flow
> - This PR fixes the mobile panel close behavior and aligns it with the current navigation model
> - The result is fewer dead-end UI states on phones
After that, cover:
- what you changed
- why you changed it
- how you verified it
- what risks remain
## Review Tips
Want the smoothest review?
- Keep diffs tight
- Name things clearly
- Avoid unnecessary rewrites
- Add short comments only where the code would otherwise be hard to follow
- Respond directly to review feedback and update the PR description if the scope changes
## Development References
- [README.md](README.md)
- [ARCHITECTURE.md](ARCHITECTURE.md)
- [TESTING.md](TESTING.md)
- [ROADMAP.md](ROADMAP.md)
- [SPRINTS.md](SPRINTS.md)
Questions are best raised early, before a large change is finished.