* 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>
9.2 KiB
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.mdif an AI assistant is doing or helping with the change. - Read
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.mdand any linked contract/RFC for the subsystem you will touch. - For UI or UX work, read
docs/UIUX-GUIDE.mdandDESIGN.md. - For runtime, streaming, recovery, replay, compression, context reconstruction,
or session metadata work, start with
docs/rfcs/README.mdand 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
for the short routing shape and 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:
./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.
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:
- 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 for setup, usage, and contributor-facing commands
- ROADMAP.md for shipped features and sprint history
- ARCHITECTURE.md for implementation details and design constraints
- TESTING.md for manual and automated verification guidance
- 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/withserver.pystaying thin - Frontend behavior belongs in the existing
static/*.jsmodules - 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
Questions are best raised early, before a large change is finished.