From bd73a2df249ef79fc75a6141c1115b5b7cd6cb53 Mon Sep 17 00:00:00 2001 From: Svrnty Date: Sat, 23 May 2026 10:41:17 -0400 Subject: [PATCH] docs(contributing): 5 recipes + decision flowchart for adding features MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- CONTRIBUTING.md | 248 ++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 2 + 2 files changed, 250 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..e03d308 --- /dev/null +++ b/CONTRIBUTING.md @@ -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/.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/.py`. Skeleton: + + ```python + """GET|POST /api/ — .""" + import json + + def register(api): + log = api.logger("svrnty.routes.") + api.register_route("/api/", "GET", _handle) + log.info(" 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", + "", # ← add here + ] + ``` + +3. Add `tests/unit/test_.py` — at minimum: + - `test_register_wires_one_route` — `api.register_route` called once with right path/method + - `test_handler_returns__on_` — success path + - `test_handler_handles_` — 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/.py plugin.py tests/unit/test_.py manifest.yaml CONNECTION-MAP.md + git commit -m "feat(plugin): endpoint at /api/" + ``` + +**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//` + +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/.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__`** convention OR keep an upstream-style `HERMES_` 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_.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): ` | new feature inside plugin | +| `fix(plugin): ` | bug fix inside plugin | +| `sync(upstream): rebase to ` | manifest.yaml min_upstream_version bumped | +| `map(connection): ` | manual CONNECTION-MAP touch (rare; usually auto) | +| `ci(drift): ` | `.github/workflows/*` changes | +| `docs(protocol): ` | 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. diff --git a/README.md b/README.md index 787505c..f85fec2 100644 --- a/README.md +++ b/README.md @@ -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