docs(contributing): 5 recipes + decision flowchart for adding features
All checks were successful
plugin-tests / test (push) Successful in 6s
All checks were successful
plugin-tests / test (push) Successful in 6s
CONTRIBUTING.md closes the "how do I add anything" gap: the protocol PRD defines the CONTRACT (what's allowed, what's verified), but until now recipes were implicit — readers had to reverse-engineer patterns from existing routes/vault_status.py + static/app.js + the STT migration. Five recipes, one decision flowchart: A Add a new /api/* endpoint B Add a new UI element (CSS/JS/asset) C Extend the loader API (add an 8th method) — STT migration is the example D Add a config value E Forced-internal escape hatch (use sparingly, discipline at 5 rows) Each recipe: numbered steps, file paths, commit message template, test requirement. Plus a PR checklist + quick-reference index. README.md gets a top-of-page pointer so a new contributor lands on the recipes within one click. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Svrnty
parent
0ef66ab599
commit
bd73a2df24
248
CONTRIBUTING.md
Normal file
248
CONTRIBUTING.md
Normal file
@ -0,0 +1,248 @@
|
||||
# CONTRIBUTING — svrnty-hermes-webui-plugin
|
||||
|
||||
Recipes for adding features to the plugin. The contract lives in `hermes/docs/SVRNTY-PLUGIN-PROTOCOL.md`; this file is the **how-to** layer.
|
||||
|
||||
**Before anything else:** read `CLAUDE.md` (the Karpathy 4 rules) and the protocol PRD §4 (the four hard rules). Every recipe assumes those are honored.
|
||||
|
||||
---
|
||||
|
||||
## Decision flowchart — where does my change belong?
|
||||
|
||||
```
|
||||
Q1. Does my change need new backend Python (a new /api/* endpoint,
|
||||
a server-side hook into streaming/agent, file I/O, DB access)?
|
||||
│
|
||||
├── NO → goes in plugin/static/ (CSS, JS, fonts, assets)
|
||||
│ Recipe B
|
||||
│
|
||||
└── YES → Q2. Can the 7-method loader API express it?
|
||||
(api.register_route · register_static · inject_script ·
|
||||
inject_stylesheet · config_get · logger ·
|
||||
register_audio_attachment_processor)
|
||||
│
|
||||
├── YES → goes in plugin/routes/<feature>.py
|
||||
│ Recipe A
|
||||
│
|
||||
└── NO → Q3. Would extending the loader API help all
|
||||
future plugin features (not just this one)?
|
||||
│
|
||||
├── YES → propose new API method
|
||||
│ Recipe C (needs PRD bump + JP review)
|
||||
│
|
||||
└── NO → forced-internal escape hatch
|
||||
Recipe E (CONNECTION-MAP entry + risk)
|
||||
```
|
||||
|
||||
If you're unsure, default to plugin (Rule 1: never touch upstream unless forced).
|
||||
|
||||
---
|
||||
|
||||
## Recipe A — Add a new `/api/*` endpoint
|
||||
|
||||
1. Create `routes/<feature>.py`. Skeleton:
|
||||
|
||||
```python
|
||||
"""GET|POST /api/<feature> — <one-line purpose>."""
|
||||
import json
|
||||
|
||||
def register(api):
|
||||
log = api.logger("svrnty.routes.<feature>")
|
||||
api.register_route("/api/<feature>", "GET", _handle)
|
||||
log.info("<feature> endpoint registered")
|
||||
|
||||
def _handle(handler, parsed):
|
||||
payload = {"ok": True, "data": ...} # your logic here
|
||||
body = json.dumps(payload).encode("utf-8")
|
||||
handler.send_response(200)
|
||||
handler.send_header("Content-Type", "application/json; charset=utf-8")
|
||||
handler.send_header("Content-Length", str(len(body)))
|
||||
handler.send_header("Cache-Control", "no-store")
|
||||
handler.end_headers()
|
||||
handler.wfile.write(body)
|
||||
return True
|
||||
```
|
||||
|
||||
2. Register the module in `plugin.py` `_phase2_routes()`:
|
||||
|
||||
```python
|
||||
return [
|
||||
"transcribe",
|
||||
"vault_status",
|
||||
"<feature>", # ← add here
|
||||
]
|
||||
```
|
||||
|
||||
3. Add `tests/unit/test_<feature>.py` — at minimum:
|
||||
- `test_register_wires_one_route` — `api.register_route` called once with right path/method
|
||||
- `test_handler_returns_<status>_on_<input>` — success path
|
||||
- `test_handler_handles_<failure>` — error path returns clean response
|
||||
|
||||
4. Regenerate connection map:
|
||||
|
||||
```bash
|
||||
python3 scripts/ast-connection-map.py
|
||||
```
|
||||
|
||||
5. Update `manifest.yaml` `routes:` section with the new path + status.
|
||||
|
||||
6. Run full suite + commit:
|
||||
|
||||
```bash
|
||||
pytest tests/ -q # must be all green
|
||||
git add routes/<feature>.py plugin.py tests/unit/test_<feature>.py manifest.yaml CONNECTION-MAP.md
|
||||
git commit -m "feat(plugin): <feature> endpoint at /api/<feature>"
|
||||
```
|
||||
|
||||
**Reference implementation:** `routes/vault_status.py` + `tests/unit/test_vault_status.py`.
|
||||
|
||||
---
|
||||
|
||||
## Recipe B — Add a new UI element (CSS / JS / asset)
|
||||
|
||||
1. Decide where it goes in `static/`:
|
||||
- **CSS rules** → `static/app.css` (loaded after upstream → wins via cascade)
|
||||
- **JS behavior** → `static/app.js` (loaded as `defer`)
|
||||
- **Fonts/images** → `static/<subdir>/`
|
||||
|
||||
2. For JS UI additions, follow the existing pattern in `static/app.js`:
|
||||
- Idempotent guard inside the IIFE (`window.__svrntyExtLoaded` already covers re-load)
|
||||
- Use a per-element `dataset.svrntyXxx="1"` sentinel to prevent double-install
|
||||
- Hook into existing DOM via `MutationObserver` (the WebUI rebuilds panels on tab switches)
|
||||
- Reference existing IDs only — never invent new IDs that upstream might use later
|
||||
|
||||
3. Test (lightweight static checks in `tests/unit/test_app_js.py`):
|
||||
- File present + non-empty
|
||||
- Idempotent guard present
|
||||
- Any `/api/*` URL string the JS calls matches a registered plugin route
|
||||
|
||||
4. CONNECTION-MAP auto-picks up `/api/*` URLs from JS — regen + commit.
|
||||
|
||||
**Reference implementations:**
|
||||
- Vault status panel: `static/app.js:_injectVaultPanel + _loadVaultStatus`
|
||||
- Voice-message mic: `static/app.js:_vmStart + _vmAttachAndSend`
|
||||
|
||||
---
|
||||
|
||||
## Recipe C — Extend the loader API (add an 8th method)
|
||||
|
||||
**Justification bar:** the new method must enable a CLASS of features (audio processors enabled STT + future processors), not just one feature. Single-feature needs go through Recipe E (forced-internal).
|
||||
|
||||
1. Open the PRD: `hermes/docs/SVRNTY-PLUGIN-PROTOCOL.md`. Edit §5.1's API table to add the new method with one-line purpose. Bump the row count in any "7-method" references.
|
||||
|
||||
2. Open `hermes-webui/api/svrnty_plugin_loader.py` (the lone fork commit). Add:
|
||||
- Module-level registry: `_NEW_THING: List[X] = []`
|
||||
- Method on `_PluginAPI`: validates input, appends to registry, logs at INFO
|
||||
- Public accessor: `def plugin_new_things() -> List[X]: return list(_NEW_THING)`
|
||||
- Update the `load_plugin()` summary log to count the new dimension
|
||||
|
||||
3. Wire the consumer in upstream (the hook point that fires the registered fns):
|
||||
- Find the loosely-coupled spot in upstream code (streaming, session, request lifecycle)
|
||||
- Add a small loop calling registered things; wrap each in try/except so a buggy plugin can't crash the agent
|
||||
- Keep the loop in the SAME loader commit (still 1 fork commit total)
|
||||
|
||||
4. Update `scripts/ast-connection-map.py`'s `PUBLIC_API` set to include the new method name.
|
||||
|
||||
5. Update `tests/evals/test_features.py:test_eval_loader_contract_unchanged` — add the new method to `required`.
|
||||
|
||||
6. Use the new method from the first consumer route under `routes/`.
|
||||
|
||||
7. Amend the lone fork commit (`hermes-webui/api/svrnty_plugin_loader.py` + new hook site):
|
||||
|
||||
```bash
|
||||
cd hermes-webui
|
||||
git add api/svrnty_plugin_loader.py api/<hook-site>.py
|
||||
git commit --amend --no-edit
|
||||
git push openharbor jp --force-with-lease # own fork branch
|
||||
```
|
||||
|
||||
8. Commit + push the consumer plugin route + PRD update.
|
||||
|
||||
9. Run `protocol-validate.sh` — must still be 7/7 (the §10.1 check counts commits, not LOC; amending leaves the count at 1).
|
||||
|
||||
**Reference implementation:** the STT migration that added `register_audio_attachment_processor` — see hermes-webui commit `79c6665f` + plugin commit `37123f57`.
|
||||
|
||||
---
|
||||
|
||||
## Recipe D — Add a config value
|
||||
|
||||
1. Define the env var with the **`SVRNTY_<PROJECT>_<KEY>`** convention OR keep an upstream-style `HERMES_<KEY>` if the value semantically belongs to the upstream layer.
|
||||
|
||||
2. Document it in the **plugin** `.env.example` (when one is added) or `README.md` install section. Never in upstream's `.env.example`.
|
||||
|
||||
3. Read at call time via `api.config_get(key, default)` (which wraps `os.environ.get`). Never cache across calls — env can be edited + WebUI restarted.
|
||||
|
||||
4. If the value gates a feature on/off, the absence path must return a clean 503 or skip gracefully — see `routes/transcribe.py:_handle_transcribe` (returns 503 when `HERMES_WEBUI_STT_URL` unset).
|
||||
|
||||
5. Add a test that asserts the absence path doesn't crash + returns a useful signal.
|
||||
|
||||
---
|
||||
|
||||
## Recipe E — Forced-internal escape hatch (use sparingly)
|
||||
|
||||
When the public API can't express what you need AND the change isn't worth extending the API for (e.g. one-off, deep upstream internal).
|
||||
|
||||
1. Import the upstream symbol directly. Add a `# CONNECTION:` comment ABOVE the import naming:
|
||||
- **Why** the public API can't handle this
|
||||
- **Risk** if upstream renames or removes the symbol
|
||||
- **Detection** — how `protocol-validate.sh` / drift CI would catch breakage
|
||||
|
||||
```python
|
||||
# CONNECTION: needs hermes-webui's internal session state — no public API
|
||||
# exposes this; risk medium (upstream renames _active_state_db_path roughly
|
||||
# 1-2× per year per git log); detection via tests/integration on each upstream tag.
|
||||
from api.profiles import get_active_hermes_home # noqa: E402
|
||||
```
|
||||
|
||||
2. The AST walker picks up the import + the comment → CONNECTION-MAP shows the row under **forced internal dependencies** with your justification.
|
||||
|
||||
3. Add an `tests/evals/test_<feature>.py` row that explicitly asserts the symbol still exists + behaves expected. This is the early-warning siren when upstream changes.
|
||||
|
||||
4. Audit quarterly: every forced-internal row > 6 months old is a candidate for promotion to a public-API method (Recipe C) or refactor to remove the dep.
|
||||
|
||||
**Discipline:** if `forced_internal` count exceeds **5 rows total**, stop adding features and refactor — protocol §11 risk row.
|
||||
|
||||
---
|
||||
|
||||
## Commit message conventions (mirrors PRD §8.1)
|
||||
|
||||
| Prefix | When |
|
||||
|---|---|
|
||||
| `feat(plugin): <subject>` | new feature inside plugin |
|
||||
| `fix(plugin): <subject>` | bug fix inside plugin |
|
||||
| `sync(upstream): rebase to <tag>` | manifest.yaml min_upstream_version bumped |
|
||||
| `map(connection): <subject>` | manual CONNECTION-MAP touch (rare; usually auto) |
|
||||
| `ci(drift): <subject>` | `.github/workflows/*` changes |
|
||||
| `docs(protocol): <subject>` | protocol PRD updates |
|
||||
| `feat(svrnty): loader …` | the lone fork commit (in `hermes-webui`, not here) |
|
||||
|
||||
Body should answer: WHAT the change does, WHY (which recipe), and the test status (`X/Y PASS`).
|
||||
|
||||
---
|
||||
|
||||
## PR checklist (drop into PR description)
|
||||
|
||||
```
|
||||
- [ ] Recipe followed: [A | B | C | D | E]
|
||||
- [ ] CONNECTION-MAP.md regenerated + committed (or N/A for static-only changes)
|
||||
- [ ] tests/ added or extended; full suite passes locally
|
||||
- [ ] manifest.yaml updated (routes section or upstream pin) if applicable
|
||||
- [ ] No fork edits unless Recipe C (loader API extension)
|
||||
- [ ] If Recipe C: PRD §5.1 bumped + hermes-webui loader commit amended
|
||||
- [ ] No new forced-internal entries unless Recipe E (justified in source)
|
||||
- [ ] Karpathy 4 rules followed (Think · Simple · Surgical · Goal-driven)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Quick reference
|
||||
|
||||
| Need | Read |
|
||||
|---|---|
|
||||
| The hard rules | `CLAUDE.md` (Karpathy) + protocol PRD §4 |
|
||||
| What "done" looks like | protocol PRD §10 + `protocol-validate.sh` |
|
||||
| Existing patterns | `routes/vault_status.py` · `routes/transcribe.py` · `static/app.js` |
|
||||
| Map of what touches what | `CONNECTION-MAP.md` (auto-generated) |
|
||||
| One-command upgrade | `make sync-upstream` |
|
||||
| One-command smoke | `make smoke` |
|
||||
|
||||
If a recipe is missing or unclear, that's a CONTRIBUTING bug — open a PR against this file.
|
||||
@ -4,6 +4,8 @@ THE single repo holding every Svrnty modification to `nesquena/hermes-webui`. Lo
|
||||
|
||||
**Protocol contract:** [`hermes/docs/SVRNTY-PLUGIN-PROTOCOL.md`](../docs/SVRNTY-PLUGIN-PROTOCOL.md) — read this before contributing.
|
||||
|
||||
**Adding features?** Read [`CONTRIBUTING.md`](CONTRIBUTING.md) for the 5 recipes + decision flowchart.
|
||||
|
||||
## Install
|
||||
|
||||
```bash
|
||||
|
||||
Loading…
Reference in New Issue
Block a user