feat: Operating Room, guided treatment, doctor recommendations & UI overhaul#15
feat: Operating Room, guided treatment, doctor recommendations & UI overhaul#15dannym-arx wants to merge 6 commits into
Conversation
Summary by CodeRabbit
WalkthroughAdds a complete "Operating Room" feature: UI elements (NIP‑07 state, CANCEL, OR editor), audit enhancements (hints, doctor’s notes, canOperate/canDelete flags), fix actions and Nak commands, many operate/* modules (editor, guide, health, procedures, room, autostage), dialog/typewriter updates, and large SNES‑style CSS. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as rgba(59,130,246,0.5)
participant OR as rgba(234,88,12,0.5)
participant Editor as rgba(34,197,94,0.5)
participant AutoStage as rgba(168,85,247,0.5)
participant Guide as rgba(249,115,22,0.5)
participant Procedures as rgba(6,182,212,0.5)
participant Health as rgba(234,179,8,0.5)
participant Relays as rgba(107,114,128,0.5)
User->>UI: click "OPERATE" / enterOperatingRoom()
UI->>OR: enterOperatingRoom(auditState)
OR->>AutoStage: autoStageFromPrescriptions(auditState, compiled)
AutoStage-->>OR: staged changes + suggestions
OR->>Editor: initEditorState(auditState)
OR->>Health: checkAllRelaysHealth(relayUrls)
Health->>Relays: ws connect / NIP-11 / read tests
Relays-->>Health: health results
Health-->>OR: per-relay health summary
alt guided workflow active
OR->>Guide: initGuide(prescriptions)
loop user fixes
User->>Guide: nextStep()/fixAction()
Guide->>Editor: stageAddRelay()/stageRemoveRelay()
end
end
User->>OR: executeOperations()
OR->>Procedures: planProcedures()
Procedures-->>OR: unsigned events
OR->>User: request sign (NIP-07)
User-->>Procedures: signed events
Procedures->>Relays: publish events
Relays-->>Procedures: publish statuses
Procedures-->>OR: aggregated results
OR-->>UI: render results, exitOperatingRoom()
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
The Grand Marmot prescribes: pay sharp attention to NIP‑07 sign/error paths, relay timeouts, DOM node preservation when entering/exiting the Operating Room, and correct propagation of capability flags (canOperate / canDelete*). Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (5 passed)
✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
🔍 Dr. Marmot Preview
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/actions.js`:
- Around line 446-467: The current relay scan in the deleteDeprecatedKind4 loop
uses a hard cap of limit: 500 when calling pool.subscribeMany (inside the for-of
relaysToInvestigate block), which can return a partial result; change this to
paginate rather than a single capped query: implement a loop around the
subscribeMany call that requests up to 500 per page, collects events, records a
marker from the last-received event (e.g., last event created_at or id), then
issues subsequent subscribeMany queries using that marker (or an appropriate
filter like since/until or id-based pagination supported by the relay) until a
page returns fewer than the page size; merge collected pages before proceeding
to deletion and only then setRelayState to success; if pagination is not
feasible for a relay, surface the 500-item cap in the UI/result message so users
are informed.
In `@js/audit/chartRender.js`:
- Around line 159-163: The 'reduce-relays' branch renders an active
"enter-operating-room" button even when canOperate is false; update the
rendering logic in the action branch that checks action === 'reduce-relays' (and
the similar branch around the other occurrence) to require canOperate before
producing an enabled button with data-action="enter-operating-room" (e.g., only
include the active <button class="rx-action-btn"
data-action="enter-operating-room" ...> when canOperate is true), and otherwise
render a disabled/locked UI (or omit the data-action / add a disabled
attribute/class) so the NIP-07 path cannot be used as a side door when
canOperate is false; make the same change where the duplicate branch appears.
In `@js/audit/findingsPrescriptions.js`:
- Around line 111-121: The marmot branch (the condition setting noteKey =
'doctorNoteMarmot') is missing a check for KeyPackage and therefore matches
multi-category failures; update the condition in
js/audit/findingsPrescriptions.js so it requires !hasKp (i.e., change the
hasMarmot branch to hasMarmot && !hasRelayConfig && !hasSync && !hasKp) or
alternatively move the categories.length >= 2 check above the single-failure
branches so noteKey = 'doctorNoteMultiple' wins for multi-category cases; adjust
the condition(s) around noteKey assignment accordingly.
- Around line 614-617: The flags returned (canDeleteKps, canDeleteOrphanedKps,
canDeleteKind4) must be strict booleans instead of possibly undefined; convert
each to a boolean before returning (e.g. use Boolean(...) or !! on expressions
that may be undefined) so change the assignments for canDeleteOrphanedKps
(currently orphanedKpRelays && orphanedKpRelays.length > 0) to a boolean, and
ensure canDeleteKind4 (auditCtx.hasDeprecatedK4) and canDeleteKps are likewise
coerced to true/false; keep the same return shape ({ prescriptions: uniqueRx,
canRebroadcast, canDeleteKps, canDeleteOrphanedKps, canDeleteKind4 }).
In `@js/dialog.js`:
- Around line 48-53: The dialog messages are being inserted with entry.innerHTML
= content which allows untrusted HTML/JS and causes XSS; change to set
entry.textContent (or explicitly HTML-escape/allowlist-sanitize the input) in
the _appendToLog function to render text safely, and apply the same fix to the
other message-rendering site (the analogous code around the second insertion
block referenced in the review). Ensure any legitimate formatting needs are
handled by a safe sanitizer/allowlist rather than using innerHTML.
In `@js/dom.js`:
- Around line 29-35: The JSDoc for getChartActionButtons is out of sync: its
`@returns` type omits the keys deleteOrphanedKps and deleteKind4 that the function
actually returns; update the JSDoc return object shape to include
deleteOrphanedKps: Element|null and deleteKind4: Element|null (alongside
rebroadcast, deleteKps, unifyRelays, operatingRoom) so the documented contract
matches the returned object from getChartActionButtons.
In `@js/operate/editor.js`:
- Around line 162-180: stageToggleReadWrite currently mutates the canonical
relay object (currentRelays) in place so a "DISCARD ALL" via clearAllChanges
cannot restore prior readWrite flags; instead, avoid mutating entry.readWrite
inside stageToggleReadWrite — only record the change (include prevReadWrite in
the stagedChanges entry already being pushed) and update any UI-only model if
needed; then update clearAllChanges to walk stagedChanges (look for entries with
type === 'modify') and restore the original readWrite using prevReadWrite on the
matching relay in currentRelays/stagedChanges when discarding so the original
state is fully reversible.
In `@js/operate/guide.js`:
- Around line 74-77: The guided mode flag is left true when there are no steps
or when the course finishes; update the logic around currentStep and isGuided so
guided mode is disabled whenever steps.length === 0 or the currentStep would be
out of range. Specifically, in the initialization/code that sets currentStep =
steps.length > 0 ? 0 : -1 and isGuided = true, change it to set isGuided =
(steps.length > 0) and ensure currentStep is -1 when steps is empty; also modify
any navigation/finish logic (the blocks around currentStep
incrementing/finishing referenced at lines ~83-89) to set isGuided = false when
currentStep >= steps.length (or when the course is emptied) so the UI state is
never left in guided mode with no valid step.
In `@js/operate/health.js`:
- Around line 32-50: The done() helper in testWsConnect() uses
clearTimeout(timer) while timer is declared later, causing a TDZ ReferenceError
when WebSocket construction throws; fix by moving the timer declaration before
done (or initialize let timer = null) and in done only call clearTimeout(timer)
if timer != null; ensure the later assignment uses timer = setTimeout(...).
Reference symbols: function testWsConnect, variables done, timer, resolved, and
the WebSocket constructor try/catch.
In `@js/operate/nip11.js`:
- Around line 45-70: The abort timeout timer is only cleared after a successful
fetch, leaving the timer running on fast failures; ensure the timer is always
cleared by moving clearTimeout(timer) into a finally block (or equivalent) that
runs after the try/catch so the timer is cleared regardless of success, HTTP
error, content-type checks, or thrown exceptions; update the function using
AbortController, timer, NIP11_TIMEOUT_MS, and the fetch/httpUrl logic so
clearTimeout(timer) is invoked in all code paths.
- Around line 42-44: Validate the incoming relayUrl at the start of fetchNip11
by calling the shared validateRelayUrl(relayUrl) helper and reject/throw if it
fails before calling wsToHttp(relayUrl) or performing any network I/O;
specifically, add the validation check at the top of the export async function
fetchNip11(relayUrl) and only call wsToHttp(relayUrl) and proceed with the
try/fetch flow when validateRelayUrl returns success, returning/throwing a clear
error for malformed input.
In `@js/operate/procedures.js`:
- Around line 35-38: The builder currently returns null when
getFinalRelayList(...) yields an empty array (e.g., in buildK10002Event), which
prevents intentional full-list removals from being published; instead of
returning null when relays.length === 0, construct and return a valid event that
contains an empty relays list so planProcedures() can treat it as an intentional
removal; update the same pattern in the other relay-builder functions noted (the
ones that short-circuit on relays.length === 0) and leave planProcedures()’s
no-op avoidance logic unchanged so it still skips genuinely unchanged kinds.
In `@js/operate/room.js`:
- Around line 178-179: The repeated DOM queries in room.js (e.g.,
relayList.querySelector(`.or-tray[data-kind="${step.targetKind}"]`) and other
querySelector/querySelectorAll uses around the referenced spots) must be moved
into dom.js as exported element references or helper functions; add functions
like getTrayByKind(relayList, kind) or export a mapped collection/getter from
dom.js that encapsulates `.or-tray[data-kind="..."]` lookups and any other
selectors used at the other locations, then import those helpers/refs into
room.js and replace direct calls to querySelector/querySelectorAll (e.g.,
replace the usage that sets tray and the occurrences at the other noted spots)
with the new dom.js exports. Ensure names match current symbols (relayList,
step.targetKind, tray) so callers can swap the direct query for the imported
helper/ref without changing surrounding logic.
- Around line 413-427: executeFixAction currently logs unknown actions but
doesn't signal failure, so the caller (the click handler around btn, and the
similar block at the other location) may mark steps as succeeded and
auto-advance; change the contract so executeFixAction either returns an explicit
boolean success (true/false) or throws on failure for unknown actions, and
update the caller(s) that use executeFixAction to check that result and treat
non-success as a failure: do not set succeeded = true unless the call returns
true (or doesn't throw), call say() from dialog.js with a user-facing error
message when a fix fails, and keep errBeep()/console.error for debugging; update
both the handler that references executeFixAction and the other similar block so
unknown actions do not report "DONE ✔" nor auto-advance the guide.
- Around line 191-246: runHealthChecks() mutates the UI and emits messages after
awaiting checkAllRelaysHealth even if exitOperatingRoom() ran, causing stale
completions to resurrect the UI; fix by creating a local session token (e.g.,
const session = Symbol() or incrementing id) at the start of runHealthChecks,
pass that token into onRelayProgress/onRelayDone callbacks and after the await
checkAllRelaysHealth(...) verify the token still matches the current OR session
before calling scanBeep(), say(...), setScanProgress(), removeScanBar(),
renderRoom(), or mutating healthResults; also ensure exitOperatingRoom()
clears/advances the session token so any late callbacks or post-await code
early-return when tokens differ to avoid repainting or late dialogs.
- Around line 526-529: The publishRelays assembly is unsafe because spreading
auditState.relaysToInvestigate or auditState.marmotRelays will throw if they are
null/undefined/non-array; update the code that builds publishRelays to
defensively coerce each source into an array (e.g., default to [] when falsy or
not an array) before spreading so executeOperations() cannot abort with a
TypeError; locate the code that declares publishRelays (the const publishRelays
= [...new Set([...])]) and wrap or replace the spread inputs with safe
conversions of auditState.relaysToInvestigate and auditState.marmotRelays into
arrays.
In `@style.css`:
- Around line 2297-2309: The CSS references to undefined custom properties
(--c-text and --c-panel) used by .chart-doctor-notes and .doctor-note-text are
causing inconsistent rendering; add canonical declarations for these tokens in
:root (or provide fallbacks using var(--c-text, <fallback>) and var(--c-panel,
<fallback>)) so the styles always resolve; update the global :root block to
include sensible default color values for --c-text and --c-panel and/or replace
the usages in .chart-doctor-notes and .doctor-note-text with var(..., fallback)
to ensure predictable text and background rendering.
- Around line 1150-1153: There are duplicate CSS rules for the selector
`#nip07-btn`:disabled; consolidate them into a single rule by merging all
properties (e.g., opacity and cursor) so no properties are lost, remove the
redundant block, and ensure the final `#nip07-btn`:disabled contains the combined
declarations (also update/verify the other occurrence referenced around the
later block so only one definition remains).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7740e0d9-375f-42c4-83dc-8a9a42e4a7fb
📒 Files selected for processing (17)
index.htmljs/actions.jsjs/audit.jsjs/audit/chartRender.jsjs/audit/findingsPrescriptions.jsjs/dialog.jsjs/dom.jsjs/main.jsjs/operate/autostage.jsjs/operate/editor.jsjs/operate/guide.jsjs/operate/health.jsjs/operate/nip11.jsjs/operate/procedures.jsjs/operate/room.jsjs/personalities.jsstyle.css
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESMimportstatements from CDN (nostr-tools via https://esm.sh/nostr-tools) or local modules only; no npm dependencies
Order imports as: Standard library → External → Local, with one blank line between groups
Use 4 spaces for indentation (not tabs)
Enforce soft line length limit of 100 characters and hard limit of 120 characters
Require semicolons at the end of statements
Use trailing commas in multi-line objects and arrays
Use camelCase for function names with descriptive action verbs (e.g.,queryRelayForKinds(),validateRelayUrl(),isValidBase64())
Use camelCase for variable names (e.g.,lastAuditState,isAuditing,relayUrl,pubkey)
Use UPPER_SNAKE_CASE for constants, with exception for exported object/map constants which use camelCase keys
Prefix boolean variables and functions withis,has,should, orcan(e.g.,isAuditing,hasError,isValidBase64())
Use JSDoc for function signatures to document parameters and return types
Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions likevalidateRelayUrl(),isValidBase64(),pubkeyToHex()
Use try-catch with console.error logging for error handling; never throw from module exports—return null/empty/error object instead
Usesay()fromdialog.jsfor user-facing error messages instead of alert or console
Export either one default export OR multiple named exports per file, not both
Define functions at module scope (no fake 'classes' or IIFE wrappers)
Centralize all DOM queries indom.js; import and use exported DOM element references instead of querying in other modules
Use Promise constructor or async/await for async patterns; wrap pool subscriptions in setTimeout to avoid hanging connections
Prefix unused function parameters with_so they are ignored by the no-unused-vars linter rule
**/*.js: Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Warn if 'throw new Error' is used dire...
Files:
js/dom.jsjs/operate/procedures.jsjs/dialog.jsjs/operate/nip11.jsjs/operate/guide.jsjs/main.jsjs/operate/health.jsjs/operate/editor.jsjs/operate/room.jsjs/audit/findingsPrescriptions.jsjs/operate/autostage.jsjs/audit.jsjs/actions.jsjs/personalities.jsjs/audit/chartRender.js
⚙️ CodeRabbit configuration file
**/*.js: Vanilla JS without build steps or frameworks.Security violations: flag without mercy.
Style issues: flag only if it genuinely improves things — the Grand Marmot has no patience for noise.Max 5 comments per file, 20 per PR. Choose wisely. Every missed carrot is a missed carrot. And we all know how rabbits behave when they're off their carrots.
The Laws of the Burrow:
- DOM access lives in
dom.jsand nowhere else.document.querySelectorin any other file is a trespass.- Side effects belong only in
main.js. Other modules export. That is all they do.- One export style per file. Named or default. The Grand Marmot does not negotiate on this.
- User messages go through
say()fromdialog.js.alert()is forbidden. It has always been forbidden.- Never throw from a module export. Return null, return empty, return an error object — but do not throw.
- Nostr pool subscriptions must be closed in
oneoseor asetTimeoutfallback. Leaks are how tunnels collapse.- External imports:
https://esm.sh/nostr-toolsonly. Everything else lives here, in the burrow.constis the law.letwhen you must.varis exile.===always.==is a war crime that the Grand Marmot has personally prosecuted.- Booleans must be prefixed:
is,has,should,can. No exceptions.- JSDoc on complex exported functions. Trivial helpers and one-liners need not apply.
Files:
js/dom.jsjs/operate/procedures.jsjs/dialog.jsjs/operate/nip11.jsjs/operate/guide.jsjs/main.jsjs/operate/health.jsjs/operate/editor.jsjs/operate/room.jsjs/audit/findingsPrescriptions.jsjs/operate/autostage.jsjs/audit.jsjs/actions.jsjs/personalities.jsjs/audit/chartRender.js
!main.js,**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Avoid side effects on import; only
main.jsis allowed to have side effects for initialization
Files:
js/dom.jsjs/operate/procedures.jsjs/dialog.jsindex.htmljs/operate/nip11.jsjs/operate/guide.jsjs/main.jsjs/operate/health.jsjs/operate/editor.jsjs/operate/room.jsjs/audit/findingsPrescriptions.jsjs/operate/autostage.jsjs/audit.jsjs/actions.jsjs/personalities.jsstyle.cssjs/audit/chartRender.js
**/*.html
⚙️ CodeRabbit configuration file
**/*.html: - Inline<script>blocks with runtime logic are a violation of the natural order. Initialization belongs injs/main.js. HTML is declarative. It does not think.
- Mode detection and URL parameter sniffing happen in JS modules. Not here.
- Interactive elements need accessible labels. Images need alt text. The Grand Marmot has patients with screen readers too.
Files:
index.html
js/audit/**/*.js
⚙️ CodeRabbit configuration file
js/audit/**/*.js: The diagnostic core. The Grand Marmot's stethoscope. Review these with the reverence they deserve.
- One function, one job. Anything doing two things is two functions pretending to be one.
40 lines is a symptom. Extract helpers — aggregation, formatting, extraction all deserve their own burrow chambers.
- Nostr kind numbers (0, 3, 443, 10002, 10051…) must be named constants. Magic numbers are for amateurs and rabbits.
- Audit results must return consistent shapes. Mixing null with valid objects gives the patient a wrong diagnosis.
- Every exported function gets JSDoc with
@paramand@returns. In the audit modules, this is not optional.
Files:
js/audit/findingsPrescriptions.jsjs/audit/chartRender.js
**/*.css
⚙️ CodeRabbit configuration file
**/*.css: - No duplicate selectors. Custom properties for repeated values. The Grand Marmot does not repeat himself, and neither should the stylesheet.
- Flexbox and grid over floats.
gapover margins.clamp()for fluid type. It is the current era.- Animations:
transformandopacityonly.will-changeis a scalpel, not a shovel.- No ID selectors for styling. Mobile-first media queries.
- Stylelint will complain about
value-keyword-caseon font family names like BlinkMacSystemFont or Comic Sans MS. Ignore it. Those are valid CSS. The linter is wrong. The marmot is right.
Files:
style.css
🧠 Learnings (5)
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use `say()` from `dialog.js` for user-facing error messages instead of alert or console
Applied to files:
js/dialog.jsjs/main.js
📚 Learning: 2026-03-04T12:34:03.627Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-04T12:34:03.627Z
Learning: Applies to **/*.js : Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Applied to files:
js/dialog.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Centralize all DOM queries in `dom.js`; import and use exported DOM element references instead of querying in other modules
Applied to files:
js/main.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Use the multi-phase audit flow orchestrated by main.js: extractUserRelays() → queryRelayForKinds() → validation stack → UI updates
Applied to files:
js/operate/editor.jsjs/audit/findingsPrescriptions.jsjs/operate/autostage.jsjs/audit.jsjs/actions.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use camelCase for function names with descriptive action verbs (e.g., `queryRelayForKinds()`, `validateRelayUrl()`, `isValidBase64()`)
Applied to files:
js/actions.js
🧬 Code graph analysis (10)
js/dom.js (1)
js/audit.js (1)
chartEl(413-413)
js/dialog.js (1)
js/dom.js (6)
dialogText(5-5)dialogText(5-5)dialogArrow(6-6)dialogArrow(6-6)dialogHint(7-7)dialogHint(7-7)
js/operate/nip11.js (1)
js/operate/room.js (1)
relayUrl(316-316)
js/operate/guide.js (3)
js/operate/room.js (2)
step(161-161)step(176-176)js/html.js (2)
html(30-39)escapeHtml(7-15)js/personalities.js (1)
speak(753-764)
js/main.js (1)
js/dom.js (4)
nip07Btn(2-2)nip07Btn(2-2)cancelBtn(8-8)cancelBtn(8-8)
js/operate/editor.js (5)
js/operate/autostage.js (3)
k10002Relays(125-125)k10002Relays(185-185)relays(66-66)js/operate/procedures.js (4)
relays(36-36)relays(60-60)relays(78-78)relays(99-99)js/operate/health.js (2)
healthScore(224-251)formatLatency(213-217)js/html.js (2)
html(30-39)escapeHtml(7-15)js/relay-validation.js (1)
shortUrl(40-42)
js/audit/findingsPrescriptions.js (2)
js/audit/chartRender.js (1)
params(208-226)js/audit.js (2)
auditCtx(612-641)categories(563-563)
js/audit.js (6)
js/audit/findingsPrescriptions.js (1)
lastAuditState(734-752)js/operate/room.js (2)
compiled(100-100)succeeded(417-417)js/audit/chartRender.js (2)
renderChart(207-281)cardHTML(255-278)js/dom.js (1)
getChartActionButtons(31-44)js/actions.js (6)
succeeded(59-59)succeeded(232-232)succeeded(406-406)succeeded(531-531)deleteOrphanedKeyPackages(297-423)deleteDeprecatedKind4(429-548)js/dialog.js (1)
setOnAllDone(33-35)
js/actions.js (4)
js/operate/room.js (1)
state(64-64)js/audit.js (1)
getAuditState(32-32)js/audit/relayBootstrap.js (1)
relaysToInvestigate(110-110)js/relay-panel.js (3)
setRelayState(29-51)appendResultSection(63-78)id(30-30)
js/audit/chartRender.js (4)
js/operate/guide.js (1)
doctorNote(146-147)js/html.js (2)
html(30-39)escapeHtml(7-15)js/audit/findingsPrescriptions.js (9)
prescriptions(523-523)canDeleteKps(613-613)canDeleteOrphanedKps(614-614)canDeleteKind4(615-615)findingsWithHints(719-732)findings(316-316)t(155-155)t(174-174)doctorNotes(709-716)js/audit.js (2)
canOperate(367-367)renderChart(369-387)
🔇 Additional comments (1)
js/operate/autostage.js (1)
25-55: Auto-staging pipeline looks clinically solid.The sequencing is deterministic and keeps mutation responsibility in the editor API, which makes the treatment plan predictable.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
style.css (2)
2397-2404:⚠️ Potential issue | 🟠 MajorUndefined CSS custom properties
--c-textand--c-panelleave styling on life support.
.doctor-note-textusesvar(--c-text)(line 2400),.guide-cardusesvar(--c-panel)(line 2471), and several guide elements reference--c-text. Neither is defined in:root. The browser discards these declarations, leaving text color and backgrounds undefined.Either define these tokens in
:rootor replace with existing tokens like--c-whiteand--c-panel-bg.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style.css` around lines 2397 - 2404, The CSS uses undefined custom properties --c-text and --c-panel (affecting .doctor-note-text and .guide-card), so either add definitions for those tokens in :root or swap references to existing tokens; update :root to define --c-text and --c-panel (e.g., mapping them to --c-white and --c-panel-bg) or replace var(--c-text) and var(--c-panel) in .doctor-note-text, .guide-card and other guide elements with existing tokens like var(--c-white) and var(--c-panel-bg) so the browser doesn't discard the color/background declarations.
1150-1153:⚠️ Potential issue | 🟡 MinorDuplicate
#nip07-btn:disabledselectors — the stylesheet has multiple personalities.Lines 1150-1153 define
opacityandcursor. Lines 1197-1202 redefine withbackground,color,border-color, andcursoragain. The cascade works today by accident. Merge them.Rx: Consolidate the patient's records
`#nip07-btn`:disabled { + opacity: 0.4; background: `#111130`; color: var(--c-dim); border-color: var(--c-dim); cursor: not-allowed; }Remove the earlier block at lines 1150-1153.
As per coding guidelines: "No duplicate selectors."
Also applies to: 1197-1202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style.css` around lines 1150 - 1153, There are duplicate CSS selectors for `#nip07-btn`:disabled; remove the earlier/first occurrence and consolidate all properties into a single rule so the final `#nip07-btn`:disabled contains opacity, cursor, background, color and border-color (merge the properties currently split between the two blocks) to avoid duplicate selectors and rely on a single definitive declaration.js/operate/guide.js (2)
87-94:⚠️ Potential issue | 🟡 Minor
nextStep()leavesisGuidedtrue after course completion.When the guide runs out of steps and returns
null, the flag stays up. Any UI checkingisGuidedMode()will still believe treatment is in progress.Rx: Discharge the patient properly
export function nextStep() { if (!isGuided) return null; currentStep++; if (currentStep >= steps.length) { + isGuided = false; return null; } return steps[currentStep]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/guide.js` around lines 87 - 94, nextStep() increments currentStep but never clears the isGuided flag when the guide finishes, so UI consumers (e.g., isGuidedMode()) still think guidance is active; update the nextStep() function so that when currentStep >= steps.length you set isGuided = false (and optionally clamp/reset currentStep to steps.length or 0 as desired) before returning null, referencing the existing symbols nextStep, currentStep, steps and isGuided/isGuidedMode().
78-81:⚠️ Potential issue | 🟡 MinorGuided mode flag remains
trueeven when no steps exist.When
prescriptionsis empty,steps.lengthis 0,currentStepbecomes -1, yetisGuidedis unconditionally set totrue. This leaves the patient in a phantom guided state — no steps, but the system thinks treatment is underway.Rx: Conditional activation
currentStep = steps.length > 0 ? 0 : -1; - isGuided = true; + isGuided = steps.length > 0; return steps.length;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/guide.js` around lines 78 - 81, The code sets currentStep and then unconditionally sets isGuided = true even when steps.length === 0; update the logic in the function that computes steps (use the existing symbols currentStep, steps, and isGuided) so isGuided is only true when steps.length > 0 (e.g., set isGuided = steps.length > 0 or explicitly set isGuided = false when steps.length === 0) so the guided flag reflects the presence of prescriptions/steps and avoids a phantom guided state.js/operate/room.js (4)
542-545:⚠️ Potential issue | 🟠 MajorUnsafe spread can abort surgery with TypeError.
If
auditState.relaysToInvestigateorauditState.marmotRelaysisnullorundefined, the spread operator throws. Mid-operation crash is not good bedside manner.Rx: Defensive array handling
const publishRelays = [...new Set([ - ...auditState.relaysToInvestigate, - ...auditState.marmotRelays, + ...(Array.isArray(auditState.relaysToInvestigate) ? auditState.relaysToInvestigate : []), + ...(Array.isArray(auditState.marmotRelays) ? auditState.marmotRelays : []), ])]; + + if (publishRelays.length === 0) { + await say('No relays available to publish changes.'); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/room.js` around lines 542 - 545, The spread of auditState.relaysToInvestigate and auditState.marmotRelays can throw if either is null/undefined; change the construction of publishRelays (the const publishRelays assignment in js/operate/room.js) to defensively handle those values by treating non-array/nullable entries as empty arrays (e.g., use fallback expressions or an Array.isArray check for auditState.relaysToInvestigate and auditState.marmotRelays before spreading) so the new Set receives only arrays and the operation won’t throw.
191-246:⚠️ Potential issue | 🔴 CriticalHealth check callbacks can resurrect the UI after discharge.
runHealthChecks()continues executing afterawait checkAllRelaysHealth(...)even ifexitOperatingRoom()was called mid-check. Line 245 callsrenderRoom()which repopulatesrelayList, potentially overwriting restored chart content.Introduce a session token incremented on entry/exit. Guard all post-await mutations.
Rx: Session token pattern
let isOperating = false; let healthResults = null; +let operatingSessionId = 0; export async function enterOperatingRoom() { + const sessionId = ++operatingSessionId; // ... existing code ... - runHealthChecks(state); + runHealthChecks(state, sessionId); } export function exitOperatingRoom() { + operatingSessionId++; // Invalidate any running session isOperating = false; // ... } -async function runHealthChecks(auditState) { +async function runHealthChecks(auditState, sessionId) { // ... after await checkAllRelaysHealth ... + if (!isOperating || sessionId !== operatingSessionId) return; // ... continue with mutations ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/room.js` around lines 191 - 246, runHealthChecks continues after async checkAllRelaysHealth even if the room was exited; add a session token to guard post-await mutations: increment a shared sessionToken whenever entering or exiting the operating room (ensure exitOperatingRoom increments it), capture const mySession = sessionToken at the top of runHealthChecks before any awaits, then after each await (especially after await checkAllRelaysHealth and any other yields) check if (mySession !== sessionToken) return; before performing any UI mutations (setScanProgress, removeScanBar, say, renderRoom, etc.) so late callbacks cannot resurrect the UI; update references to sessionToken, runHealthChecks, checkAllRelaysHealth, exitOperatingRoom, and renderRoom when implementing.
464-494:⚠️ Potential issue | 🟠 MajorUnknown fix actions silently "succeed" — the patient thinks surgery happened.
executeFixActionlogs unknown actions (line 492) but returns normally. The caller setssucceeded = trueand shows "DONE ✔". Auto-advancement proceeds for fixes that never occurred.The function should return a boolean or throw on unknown actions.
Rx: Explicit success contract
async function executeFixAction(fixAction) { switch (fixAction) { case 'rebroadcast': { const { rebroadcastProfileAndContacts } = await import('../actions.js'); await rebroadcastProfileAndContacts(); - break; + return true; } // ... other cases with return true ... default: console.error('Unknown fix action:', fixAction); + return false; } }Then in the caller:
- await executeFixAction(fixAction); - succeeded = true; + succeeded = await executeFixAction(fixAction);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/room.js` around lines 464 - 494, The executeFixAction function currently swallows unknown actions by logging and returning normally, causing callers (which set succeeded = true) to believe the fix ran; change executeFixAction to have an explicit success contract by either throwing an Error for unknown actions or returning a boolean success value: for each known case (rebroadcast, delete-kps, delete-orphaned-kps, unify-relays, delete-kind4) return true after awaiting the imported action, and in the default branch throw a descriptive Error (or return false). Then update the caller that uses succeeded to await the boolean result (e.g., succeeded = await executeFixAction(...)) or catch the thrown Error and set succeeded = false so the UI/auto-advance only proceeds on real success.
178-179: 🧹 Nitpick | 🔵 TrivialDOM queries outside
dom.js— trespassing in the burrow.
relayList.querySelector(...)appears here and at lines 254-257, 314-315, 503-504. Per the Laws of the Burrow, DOM access should be centralized indom.js.For dynamically rendered content, consider exporting helper functions from
dom.jslikegetTrayByKind(container, kind)rather than querying directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/room.js` around lines 178 - 179, Replace direct DOM queries from operate/room.js with a dom.js helper: create and export a function like getTrayByKind(container, kind) in dom.js that performs container.querySelector(`.or-tray[data-kind="${kind}"]`) and use it wherever relayList.querySelector(...) is used (e.g., the occurrences that assign tray from relayList and similar blocks referencing step.targetKind). Update the code to call getTrayByKind(relayList, step.targetKind) (or getTrayByKind(someContainer, kind)) instead of inline querySelector calls, and ensure dom.js exports and imports are updated accordingly.js/audit/chartRender.js (1)
160-163:⚠️ Potential issue | 🟠 Major
reduce-relaysaction renders OPERATE button withoutcanOperateguard.When NIP-07 is present but the user hasn't signed in via NIP-07,
canOperateisfalseyet this button remains active. The Operating Room entry should require both conditions.Past review flagged this exact bypass. The side door remains ajar.
Rx: Gate the entrance properly
- } else if (action === 'reduce-relays') { + } else if (action === 'reduce-relays' && canOperate) { actionButton = html` <button class="rx-action-btn" data-action="enter-operating-room"${requiresNip07Attrs}>⚕ OPERATE</button> `; + } else if (action === 'reduce-relays' && !canOperate) { + actionButton = html` + <button class="rx-action-btn" disabled title="Sign in with NIP-07 to operate">⚕ OPERATE</button> + `; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/audit/chartRender.js` around lines 160 - 163, The 'reduce-relays' branch in chartRender.js renders the OPERATE button using the template that includes requiresNip07Attrs but does not honor the canOperate guard; update the rendering logic in the same block where action === 'reduce-relays' so it either (a) only inserts the <button class="rx-action-btn" data-action="enter-operating-room"...> when canOperate is true, or (b) when canOperate is false render a disabled/removed-action variant (no data-action and disabled attribute or distinct class) so the NIP-07 gate cannot be bypassed; reference the action string 'reduce-relays', the template construction for actionButton, the requiresNip07Attrs variable and the canOperate flag to locate and change the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/audit/chartRender.js`:
- Around line 75-86: The rendering uses effect.hint with escapeHtml but outputs
effect.text raw, creating an XSS risk; update the rows mapping in the rows
constant so the span that currently injects ${effect.text} instead injects the
escaped value via escapeHtml(effect.text) (ensuring escapeHtml is available
where chartRender.js defines rows) while keeping the existing html template and
hint escaping logic intact.
In `@js/nak-commands.js`:
- Around line 25-26: relayArgs currently joins relaysToInvestigate directly
which can produce shell-injectionable strings (e.g., relay URLs containing ; or
backticks); update the construction of relayArgs and sourceRelay so each URL is
safely quoted/escaped before joining/using: iterate relaysToInvestigate and
replace any single-quote characters in each URL (or otherwise escape
shell-special chars), then wrap each URL in single quotes (e.g., map URL -> "'"
+ escapedUrl + "'") before calling join, and set sourceRelay to the
similarly-escaped first entry (falling back to a safely-quoted
'wss://relay.damus.io'); reference relayArgs, sourceRelay and
relaysToInvestigate when implementing this change.
---
Duplicate comments:
In `@js/audit/chartRender.js`:
- Around line 160-163: The 'reduce-relays' branch in chartRender.js renders the
OPERATE button using the template that includes requiresNip07Attrs but does not
honor the canOperate guard; update the rendering logic in the same block where
action === 'reduce-relays' so it either (a) only inserts the <button
class="rx-action-btn" data-action="enter-operating-room"...> when canOperate is
true, or (b) when canOperate is false render a disabled/removed-action variant
(no data-action and disabled attribute or distinct class) so the NIP-07 gate
cannot be bypassed; reference the action string 'reduce-relays', the template
construction for actionButton, the requiresNip07Attrs variable and the
canOperate flag to locate and change the code.
In `@js/operate/guide.js`:
- Around line 87-94: nextStep() increments currentStep but never clears the
isGuided flag when the guide finishes, so UI consumers (e.g., isGuidedMode())
still think guidance is active; update the nextStep() function so that when
currentStep >= steps.length you set isGuided = false (and optionally clamp/reset
currentStep to steps.length or 0 as desired) before returning null, referencing
the existing symbols nextStep, currentStep, steps and isGuided/isGuidedMode().
- Around line 78-81: The code sets currentStep and then unconditionally sets
isGuided = true even when steps.length === 0; update the logic in the function
that computes steps (use the existing symbols currentStep, steps, and isGuided)
so isGuided is only true when steps.length > 0 (e.g., set isGuided =
steps.length > 0 or explicitly set isGuided = false when steps.length === 0) so
the guided flag reflects the presence of prescriptions/steps and avoids a
phantom guided state.
In `@js/operate/room.js`:
- Around line 542-545: The spread of auditState.relaysToInvestigate and
auditState.marmotRelays can throw if either is null/undefined; change the
construction of publishRelays (the const publishRelays assignment in
js/operate/room.js) to defensively handle those values by treating
non-array/nullable entries as empty arrays (e.g., use fallback expressions or an
Array.isArray check for auditState.relaysToInvestigate and
auditState.marmotRelays before spreading) so the new Set receives only arrays
and the operation won’t throw.
- Around line 191-246: runHealthChecks continues after async
checkAllRelaysHealth even if the room was exited; add a session token to guard
post-await mutations: increment a shared sessionToken whenever entering or
exiting the operating room (ensure exitOperatingRoom increments it), capture
const mySession = sessionToken at the top of runHealthChecks before any awaits,
then after each await (especially after await checkAllRelaysHealth and any other
yields) check if (mySession !== sessionToken) return; before performing any UI
mutations (setScanProgress, removeScanBar, say, renderRoom, etc.) so late
callbacks cannot resurrect the UI; update references to sessionToken,
runHealthChecks, checkAllRelaysHealth, exitOperatingRoom, and renderRoom when
implementing.
- Around line 464-494: The executeFixAction function currently swallows unknown
actions by logging and returning normally, causing callers (which set succeeded
= true) to believe the fix ran; change executeFixAction to have an explicit
success contract by either throwing an Error for unknown actions or returning a
boolean success value: for each known case (rebroadcast, delete-kps,
delete-orphaned-kps, unify-relays, delete-kind4) return true after awaiting the
imported action, and in the default branch throw a descriptive Error (or return
false). Then update the caller that uses succeeded to await the boolean result
(e.g., succeeded = await executeFixAction(...)) or catch the thrown Error and
set succeeded = false so the UI/auto-advance only proceeds on real success.
- Around line 178-179: Replace direct DOM queries from operate/room.js with a
dom.js helper: create and export a function like getTrayByKind(container, kind)
in dom.js that performs container.querySelector(`.or-tray[data-kind="${kind}"]`)
and use it wherever relayList.querySelector(...) is used (e.g., the occurrences
that assign tray from relayList and similar blocks referencing step.targetKind).
Update the code to call getTrayByKind(relayList, step.targetKind) (or
getTrayByKind(someContainer, kind)) instead of inline querySelector calls, and
ensure dom.js exports and imports are updated accordingly.
In `@style.css`:
- Around line 2397-2404: The CSS uses undefined custom properties --c-text and
--c-panel (affecting .doctor-note-text and .guide-card), so either add
definitions for those tokens in :root or swap references to existing tokens;
update :root to define --c-text and --c-panel (e.g., mapping them to --c-white
and --c-panel-bg) or replace var(--c-text) and var(--c-panel) in
.doctor-note-text, .guide-card and other guide elements with existing tokens
like var(--c-white) and var(--c-panel-bg) so the browser doesn't discard the
color/background declarations.
- Around line 1150-1153: There are duplicate CSS selectors for
`#nip07-btn`:disabled; remove the earlier/first occurrence and consolidate all
properties into a single rule so the final `#nip07-btn`:disabled contains opacity,
cursor, background, color and border-color (merge the properties currently split
between the two blocks) to avoid duplicate selectors and rely on a single
definitive declaration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac590fd9-b657-4393-a6e8-8b2257f43599
📒 Files selected for processing (6)
js/audit.jsjs/audit/chartRender.jsjs/nak-commands.jsjs/operate/guide.jsjs/operate/room.jsstyle.css
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESMimportstatements from CDN (nostr-tools via https://esm.sh/nostr-tools) or local modules only; no npm dependencies
Order imports as: Standard library → External → Local, with one blank line between groups
Use 4 spaces for indentation (not tabs)
Enforce soft line length limit of 100 characters and hard limit of 120 characters
Require semicolons at the end of statements
Use trailing commas in multi-line objects and arrays
Use camelCase for function names with descriptive action verbs (e.g.,queryRelayForKinds(),validateRelayUrl(),isValidBase64())
Use camelCase for variable names (e.g.,lastAuditState,isAuditing,relayUrl,pubkey)
Use UPPER_SNAKE_CASE for constants, with exception for exported object/map constants which use camelCase keys
Prefix boolean variables and functions withis,has,should, orcan(e.g.,isAuditing,hasError,isValidBase64())
Use JSDoc for function signatures to document parameters and return types
Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions likevalidateRelayUrl(),isValidBase64(),pubkeyToHex()
Use try-catch with console.error logging for error handling; never throw from module exports—return null/empty/error object instead
Usesay()fromdialog.jsfor user-facing error messages instead of alert or console
Export either one default export OR multiple named exports per file, not both
Define functions at module scope (no fake 'classes' or IIFE wrappers)
Centralize all DOM queries indom.js; import and use exported DOM element references instead of querying in other modules
Use Promise constructor or async/await for async patterns; wrap pool subscriptions in setTimeout to avoid hanging connections
Prefix unused function parameters with_so they are ignored by the no-unused-vars linter rule
**/*.js: Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Warn if 'throw new Error' is used dire...
Files:
js/audit.jsjs/nak-commands.jsjs/audit/chartRender.jsjs/operate/room.jsjs/operate/guide.js
⚙️ CodeRabbit configuration file
**/*.js: Vanilla JS without build steps or frameworks.Security violations: flag without mercy.
Style issues: flag only if it genuinely improves things — the Grand Marmot has no patience for noise.Max 5 comments per file, 20 per PR. Choose wisely. Every missed carrot is a missed carrot. And we all know how rabbits behave when they're off their carrots.
The Laws of the Burrow:
- DOM access lives in
dom.jsand nowhere else.document.querySelectorin any other file is a trespass.- Side effects belong only in
main.js. Other modules export. That is all they do.- One export style per file. Named or default. The Grand Marmot does not negotiate on this.
- User messages go through
say()fromdialog.js.alert()is forbidden. It has always been forbidden.- Never throw from a module export. Return null, return empty, return an error object — but do not throw.
- Nostr pool subscriptions must be closed in
oneoseor asetTimeoutfallback. Leaks are how tunnels collapse.- External imports:
https://esm.sh/nostr-toolsonly. Everything else lives here, in the burrow.constis the law.letwhen you must.varis exile.===always.==is a war crime that the Grand Marmot has personally prosecuted.- Booleans must be prefixed:
is,has,should,can. No exceptions.- JSDoc on complex exported functions. Trivial helpers and one-liners need not apply.
Files:
js/audit.jsjs/nak-commands.jsjs/audit/chartRender.jsjs/operate/room.jsjs/operate/guide.js
!main.js,**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Avoid side effects on import; only
main.jsis allowed to have side effects for initialization
Files:
js/audit.jsjs/nak-commands.jsjs/audit/chartRender.jsstyle.cssjs/operate/room.jsjs/operate/guide.js
js/audit/**/*.js
⚙️ CodeRabbit configuration file
js/audit/**/*.js: The diagnostic core. The Grand Marmot's stethoscope. Review these with the reverence they deserve.
- One function, one job. Anything doing two things is two functions pretending to be one.
40 lines is a symptom. Extract helpers — aggregation, formatting, extraction all deserve their own burrow chambers.
- Nostr kind numbers (0, 3, 443, 10002, 10051…) must be named constants. Magic numbers are for amateurs and rabbits.
- Audit results must return consistent shapes. Mixing null with valid objects gives the patient a wrong diagnosis.
- Every exported function gets JSDoc with
@paramand@returns. In the audit modules, this is not optional.
Files:
js/audit/chartRender.js
**/*.css
⚙️ CodeRabbit configuration file
**/*.css: - No duplicate selectors. Custom properties for repeated values. The Grand Marmot does not repeat himself, and neither should the stylesheet.
- Flexbox and grid over floats.
gapover margins.clamp()for fluid type. It is the current era.- Animations:
transformandopacityonly.will-changeis a scalpel, not a shovel.- No ID selectors for styling. Mobile-first media queries.
- Stylelint will complain about
value-keyword-caseon font family names like BlinkMacSystemFont or Comic Sans MS. Ignore it. Those are valid CSS. The linter is wrong. The marmot is right.
Files:
style.css
🧠 Learnings (6)
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Use the multi-phase audit flow orchestrated by main.js: extractUserRelays() → queryRelayForKinds() → validation stack → UI updates
Applied to files:
js/audit.jsjs/operate/room.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Centralize all DOM queries in `dom.js`; import and use exported DOM element references instead of querying in other modules
Applied to files:
js/operate/room.js
📚 Learning: 2026-03-04T12:34:03.627Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-04T12:34:03.627Z
Learning: Applies to **/*.js !(**/dom.js) : Warn if DOM query methods (like document.querySelector) are used in JavaScript files other than dom.js. DOM queries should be centralized in dom.js
Applied to files:
js/operate/room.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use camelCase for function names with descriptive action verbs (e.g., `queryRelayForKinds()`, `validateRelayUrl()`, `isValidBase64()`)
Applied to files:
js/operate/room.js
📚 Learning: 2026-03-04T12:34:03.627Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-04T12:34:03.627Z
Learning: Applies to **/*.js : Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Applied to files:
js/operate/room.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use `say()` from `dialog.js` for user-facing error messages instead of alert or console
Applied to files:
js/operate/room.js
🧬 Code graph analysis (2)
js/nak-commands.js (1)
js/html.js (1)
escapeHtml(7-15)
js/audit/chartRender.js (5)
js/operate/guide.js (2)
doctorNote(151-152)nakCmd(168-168)js/html.js (2)
html(30-39)escapeHtml(7-15)js/audit/findingsPrescriptions.js (9)
prescriptions(523-523)canDeleteKps(613-613)canDeleteOrphanedKps(614-614)canDeleteKind4(615-615)findingsWithHints(719-732)findings(316-316)t(155-155)t(174-174)doctorNotes(709-716)js/audit.js (2)
canOperate(367-367)renderChart(369-388)js/nak-commands.js (2)
getNakCommand(19-42)renderNakCommandBlock(155-166)
🔇 Additional comments (9)
js/operate/guide.js (2)
1-26: LGTM — clean module scaffolding.Imports are properly structured, typedef is well-documented, and module state is appropriately initialized. The Grand Marmot approves this surgical prep.
136-206: Solid rendering with proper sanitization.
escapeHtml(step.prescription)on line 190 prevents XSS from user-provided prescription text. The nak command integration is cleanly gated. No complaints from the burrow.js/nak-commands.js (1)
155-166: Clean HTML rendering with proper escaping.Both
nakCmd.descriptionandnakCmd.cmdpass throughescapeHtmlbefore DOM insertion. No XSS vector here. The marmot nods approvingly.js/audit/chartRender.js (2)
225-300: ExtendedrenderChartsignature properly integrates new features.The optional
speakFnparameter, hinted findings, and new action capabilities are cleanly wired. The audit state propagation enables nak command generation. Surgical precision achieved.
57-68: No XSS risk here.noteVarscontains only numeric counts (passCount,relayCount,warnCount,failCount,categoryCount) and booleans (isRelayDivergence,isBloated,hasRelayConfig, etc.) — all derived from internal audit logic, not user input.fmt()lacks escaping, but has nothing unsafe to escape. This code is clean.> Likely an incorrect or invalid review comment.js/audit.js (3)
352-388: Audit state extension and chart rendering integration look healthy.
canOperatecorrectly gates on both NIP-07 presence and sign-in state. The compiled result attachment enables downstream modules to access findings context. Clean surgical prep for the Operating Room.
417-435: Nak toggle and copy handlers cleanly wired.The delegated click handling on
chartElproperly scopes queries to the closest wrapper. Clipboard feedback with temporary "COPIED ✔" is a nice UX touch. No issues.
550-566: Operating Room entry handler follows established patterns.Dynamic import, loading state, error recovery with button re-enablement — consistent with the other action handlers. The "PREPPING…" text adds thematic continuity.
js/operate/room.js (1)
63-78: Entry guards properly validate prerequisites.Checks for audit state, NIP-07 presence, and NIP-07 sign-in status before allowing OR entry. User-facing messages via
say()explain each rejection. The Grand Marmot's security posture is maintained.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
js/operate/editor.js (1)
171-179:⚠️ Potential issue | 🟠 MajorDiscard still restores the wrong read/write baseline after repeated toggles.
Line 171 captures
prevReadWritefrom the current value, then Line 176 replaces the prior modify record. Afterrw → read → write, discard restoresread, not originalrw. That’s a lingering suture.Suggested fix
export function stageToggleReadWrite(url, newReadWrite) { @@ - const prevReadWrite = entry.readWrite; - entry.readWrite = newReadWrite; - - // Remove any existing modify change for this URL - const existingIdx = changes.findIndex(c => c.type === 'modify' && c.url.toLowerCase() === trimmed.toLowerCase()); - if (existingIdx !== -1) changes.splice(existingIdx, 1); - - changes.push({ type: 'modify', url: trimmed, readWrite: newReadWrite, prevReadWrite }); + const existingIdx = changes.findIndex( + c => c.type === 'modify' && c.url.toLowerCase() === trimmed.toLowerCase(), + ); + + if (existingIdx !== -1) { + const original = changes[existingIdx].prevReadWrite; + entry.readWrite = newReadWrite; + if (newReadWrite === original) { + changes.splice(existingIdx, 1); + } else { + changes[existingIdx].readWrite = newReadWrite; + } + stagedChanges.set(10002, changes); + return { ok: true }; + } + + const prevReadWrite = entry.readWrite; + entry.readWrite = newReadWrite; + changes.push({ + type: 'modify', + url: trimmed, + readWrite: newReadWrite, + prevReadWrite, + }); stagedChanges.set(10002, changes); return { ok: true }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/editor.js` around lines 171 - 179, The code currently captures prevReadWrite from the current entry.readWrite, which gets updated on each toggle and causes discard to restore an intermediate state; instead, when preparing the new modify change for a given URL (the block using entry.readWrite, prevReadWrite, changes.findIndex, changes.splice and stagedChanges.set(10002,...)), first look for an existing modify record in changes for that URL and, if found, reuse that record's prevReadWrite as the baseline; only if no existing modify exists fall back to entry.readWrite. Then replace or push the modify entry keeping prevReadWrite from the original modify record so discard restores the true original state.js/operate/guide.js (1)
125-129:⚠️ Potential issue | 🟡 Minor
resumeGuide()can re-enable guided mode with no valid step.If the course is already finished (
currentStep >= steps.length), Lines 126-129 still setisGuided = true. That leaves guided mode “alive” whilegetCurrentStep()returns null.Suggested fix
export function resumeGuide() { - if (steps.length > 0) { - isGuided = true; - if (currentStep < 0) currentStep = 0; - } + if (steps.length === 0) return; + if (currentStep >= steps.length) { + isGuided = false; + return; + } + isGuided = true; + if (currentStep < 0) currentStep = 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/guide.js` around lines 125 - 129, resumeGuide() currently enables guided mode whenever steps.length > 0 even if currentStep is past the end, leaving isGuided=true while getCurrentStep() returns null; change the guard so you only set isGuided = true when steps.length > 0 AND currentStep < steps.length (and still normalize negative currentStep to 0), i.e. check currentStep < steps.length before setting isGuided and adjusting currentStep in the resumeGuide function.js/dialog.js (1)
53-58:⚠️ Potential issue | 🔴 Critical
innerHTMLrendering is still an injection site; sanitize before writing.The dialog still writes dynamic message content to
innerHTMLin two paths. If upstream text carries tags, this is executable markup, not bedside manners.🛡️ Suggested fix (allowlist sanitizer)
+function sanitizeDialogHtml(content) { + const tpl = document.createElement('template'); + tpl.innerHTML = String(content ?? ''); + const allowedTags = new Set(['SPAN', 'EM', 'STRONG', 'BR']); + + tpl.content.querySelectorAll('*').forEach((el) => { + if (!allowedTags.has(el.tagName)) { + el.replaceWith(document.createTextNode(el.textContent || '')); + return; + } + [...el.attributes].forEach((attr) => { + if (attr.name !== 'class') { + el.removeAttribute(attr.name); + } + }); + }); + return tpl.innerHTML; +} + function _appendToLog(content, isHtml = false) { @@ if (isHtml) { - entry.innerHTML = content; + entry.innerHTML = sanitizeDialogHtml(content); @@ if (lastEntry && !text.includes('<')) { lastEntry.textContent = text; } else if (lastEntry) { - lastEntry.innerHTML = text; + lastEntry.innerHTML = sanitizeDialogHtml(text); }Also applies to: 80-84, 153-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/dialog.js` around lines 53 - 58, The code currently writes untrusted HTML into the DOM via innerHTML in _appendToLog and the other similar locations (around the other innerHTML uses), creating an XSS risk; instead either set textContent when content is plain text or run the content through a strict allowlist sanitizer (e.g., DOMPurify configured to only allow safe tags/attributes) before assigning to innerHTML; update _appendToLog and the other functions that set innerHTML (the other two innerHTML sites noted) to detect when input is untrusted and sanitize or use textContent accordingly and ensure any trusted-html code paths are clearly documented.js/operate/room.js (3)
267-268: 🛠️ Refactor suggestion | 🟠 MajorMove remaining selector lookups into
dom.js; this room is still querying the walls directly.
relayList.querySelectorAll(...)/querySelector(...)remain in this module. The Burrow Law says DOM querying belongs indom.jsonly.🧭 Refactor direction
-import { relayList, getTrayByKind, getAddBtnByKind } from '../dom.js'; +import { + relayList, + getTrayByKind, + getAddBtnByKind, + getAddInputs, + getAddInputByKind, + getAddErrorByKind, + getNakToggleContainer, + getNakCodeElement, +} from '../dom.js'; -const inputs = relayList.querySelectorAll('.or-add-input'); +const inputs = getAddInputs(relayList); -const input = relayList.querySelector(`.or-add-input[data-kind="${kind}"]`); -const errorEl = relayList.querySelector(`.or-add-error[data-kind="${kind}"]`); +const input = getAddInputByKind(relayList, kind); +const errorEl = getAddErrorByKind(relayList, kind); -const container = btn.closest('.guide-card, .rx-row-wrap')?.querySelector('.guide-nak-container, .rx-nak-container'); +const container = getNakToggleContainer(btn); -const pre = btn.closest('.nak-cmd-block')?.querySelector('.nak-cmd-pre code'); +const pre = getNakCodeElement(btn);Based on learnings, "Applies to /*.js !(/dom.js) : Warn if DOM query methods (like document.querySelector) are used in JavaScript files other than dom.js. DOM queries should be centralized in dom.js".
Also applies to: 327-328, 427-427, 432-432
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/room.js` around lines 267 - 268, This module still performs DOM queries (e.g., relayList.querySelectorAll('.or-add-input') and other querySelector/querySelectorAll uses at the noted locations); move those selector lookups into dom.js by adding and exporting focused helper functions (for example: getRelayAddInputs(container) that wraps container.querySelectorAll('.or-add-input'), getRelayNode(container, selector) or similarly named helpers for the other selectors used at the 327–328, 427 and 432 sites), then import and call those helpers from js/operate/room.js in place of direct querySelector/querySelectorAll calls (replace relayList.querySelectorAll(...) and other direct queries with the new dom.js helpers). Ensure names are unique and descriptive (e.g., getRelayAddInputs, findRelayButton, findRelayField) so callers in room.js can be updated cleanly.
221-240:⚠️ Potential issue | 🟠 MajorGuard diagnostics with
try/catch/finally; a thrown relay probe can leave the OR monitors hanging.
checkAllRelaysHealth(...)is awaited without a failure path. If it rejects, cleanup and user feedback are skipped, and the async error bubbles out.🩺 Suggested fix
- const results = await checkAllRelaysHealth( - [...allUrls], - (_url, phase) => { - if (session !== orSessionId) return; - if (phase === 'connecting') scanBeep(); - }, - (_result) => { - if (session !== orSessionId) return; - checked++; - const pct = Math.round(10 + (80 * checked / total)); - setScanProgress(pct); - }, - ); + let results; + try { + results = await checkAllRelaysHealth( + [...allUrls], + (_url, phase) => { + if (session !== orSessionId) return; + if (phase === 'connecting') scanBeep(); + }, + (_result) => { + if (session !== orSessionId) return; + checked++; + const pct = Math.round(10 + (80 * checked / total)); + setScanProgress(pct); + }, + ); + } catch (err) { + console.error('Health diagnostics failed', err); + if (session === orSessionId) { + await say(speak('orHealthFail') || 'Relay diagnostics failed.'); + } + return; + } finally { + if (session === orSessionId) { + removeScanBar(); + } + } // Bail if OR was closed during health checks if (session !== orSessionId) return; healthResults = results; setScanProgress(100); - removeScanBar();As per coding guidelines, "Use try-catch with console.error logging for error handling; never throw from module exports—return null/empty/error object instead".
Also applies to: 255-258
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/room.js` around lines 221 - 240, Wrap the await checkAllRelaysHealth(...) call in a try/catch/finally block so any rejection is caught, logged, and cleanup always runs; inside try assign results to healthResults and setScanProgress(100) as now, in catch console.error the error and set healthResults to an empty/failure value (or null) and setScanProgress(0) or an error state to inform the UI, and in finally always call removeScanBar() and bail early if session !== orSessionId. Ensure the same pattern is applied to the later block around lines 255-258 so monitors and UI are never left hanging.
555-582:⚠️ Potential issue | 🟠 MajorHarden surgery execution: empty publish targets and thrown procedure runs should not leave the UI in limbo.
Two hazards here:
publishRelayscan be empty and still proceed.executeAllProcedures(...)has no catch/finally, so failures can strand progress UI and skip a patient-facing error.🛠️ Suggested fix
const publishRelays = [...new Set([ ...(Array.isArray(auditState.relaysToInvestigate) ? auditState.relaysToInvestigate : []), ...(Array.isArray(auditState.marmotRelays) ? auditState.marmotRelays : []), ])]; - const results = await executeAllProcedures( + if (publishRelays.length === 0) { + removeScanBar(); + errBeep(); + await say(speak('orNoPublishRelays') || 'No publish relays available.'); + return; + } + + let results; + try { + results = await executeAllProcedures( publishRelays, async (procedure, index, total) => { const pct = Math.round(5 + (85 * index / total)); setScanProgress(pct); await say(speak('orProcedure', { index: index + 1, total, name: procedure.name })); @@ (relay, status) => { const cssState = status === 'SENDING' ? 'connecting' : status === 'OK' ? 'ok' : 'error'; setRelayState(relay, cssState, status); }, - ); + ); + } catch (err) { + console.error('Operating Room execution failed', err); + errBeep(); + await say(speak('orOperateFail') || 'Operation failed before completion.'); + return; + } finally { + removeScanBar(); + } setScanProgress(100); - removeScanBar();As per coding guidelines, "Use try-catch with console.error logging for error handling" and "Use
say()fromdialog.jsfor user-facing error messages instead of alert or console".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/room.js` around lines 555 - 582, Check for an empty publishRelays before calling executeAllProcedures and abort early (e.g., setScanProgress(100) or reset UI) to avoid running with no targets, and wrap the executeAllProcedures(...) invocation in a try-catch-finally: in try keep the existing callbacks (passing publishRelays, the progress/success/failure handlers and setRelayState), in catch console.error the thrown error and call await say(speak('orProcedureError', { error: String(err) })) to surface a user-facing message, and in finally ensure the UI is left in a terminal state by calling setScanProgress(100) (or resetting progress) and any necessary cleanup of relay state via setRelayState so failures do not leave the UI in limbo; reference publishRelays, executeAllProcedures, setScanProgress, say, speak, and setRelayState when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/actions.js`:
- Around line 515-527: The current approach builds a single kind:5 unsigned
event (unsigned) containing tags: [...kind4Ids].map(id => ['e', id]) which can
exceed relay payload limits; instead, chunk kind4Ids into smaller batches (e.g.,
50–100 ids per batch) and for each chunk create its own unsigned event, call
window.nostr.signEvent(unsigned) for each chunk, and push each signed event into
deleteEvents; ensure you preserve created_at, pubkey, content and the tags
mapping logic for each chunk and handle signEvent errors individually to avoid a
single silent failure from stopping the whole cleanup.
- Around line 491-494: The loop advances the cursor by doing until = oldest - 1
which skips events that have the same created_at as oldest when the page is
full; instead, stop subtracting 1 and implement a tie-breaker: keep until =
oldest and also record the set of IDs for events with that timestamp (e.g.,
lastPageIds), then on the next fetch exclude any events whose id is in
lastPageIds (or fetch using a composite cursor (created_at,id) and fetch events
where created_at < oldest OR (created_at == oldest AND id < lastId)); modify the
code around the reduce that computes oldest and the paging logic that calls the
fetch so it uses this composite/tie-breaker approach with symbols page, oldest,
until and the event id set to avoid dropping same-timestamp events.
In `@js/audit/chartRender.js`:
- Around line 57-67: The buildDoctorNotesSection is inserting doctorNote raw
into the template; modify it to pass doctorNote through the existing escapeHtml
helper before interpolation (e.g., use escapeHtml(doctorNote)) and do the same
for rxText wherever it is rendered (the prescription rendering blocks referenced
around the other ranges). Locate the templates that render doctorNote and rxText
(e.g., buildDoctorNotesSection and the prescription rendering functions/blocks)
and replace direct interpolation with escaped values so all dynamic audit text
is sanitized before being injected into the HTML template.
In `@js/dialog.js`:
- Line 153: Replace the direct DOM query in dialog.js that defines lastEntry via
dialogText.querySelector('.dialog-msg:last-child') by using an exported DOM
reference or helper from dom.js (e.g., import the dialog container or a
getLastDialogEntry/getLastChildOfDialog function) so all DOM access is
centralized; update dialog.js to import the appropriate symbol from dom.js
(instead of running querySelector) and use that exported reference/function to
obtain the last dialog entry.
In `@js/nak-commands.js`:
- Around line 59-60: The generated CLI tokens (e.g., when building eTags from
missingITagIds and the cmd string) are inserted raw and must be validated and
quoted: validate each id/pubkey with the appropriate validators (isValidBase64()
for base64 IDs, pubkeyToHex()/isValidPubkey() for pubkeys, validateRelayUrl()
for relays), reject or sanitize invalid values, and wrap every dynamic token in
single quotes (escaping any embedded single quotes) when constructing eTags and
the cmd. Apply the same fixes to the other places that build command-line args
(the other command-builder blocks referenced in the review) so all injected
tokens are validated and single-quoted before concatenation.
In `@js/operate/guide.js`:
- Around line 291-305: The clientPatterns array currently includes the string
"delete events", which flags prescriptions as CLIENT-only while your
prescription mapper returns the "delete-kps" family that has an auto-fix,
causing a mixed CLIENT + FIX IT UI. Remove "delete events" from the
clientPatterns array (or alternatively move its handling to a mapping that
returns a non-CLIENT classification) so the classification aligns with the
mapper's "delete-kps" auto-fix; locate the clientPatterns constant and the
lower.includes(p) check in js/operate/guide.js and adjust accordingly to keep
classification consistent with the prescription family.
---
Duplicate comments:
In `@js/dialog.js`:
- Around line 53-58: The code currently writes untrusted HTML into the DOM via
innerHTML in _appendToLog and the other similar locations (around the other
innerHTML uses), creating an XSS risk; instead either set textContent when
content is plain text or run the content through a strict allowlist sanitizer
(e.g., DOMPurify configured to only allow safe tags/attributes) before assigning
to innerHTML; update _appendToLog and the other functions that set innerHTML
(the other two innerHTML sites noted) to detect when input is untrusted and
sanitize or use textContent accordingly and ensure any trusted-html code paths
are clearly documented.
In `@js/operate/editor.js`:
- Around line 171-179: The code currently captures prevReadWrite from the
current entry.readWrite, which gets updated on each toggle and causes discard to
restore an intermediate state; instead, when preparing the new modify change for
a given URL (the block using entry.readWrite, prevReadWrite, changes.findIndex,
changes.splice and stagedChanges.set(10002,...)), first look for an existing
modify record in changes for that URL and, if found, reuse that record's
prevReadWrite as the baseline; only if no existing modify exists fall back to
entry.readWrite. Then replace or push the modify entry keeping prevReadWrite
from the original modify record so discard restores the true original state.
In `@js/operate/guide.js`:
- Around line 125-129: resumeGuide() currently enables guided mode whenever
steps.length > 0 even if currentStep is past the end, leaving isGuided=true
while getCurrentStep() returns null; change the guard so you only set isGuided =
true when steps.length > 0 AND currentStep < steps.length (and still normalize
negative currentStep to 0), i.e. check currentStep < steps.length before setting
isGuided and adjusting currentStep in the resumeGuide function.
In `@js/operate/room.js`:
- Around line 267-268: This module still performs DOM queries (e.g.,
relayList.querySelectorAll('.or-add-input') and other
querySelector/querySelectorAll uses at the noted locations); move those selector
lookups into dom.js by adding and exporting focused helper functions (for
example: getRelayAddInputs(container) that wraps
container.querySelectorAll('.or-add-input'), getRelayNode(container, selector)
or similarly named helpers for the other selectors used at the 327–328, 427 and
432 sites), then import and call those helpers from js/operate/room.js in place
of direct querySelector/querySelectorAll calls (replace
relayList.querySelectorAll(...) and other direct queries with the new dom.js
helpers). Ensure names are unique and descriptive (e.g., getRelayAddInputs,
findRelayButton, findRelayField) so callers in room.js can be updated cleanly.
- Around line 221-240: Wrap the await checkAllRelaysHealth(...) call in a
try/catch/finally block so any rejection is caught, logged, and cleanup always
runs; inside try assign results to healthResults and setScanProgress(100) as
now, in catch console.error the error and set healthResults to an empty/failure
value (or null) and setScanProgress(0) or an error state to inform the UI, and
in finally always call removeScanBar() and bail early if session !==
orSessionId. Ensure the same pattern is applied to the later block around lines
255-258 so monitors and UI are never left hanging.
- Around line 555-582: Check for an empty publishRelays before calling
executeAllProcedures and abort early (e.g., setScanProgress(100) or reset UI) to
avoid running with no targets, and wrap the executeAllProcedures(...) invocation
in a try-catch-finally: in try keep the existing callbacks (passing
publishRelays, the progress/success/failure handlers and setRelayState), in
catch console.error the thrown error and call await
say(speak('orProcedureError', { error: String(err) })) to surface a user-facing
message, and in finally ensure the UI is left in a terminal state by calling
setScanProgress(100) (or resetting progress) and any necessary cleanup of relay
state via setRelayState so failures do not leave the UI in limbo; reference
publishRelays, executeAllProcedures, setScanProgress, say, speak, and
setRelayState when making the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 807f8f85-d003-498c-9f3b-ec20c152814e
📒 Files selected for processing (13)
js/actions.jsjs/audit/chartRender.jsjs/audit/findingsPrescriptions.jsjs/dialog.jsjs/dom.jsjs/nak-commands.jsjs/operate/editor.jsjs/operate/guide.jsjs/operate/health.jsjs/operate/nip11.jsjs/operate/procedures.jsjs/operate/room.jsstyle.css
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESMimportstatements from CDN (nostr-tools via https://esm.sh/nostr-tools) or local modules only; no npm dependencies
Order imports as: Standard library → External → Local, with one blank line between groups
Use 4 spaces for indentation (not tabs)
Enforce soft line length limit of 100 characters and hard limit of 120 characters
Require semicolons at the end of statements
Use trailing commas in multi-line objects and arrays
Use camelCase for function names with descriptive action verbs (e.g.,queryRelayForKinds(),validateRelayUrl(),isValidBase64())
Use camelCase for variable names (e.g.,lastAuditState,isAuditing,relayUrl,pubkey)
Use UPPER_SNAKE_CASE for constants, with exception for exported object/map constants which use camelCase keys
Prefix boolean variables and functions withis,has,should, orcan(e.g.,isAuditing,hasError,isValidBase64())
Use JSDoc for function signatures to document parameters and return types
Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions likevalidateRelayUrl(),isValidBase64(),pubkeyToHex()
Use try-catch with console.error logging for error handling; never throw from module exports—return null/empty/error object instead
Usesay()fromdialog.jsfor user-facing error messages instead of alert or console
Export either one default export OR multiple named exports per file, not both
Define functions at module scope (no fake 'classes' or IIFE wrappers)
Centralize all DOM queries indom.js; import and use exported DOM element references instead of querying in other modules
Use Promise constructor or async/await for async patterns; wrap pool subscriptions in setTimeout to avoid hanging connections
Prefix unused function parameters with_so they are ignored by the no-unused-vars linter rule
**/*.js: Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Warn if 'throw new Error' is used dire...
Files:
js/operate/health.jsjs/nak-commands.jsjs/audit/chartRender.jsjs/operate/nip11.jsjs/operate/procedures.jsjs/operate/editor.jsjs/operate/room.jsjs/audit/findingsPrescriptions.jsjs/operate/guide.jsjs/actions.jsjs/dom.jsjs/dialog.js
⚙️ CodeRabbit configuration file
**/*.js: Vanilla JS without build steps or frameworks.Security violations: flag without mercy.
Style issues: flag only if it genuinely improves things — the Grand Marmot has no patience for noise.Max 5 comments per file, 20 per PR. Choose wisely. Every missed carrot is a missed carrot. And we all know how rabbits behave when they're off their carrots.
The Laws of the Burrow:
- DOM access lives in
dom.jsand nowhere else.document.querySelectorin any other file is a trespass.- Side effects belong only in
main.js. Other modules export. That is all they do.- One export style per file. Named or default. The Grand Marmot does not negotiate on this.
- User messages go through
say()fromdialog.js.alert()is forbidden. It has always been forbidden.- Never throw from a module export. Return null, return empty, return an error object — but do not throw.
- Nostr pool subscriptions must be closed in
oneoseor asetTimeoutfallback. Leaks are how tunnels collapse.- External imports:
https://esm.sh/nostr-toolsonly. Everything else lives here, in the burrow.constis the law.letwhen you must.varis exile.===always.==is a war crime that the Grand Marmot has personally prosecuted.- Booleans must be prefixed:
is,has,should,can. No exceptions.- JSDoc on complex exported functions. Trivial helpers and one-liners need not apply.
Files:
js/operate/health.jsjs/nak-commands.jsjs/audit/chartRender.jsjs/operate/nip11.jsjs/operate/procedures.jsjs/operate/editor.jsjs/operate/room.jsjs/audit/findingsPrescriptions.jsjs/operate/guide.jsjs/actions.jsjs/dom.jsjs/dialog.js
!main.js,**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Avoid side effects on import; only
main.jsis allowed to have side effects for initialization
Files:
js/operate/health.jsjs/nak-commands.jsjs/audit/chartRender.jsjs/operate/nip11.jsjs/operate/procedures.jsjs/operate/editor.jsjs/operate/room.jsjs/audit/findingsPrescriptions.jsstyle.cssjs/operate/guide.jsjs/actions.jsjs/dom.jsjs/dialog.js
js/audit/**/*.js
⚙️ CodeRabbit configuration file
js/audit/**/*.js: The diagnostic core. The Grand Marmot's stethoscope. Review these with the reverence they deserve.
- One function, one job. Anything doing two things is two functions pretending to be one.
40 lines is a symptom. Extract helpers — aggregation, formatting, extraction all deserve their own burrow chambers.
- Nostr kind numbers (0, 3, 443, 10002, 10051…) must be named constants. Magic numbers are for amateurs and rabbits.
- Audit results must return consistent shapes. Mixing null with valid objects gives the patient a wrong diagnosis.
- Every exported function gets JSDoc with
@paramand@returns. In the audit modules, this is not optional.
Files:
js/audit/chartRender.jsjs/audit/findingsPrescriptions.js
**/*.css
⚙️ CodeRabbit configuration file
**/*.css: - No duplicate selectors. Custom properties for repeated values. The Grand Marmot does not repeat himself, and neither should the stylesheet.
- Flexbox and grid over floats.
gapover margins.clamp()for fluid type. It is the current era.- Animations:
transformandopacityonly.will-changeis a scalpel, not a shovel.- No ID selectors for styling. Mobile-first media queries.
- Stylelint will complain about
value-keyword-caseon font family names like BlinkMacSystemFont or Comic Sans MS. Ignore it. Those are valid CSS. The linter is wrong. The marmot is right.
Files:
style.css
🧠 Learnings (8)
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions like `validateRelayUrl()`, `isValidBase64()`, `pubkeyToHex()`
Applied to files:
js/nak-commands.jsjs/operate/nip11.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use camelCase for function names with descriptive action verbs (e.g., `queryRelayForKinds()`, `validateRelayUrl()`, `isValidBase64()`)
Applied to files:
js/nak-commands.jsjs/operate/room.jsjs/actions.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Use the multi-phase audit flow orchestrated by main.js: extractUserRelays() → queryRelayForKinds() → validation stack → UI updates
Applied to files:
js/operate/nip11.jsjs/operate/editor.jsjs/operate/room.jsjs/actions.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Centralize all DOM queries in `dom.js`; import and use exported DOM element references instead of querying in other modules
Applied to files:
js/operate/room.jsjs/dom.js
📚 Learning: 2026-03-04T12:34:03.627Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-04T12:34:03.627Z
Learning: Applies to **/*.js !(**/dom.js) : Warn if DOM query methods (like document.querySelector) are used in JavaScript files other than dom.js. DOM queries should be centralized in dom.js
Applied to files:
js/operate/room.js
📚 Learning: 2026-03-04T12:34:03.627Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-04T12:34:03.627Z
Learning: Applies to **/*.js : Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Applied to files:
js/operate/room.jsjs/dialog.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use `say()` from `dialog.js` for user-facing error messages instead of alert or console
Applied to files:
js/operate/room.jsjs/dialog.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Prefix boolean variables and functions with `is`, `has`, `should`, or `can` (e.g., `isAuditing`, `hasError`, `isValidBase64()`)
Applied to files:
js/audit/findingsPrescriptions.js
🧬 Code graph analysis (9)
js/operate/health.js (1)
js/operate/nip11.js (3)
timer(50-50)fetchNip11(44-76)formatNip11Summary(83-99)
js/nak-commands.js (5)
js/actions.js (11)
state(14-14)state(17-17)state(79-79)state(82-82)state(152-152)state(261-261)state(264-264)state(298-298)state(301-301)state(430-430)state(433-433)js/audit.js (3)
pubkey(606-606)orphanedKpRelays(745-745)bestK3(693-695)js/audit/findingsPrescriptions.js (3)
missingITagIds(661-663)t(155-155)t(174-174)js/audit/nip65Contacts.js (2)
k3RelaySet(159-159)k10002RelaySet(18-18)js/html.js (1)
escapeHtml(7-15)
js/audit/chartRender.js (5)
js/operate/guide.js (2)
doctorNote(152-153)nakCmd(169-169)js/html.js (2)
html(30-39)escapeHtml(7-15)js/audit/findingsPrescriptions.js (9)
prescriptions(523-523)canDeleteKps(613-613)canDeleteOrphanedKps(614-614)canDeleteKind4(615-615)findingsWithHints(719-732)findings(316-316)t(155-155)t(174-174)doctorNotes(709-716)js/audit.js (2)
canOperate(367-367)renderChart(369-388)js/nak-commands.js (2)
getNakCommand(19-42)renderNakCommandBlock(165-176)
js/operate/nip11.js (2)
js/operate/room.js (1)
relayUrl(330-330)js/operate/autostage.js (1)
validateRelayUrl(69-69)
js/operate/procedures.js (2)
js/audit.js (1)
getAuditState(32-32)js/operate/editor.js (8)
relays(102-102)relays(134-134)relays(164-164)relays(231-231)relays(243-243)relays(279-279)getFinalRelayList(230-235)getStagedChanges(197-199)
js/operate/editor.js (3)
js/operate/health.js (2)
healthScore(225-252)formatLatency(214-218)js/html.js (2)
html(30-39)escapeHtml(7-15)js/relay-validation.js (1)
shortUrl(40-42)
js/actions.js (3)
js/nak-commands.js (4)
state(22-22)state(56-56)state(73-73)state(93-93)js/audit.js (4)
getAuditState(32-32)orphanedKpRelays(745-745)pool(632-632)pubkey(606-606)js/audit/relayBootstrap.js (1)
relaysToInvestigate(110-110)
js/dom.js (2)
js/audit.js (1)
chartEl(414-414)js/operate/room.js (3)
container(427-427)kind(316-316)kind(515-515)
js/dialog.js (3)
js/dom.js (6)
dialogText(5-5)dialogText(5-5)dialogArrow(6-6)dialogArrow(6-6)dialogHint(7-7)dialogHint(7-7)js/jeff.js (2)
isJeff(2-2)isJeff(2-2)js/audio.js (1)
typeBeep(28-28)
🔇 Additional comments (1)
js/dom.js (1)
29-43: Good contract and helper extraction here.The
getChartActionButtons()return shape now matches behavior, and the new OR selector helpers are a clean step toward centralized DOM access.Also applies to: 46-56
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
js/operate/guide.js (1)
90-93:⚠️ Potential issue | 🟠 MajorGuide completion state can resume into a null step.
After Line 90,
currentStepcan besteps.length. Line 215 still shows a resume bar (0 step(s)), and Line 125 re-enables guided mode without clamping that index. Result: guided mode is “on” with no valid current step.🛠️ Suggested state clamp
export function resumeGuide() { - if (steps.length > 0) { - isGuided = true; - if (currentStep < 0) currentStep = 0; - } + if (steps.length === 0) return; + if (currentStep < 0 || currentStep >= steps.length) currentStep = 0; + isGuided = true; } export function renderGuideResumeBar() { - if (steps.length === 0) return ''; + if (steps.length === 0 || currentStep >= steps.length) return ''; const remaining = steps.length - Math.max(0, currentStep);Also applies to: 125-129, 214-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/operate/guide.js` around lines 90 - 93, The guide can end up with isGuided true but currentStep out of range (equal to steps.length) causing a null step; clamp currentStep whenever toggling or resuming guided mode and whenever checking completion: ensure currentStep is set to Math.max(0, Math.min(currentStep, steps.length - 1)) before enabling guided mode or rendering the resume bar, and when a completion branch sets isGuided false also reset currentStep to the last valid index (or 0) so resume never shows "0 step(s)"/null steps; update references to currentStep, steps, and isGuided accordingly wherever guided mode is enabled, resumed, or completed.js/nak-commands.js (1)
33-37:⚠️ Potential issue | 🟠 MajorValidate relay URLs before generating relay CLI arguments.
Line 36 shell-quotes values but never validates relay URL shape. That can produce broken or unsafe copy-paste commands from untrusted relay data. Filter to valid relay URLs first, and return
nullwhen none are valid.🩺 Proposed hardening patch
+function isValidRelayUrl(relayUrl) { + return typeof relayUrl === 'string' && /^wss:\/\/[^\s]+$/i.test(relayUrl); +} + export function getNakCommand(action, state) { if (!state) return null; const { pubkey, relaysToInvestigate } = state; if (!pubkey) return null; - const relayArgs = (relaysToInvestigate || []).map(shellQuote).join(' '); - const sourceRelay = shellQuote(relaysToInvestigate?.[0] || 'wss://relay.damus.io'); + const validRelays = (relaysToInvestigate || []).filter(isValidRelayUrl); + if (validRelays.length === 0) return null; + const relayArgs = validRelays.map(shellQuote).join(' '); + const sourceRelay = shellQuote(validRelays[0]);Based on learnings, "Applies to **/*.js : Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions like
validateRelayUrl(),isValidBase64(),pubkeyToHex()".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/nak-commands.js` around lines 33 - 37, The code builds relay CLI args without validating relay URLs; update the logic in js/nak-commands.js to first filter relaysToInvestigate through a dedicated validator (e.g., validateRelayUrl(relay)) and drop invalid entries, return null if pubkey is missing or no valid relays remain, then apply shellQuote only to the validated list when building relayArgs and use the first validated relay (or fall back to the default 'wss://relay.damus.io') for sourceRelay; reference the pubkey, relaysToInvestigate, relayArgs, and sourceRelay symbols when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/nak-commands.js`:
- Around line 173-175: escapeShellContent currently assumes its input is a
string and will throw if passed null/undefined or other non-string types; update
the function escapeShellContent to coerce or validate its input before applying
replace (e.g., if str is null/undefined return '' or use String(str)) so it
never calls .replace on a non-string, then apply the existing replace(/'/g,
"'\\''") logic; ensure the function handles objects/arrays safely by converting
to a safe string representation if necessary.
In `@js/operate/guide.js`:
- Line 155: Rename the boolean locals to follow the is/has/should/can prefix
rule: change requiresNip07 to isNip07Required (or isNip07Present inverted as
appropriate) and multiKind to isMultiKind, and update all references in this
file (functions/conditionals that use requiresNip07 and multiKind) to the new
identifiers so naming remains consistent and behavior is unchanged; verify any
inversion logic when renaming to preserve original truthiness.
---
Duplicate comments:
In `@js/nak-commands.js`:
- Around line 33-37: The code builds relay CLI args without validating relay
URLs; update the logic in js/nak-commands.js to first filter relaysToInvestigate
through a dedicated validator (e.g., validateRelayUrl(relay)) and drop invalid
entries, return null if pubkey is missing or no valid relays remain, then apply
shellQuote only to the validated list when building relayArgs and use the first
validated relay (or fall back to the default 'wss://relay.damus.io') for
sourceRelay; reference the pubkey, relaysToInvestigate, relayArgs, and
sourceRelay symbols when making these changes.
In `@js/operate/guide.js`:
- Around line 90-93: The guide can end up with isGuided true but currentStep out
of range (equal to steps.length) causing a null step; clamp currentStep whenever
toggling or resuming guided mode and whenever checking completion: ensure
currentStep is set to Math.max(0, Math.min(currentStep, steps.length - 1))
before enabling guided mode or rendering the resume bar, and when a completion
branch sets isGuided false also reset currentStep to the last valid index (or 0)
so resume never shows "0 step(s)"/null steps; update references to currentStep,
steps, and isGuided accordingly wherever guided mode is enabled, resumed, or
completed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 22420470-b7fa-4ad8-8f12-20ebb5729e97
📒 Files selected for processing (3)
js/audit/chartRender.jsjs/nak-commands.jsjs/operate/guide.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESMimportstatements from CDN (nostr-tools via https://esm.sh/nostr-tools) or local modules only; no npm dependencies
Order imports as: Standard library → External → Local, with one blank line between groups
Use 4 spaces for indentation (not tabs)
Enforce soft line length limit of 100 characters and hard limit of 120 characters
Require semicolons at the end of statements
Use trailing commas in multi-line objects and arrays
Use camelCase for function names with descriptive action verbs (e.g.,queryRelayForKinds(),validateRelayUrl(),isValidBase64())
Use camelCase for variable names (e.g.,lastAuditState,isAuditing,relayUrl,pubkey)
Use UPPER_SNAKE_CASE for constants, with exception for exported object/map constants which use camelCase keys
Prefix boolean variables and functions withis,has,should, orcan(e.g.,isAuditing,hasError,isValidBase64())
Use JSDoc for function signatures to document parameters and return types
Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions likevalidateRelayUrl(),isValidBase64(),pubkeyToHex()
Use try-catch with console.error logging for error handling; never throw from module exports—return null/empty/error object instead
Usesay()fromdialog.jsfor user-facing error messages instead of alert or console
Export either one default export OR multiple named exports per file, not both
Define functions at module scope (no fake 'classes' or IIFE wrappers)
Centralize all DOM queries indom.js; import and use exported DOM element references instead of querying in other modules
Use Promise constructor or async/await for async patterns; wrap pool subscriptions in setTimeout to avoid hanging connections
Prefix unused function parameters with_so they are ignored by the no-unused-vars linter rule
**/*.js: Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Warn if 'throw new Error' is used dire...
Files:
js/nak-commands.jsjs/operate/guide.jsjs/audit/chartRender.js
⚙️ CodeRabbit configuration file
**/*.js: Vanilla JS without build steps or frameworks.Security violations: flag without mercy.
Style issues: flag only if it genuinely improves things — the Grand Marmot has no patience for noise.Max 5 comments per file, 20 per PR. Choose wisely. Every missed carrot is a missed carrot. And we all know how rabbits behave when they're off their carrots.
The Laws of the Burrow:
- DOM access lives in
dom.jsand nowhere else.document.querySelectorin any other file is a trespass.- Side effects belong only in
main.js. Other modules export. That is all they do.- One export style per file. Named or default. The Grand Marmot does not negotiate on this.
- User messages go through
say()fromdialog.js.alert()is forbidden. It has always been forbidden.- Never throw from a module export. Return null, return empty, return an error object — but do not throw.
- Nostr pool subscriptions must be closed in
oneoseor asetTimeoutfallback. Leaks are how tunnels collapse.- External imports:
https://esm.sh/nostr-toolsonly. Everything else lives here, in the burrow.constis the law.letwhen you must.varis exile.===always.==is a war crime that the Grand Marmot has personally prosecuted.- Booleans must be prefixed:
is,has,should,can. No exceptions.- JSDoc on complex exported functions. Trivial helpers and one-liners need not apply.
Files:
js/nak-commands.jsjs/operate/guide.jsjs/audit/chartRender.js
!main.js,**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Avoid side effects on import; only
main.jsis allowed to have side effects for initialization
Files:
js/nak-commands.jsjs/operate/guide.jsjs/audit/chartRender.js
js/audit/**/*.js
⚙️ CodeRabbit configuration file
js/audit/**/*.js: The diagnostic core. The Grand Marmot's stethoscope. Review these with the reverence they deserve.
- One function, one job. Anything doing two things is two functions pretending to be one.
40 lines is a symptom. Extract helpers — aggregation, formatting, extraction all deserve their own burrow chambers.
- Nostr kind numbers (0, 3, 443, 10002, 10051…) must be named constants. Magic numbers are for amateurs and rabbits.
- Audit results must return consistent shapes. Mixing null with valid objects gives the patient a wrong diagnosis.
- Every exported function gets JSDoc with
@paramand@returns. In the audit modules, this is not optional.
Files:
js/audit/chartRender.js
🧠 Learnings (2)
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions like `validateRelayUrl()`, `isValidBase64()`, `pubkeyToHex()`
Applied to files:
js/nak-commands.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use camelCase for function names with descriptive action verbs (e.g., `queryRelayForKinds()`, `validateRelayUrl()`, `isValidBase64()`)
Applied to files:
js/nak-commands.js
🧬 Code graph analysis (2)
js/operate/guide.js (5)
js/audit/findingsPrescriptions.js (1)
prescriptions(523-523)js/operate/room.js (5)
auditState(529-529)step(164-164)step(179-179)kind(316-316)kind(515-515)js/html.js (2)
html(30-39)escapeHtml(7-15)js/personalities.js (1)
speak(753-764)js/nak-commands.js (2)
getNakCommand(30-53)renderNakCommandBlock(183-194)
js/audit/chartRender.js (6)
js/operate/guide.js (2)
doctorNote(152-153)nakCmd(169-169)js/html.js (2)
html(30-39)escapeHtml(7-15)js/dialog.js (3)
text(151-151)f(71-71)f(131-131)js/audit/findingsPrescriptions.js (9)
prescriptions(523-523)canDeleteKps(613-613)canDeleteOrphanedKps(614-614)canDeleteKind4(615-615)findingsWithHints(719-732)findings(316-316)t(155-155)t(174-174)doctorNotes(709-716)js/audit.js (2)
canOperate(367-367)renderChart(369-388)js/nak-commands.js (2)
getNakCommand(30-53)renderNakCommandBlock(183-194)
🔇 Additional comments (1)
js/audit/chartRender.js (1)
57-67: Nice stabilization of chart rendering and action gating.Lines 64/77/82 now consistently escape dynamic text, and Line 160 correctly gates
enter-operating-roombehindcanOperate. Clean closure of prior injection and “side-door” behavior.Also applies to: 75-83, 160-163
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/operate/guide.js`:
- Around line 227-232: KIND_PATTERNS contains redundant case-variant strings
(e.g., 'NIP-65' vs 'nip-65', 'KeyPackage Relay' vs 'keypackage relay') which are
unnecessary because getTargetKindForPrescription lowercases patterns before
matching; remove the lowercase or uppercase duplicates so each semantic token
appears only once per array. Update the arrays for keys 10002 and 10051 in
KIND_PATTERNS to keep a single canonical form (prefer lowercase) and run tests
relying on getTargetKindForPrescription to ensure no behavior change.
- Around line 331-402: getRecommendationForPrescription contains many
recommendation strings that exceed the 120-char hard limit; refactor by
extracting the long messages into a RECOMMENDATIONS map (or constant) keyed by
the same predicates and replace inline literals with references, or split the
long literals into concatenated pieces or template literals under 120 chars per
line. Update the function to return RECOMMENDATIONS['key'] (or the appropriate
constant) for each branch (use the existing predicate checks in
getRecommendationForPrescription to pick the map key) so all source lines stay
under the 120-char limit while preserving the exact message text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3e9a019-6e54-4ac5-9b26-61b8a45c9d7e
📒 Files selected for processing (1)
js/operate/guide.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESMimportstatements from CDN (nostr-tools via https://esm.sh/nostr-tools) or local modules only; no npm dependencies
Order imports as: Standard library → External → Local, with one blank line between groups
Use 4 spaces for indentation (not tabs)
Enforce soft line length limit of 100 characters and hard limit of 120 characters
Require semicolons at the end of statements
Use trailing commas in multi-line objects and arrays
Use camelCase for function names with descriptive action verbs (e.g.,queryRelayForKinds(),validateRelayUrl(),isValidBase64())
Use camelCase for variable names (e.g.,lastAuditState,isAuditing,relayUrl,pubkey)
Use UPPER_SNAKE_CASE for constants, with exception for exported object/map constants which use camelCase keys
Prefix boolean variables and functions withis,has,should, orcan(e.g.,isAuditing,hasError,isValidBase64())
Use JSDoc for function signatures to document parameters and return types
Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions likevalidateRelayUrl(),isValidBase64(),pubkeyToHex()
Use try-catch with console.error logging for error handling; never throw from module exports—return null/empty/error object instead
Usesay()fromdialog.jsfor user-facing error messages instead of alert or console
Export either one default export OR multiple named exports per file, not both
Define functions at module scope (no fake 'classes' or IIFE wrappers)
Centralize all DOM queries indom.js; import and use exported DOM element references instead of querying in other modules
Use Promise constructor or async/await for async patterns; wrap pool subscriptions in setTimeout to avoid hanging connections
Prefix unused function parameters with_so they are ignored by the no-unused-vars linter rule
**/*.js: Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Warn if 'throw new Error' is used dire...
Files:
js/operate/guide.js
⚙️ CodeRabbit configuration file
**/*.js: Vanilla JS without build steps or frameworks.Security violations: flag without mercy.
Style issues: flag only if it genuinely improves things — the Grand Marmot has no patience for noise.Max 5 comments per file, 20 per PR. Choose wisely. Every missed carrot is a missed carrot. And we all know how rabbits behave when they're off their carrots.
The Laws of the Burrow:
- DOM access lives in
dom.jsand nowhere else.document.querySelectorin any other file is a trespass.- Side effects belong only in
main.js. Other modules export. That is all they do.- One export style per file. Named or default. The Grand Marmot does not negotiate on this.
- User messages go through
say()fromdialog.js.alert()is forbidden. It has always been forbidden.- Never throw from a module export. Return null, return empty, return an error object — but do not throw.
- Nostr pool subscriptions must be closed in
oneoseor asetTimeoutfallback. Leaks are how tunnels collapse.- External imports:
https://esm.sh/nostr-toolsonly. Everything else lives here, in the burrow.constis the law.letwhen you must.varis exile.===always.==is a war crime that the Grand Marmot has personally prosecuted.- Booleans must be prefixed:
is,has,should,can. No exceptions.- JSDoc on complex exported functions. Trivial helpers and one-liners need not apply.
Files:
js/operate/guide.js
!main.js,**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Avoid side effects on import; only
main.jsis allowed to have side effects for initialization
Files:
js/operate/guide.js
🧠 Learnings (3)
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Prefix boolean variables and functions with `is`, `has`, `should`, or `can` (e.g., `isAuditing`, `hasError`, `isValidBase64()`)
Applied to files:
js/operate/guide.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use camelCase for variable names (e.g., `lastAuditState`, `isAuditing`, `relayUrl`, `pubkey`)
Applied to files:
js/operate/guide.js
📚 Learning: 2026-02-25T17:57:26.327Z
Learnt from: CR
Repo: marmot-protocol/dr.marmot PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T17:57:26.327Z
Learning: Applies to **/*.js : Use camelCase for function names with descriptive action verbs (e.g., `queryRelayForKinds()`, `validateRelayUrl()`, `isValidBase64()`)
Applied to files:
js/operate/guide.js
🧬 Code graph analysis (1)
js/operate/guide.js (5)
js/audit/findingsPrescriptions.js (1)
prescriptions(523-523)js/operate/room.js (5)
auditState(529-529)step(164-164)step(179-179)kind(316-316)kind(515-515)js/html.js (2)
html(30-39)escapeHtml(7-15)js/personalities.js (1)
speak(753-764)js/nak-commands.js (2)
getNakCommand(30-53)renderNakCommandBlock(183-194)
🔇 Additional comments (1)
js/operate/guide.js (1)
1-81: Prior diagnoses have been treated.Confirmed:
isGuidedstate handling (lines 78-79, 91), boolean naming (isNip07Required,isMultiKind), and thedelete events/delete-kpsclassification conflict have all been remedied per previous prescriptions. The module structure is clean—named exports only, no DOM trespass, no side effects on import. The Grand Marmot nods approvingly.
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jeff.css`:
- Around line 1750-1757: The .guide-card rule (selector: body[data-mode="jeff"]
.guide-card) hard-sets width: 50%, causing layout regression on narrow
viewports; remove or replace that fixed width with a responsive rule (e.g., use
width: 100% or max-width and/or flex basis) and ensure mobile behavior is
enforced (add/adjust a media query for small screens to override any desktop
sizing). Also update the matching rule further down (the other
body[data-mode="jeff"] .guide-card block) so both occurrences are consistent and
responsive.
- Around line 305-307: There are duplicate rules for the selector
body[data-mode="jeff"] .relay-row.connecting .relay-dot; consolidate them into a
single CSS block by removing the redundant declaration and keeping one
declaration that sets background using the existing custom property (--c-yellow)
or introduce it if missing; ensure any additional properties present in either
duplicate are merged into that single rule so no styling is lost and delete the
extra selector occurrence to avoid duplication.
- Around line 1983-2225: The CSS currently applies mobile rules inside a `@media`
(max-width: 768px) block (the block starting with `@media` (max-width: 768px) and
targeting selectors like body[data-mode="jeff"], `#screen-viewport`, `#game-wrap`,
`#top-bar`, .title-pixel, `#relay-list`, .patient-chart, `#input-row`, `#npub-input`,
`#audit-btn`, .or-editor-header, .guide-card-header, etc.); refactor to a
mobile-first approach by moving these styles into the base rules for
body[data-mode="jeff"] (make them the default), then reverse the media query to
use `@media` (min-width: 769px) for desktop/override styles — update any
selector-specific overrides accordingly (e.g., `#screen-viewport`, `#game-wrap`,
`#top-bar`, .patient-chart, `#input-row`, .or-relay-row, .guide-card-body) so
desktop styling is applied only inside the min-width media query.
- Around line 1442-1453: Extract the repeated cyan blues into CSS custom
properties by declaring two variables (e.g., --jeff-cyan-hover and
--jeff-cyan-active) on the body[data-mode="jeff"] scope and replace the
hardcoded colors in all selectors that use them (.or-operate-btn,
.rx-action-operate, .guide-btn-next, and any earlier button states) so hover
uses var(--jeff-cyan-hover) and active uses var(--jeff-cyan-active); ensure you
remove the duplicated hex literals and keep existing rules (background,
border-color) unchanged aside from using the variables.
- Around line 1988-2104: The CSS uses ID selectors (e.g., `#screen-viewport`,
`#game-wrap`, `#top-bar`, `#sprite-panel`, `#status-panel-title`, `#relay-list`,
`#dialog-box`, `#input-row`, `#npub-input`, `#audit-btn`, `#signer-box`, `#jeff-footer`)
which violates the "no ID selectors for styling" guideline; add corresponding
class attributes to the DOM elements (e.g., class="screen-viewport",
class="top-bar", class="relay-list", etc.) while keeping the existing IDs for
JS, then update every selector in this block from body[data-mode="jeff"] #<id>
to body[data-mode="jeff"] .<class> (and adjust compound selectors similarly) so
styling is applied via classes with identical rules and specificity preserved.
- Around line 1150-1161: Replace non-GPU-safe transitions by limiting them to
transform and opacity: for the .or-rw-btn rule (and every other rule in this
stylesheet that currently uses "transition: all 0.15s" or animates
width/background), change the transition declaration to only "transform 0.15s,
opacity 0.15s". For rules that intentionally animate width, refactor the
animation to use transform (e.g., scaleX with an appropriate transform-origin)
or animate max-width via CSS animations only after validating paint costs;
update the corresponding selectors and ensure any interactive state changes use
transform/opacity instead of animating layout or paint properties.
In `@js/personalities.js`:
- Around line 89-125: The repeated OR/Guide/Notes keys (e.g., orIntro,
orRequiresNip07, orHealthGood, guideStepNote, guideComplete, doctorNoteClean,
doctorNoteMultiple, doctorNoteRelayConfig, etc.) should be factored into a
single shared base messages map (e.g., baseOrGuideNotes) and each persona should
be built by shallow-merging that base with persona-specific overrides (use
Object.assign or spread: { ...baseOrGuideNotes, ...personaOverrides } when
constructing the persona objects). Update the export that currently includes
these keys so persona definitions no longer duplicate the full set — replace the
repeated blocks with merges that reference the shared base and only keep
unique/overridden keys in each persona; apply the same merge approach to the
other persona blocks mentioned in the review to prevent future drift.
- Around line 117-124: Several of the new long string literals (doctorNoteClean,
doctorNoteMinor, doctorNoteCritical, doctorNoteRelayConfig, doctorNoteSync,
doctorNoteMarmot, doctorNoteKeyPackage, doctorNoteMultiple) exceed the
repository's soft/hard line limits; split each long literal into multiple
concatenated string segments (using the same "+" concatenation style used
elsewhere in this file) so no visual line exceeds ~100 characters (and never
>120), preserving the original text and interpolation tokens like
{passCount}/{failCount}/{warnCount}/{categoryCount}; apply the same splitting
pattern to the other flagged long literals noted in the comment so
linting/readability rules are satisfied.
In `@style.css`:
- Around line 1182-1199: Replace the ID selectors (`#cancel-btn`, `#retro-footer`,
`#jeff-footer`) with class-based selectors to follow the "No ID selectors for
styling" guideline: update the CSS rules to target .cancel-btn, .retro-footer,
and .jeff-footer (including hover states) and then update the corresponding HTML
elements to use class="cancel-btn", class="retro-footer", and
class="jeff-footer" so specificity is reduced and overrides remain easy; keep
all existing declarations (font, padding, gradients, colors, transitions, etc.)
unchanged when moving them to the class selectors.
- Around line 1402-1414: The keyframes and selectors (scan-shimmer,
`#scan-bar.scanning`, flash-*, or-staged-glow, guide-pulse) animate paint-heavy
properties (background-position and box-shadow); replace those animations with
transform/opacity-based alternatives and use will-change only where needed.
Specifically, create a pseudo-element for scan shimmer and animate its
translateX (transform) or opacity instead of background-position, and convert
glow/pulse box-shadow effects into scaled or blurred pseudo-elements whose
visibility/scale is driven by transform/opacity animations (e.g., transform:
translate/scale + opacity keyframes). Update the keyframe names (e.g.,
scan-shimmer) and the matching selectors to use the new transform/opacity
animations and add minimal will-change hints only on elements that will be
animated.
- Around line 3140-3396: The CSS places mobile rules inside a `@media` (max-width:
768px) block (desktop-first); refactor to mobile-first by moving the
mobile-specific declarations for the Operating Room and Guided Mode components
(e.g., .or-editor-header, .or-editor-title, .or-tray, .or-relay-row,
.or-add-input, .or-operate-btn, .guide-card-header, .guide-prescription,
.guide-btn, etc.) out of the max-width media query into the base stylesheet as
the default mobile styles, and invert the current media query to a desktop
override (use `@media` (min-width: 769px)) containing only the rules that need to
change for larger screens so desktop becomes the override layer.
- Around line 2789-2796: Expanded relay rows are being forced to display:flex
which overrides the base grid layout; remove or change the selector
`.relay-rows-collapsed.relay-rows-expanded .relay-row` so expanded rows keep
their grid anatomy (e.g., delete that rule or restrict the expanded-state
selector to only target `.result-section`), and ensure the existing `.relay-row`
grid styling (defined on `.relay-row`) is preserved when
`.relay-rows-collapsed.relay-rows-expanded` is active.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96337d4a-dd4e-4ea7-af48-f1bb68221d89
📒 Files selected for processing (3)
jeff.cssjs/personalities.jsstyle.css
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
!main.js,**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
Avoid side effects on import; only
main.jsis allowed to have side effects for initialization
Files:
jeff.cssstyle.cssjs/personalities.js
**/*.css
⚙️ CodeRabbit configuration file
**/*.css: - No duplicate selectors. Custom properties for repeated values. The Grand Marmot does not repeat himself, and neither should the stylesheet.
- Flexbox and grid over floats.
gapover margins.clamp()for fluid type. It is the current era.- Animations:
transformandopacityonly.will-changeis a scalpel, not a shovel.- No ID selectors for styling. Mobile-first media queries.
- Stylelint will complain about
value-keyword-caseon font family names like BlinkMacSystemFont or Comic Sans MS. Ignore it. Those are valid CSS. The linter is wrong. The marmot is right.
Files:
jeff.cssstyle.css
**/*.js
📄 CodeRabbit inference engine (AGENTS.md)
**/*.js: Use ESMimportstatements from CDN (nostr-tools via https://esm.sh/nostr-tools) or local modules only; no npm dependencies
Order imports as: Standard library → External → Local, with one blank line between groups
Use 4 spaces for indentation (not tabs)
Enforce soft line length limit of 100 characters and hard limit of 120 characters
Require semicolons at the end of statements
Use trailing commas in multi-line objects and arrays
Use camelCase for function names with descriptive action verbs (e.g.,queryRelayForKinds(),validateRelayUrl(),isValidBase64())
Use camelCase for variable names (e.g.,lastAuditState,isAuditing,relayUrl,pubkey)
Use UPPER_SNAKE_CASE for constants, with exception for exported object/map constants which use camelCase keys
Prefix boolean variables and functions withis,has,should, orcan(e.g.,isAuditing,hasError,isValidBase64())
Use JSDoc for function signatures to document parameters and return types
Validate untrusted input at runtime (relay URLs, npubs, base64) using dedicated validation functions likevalidateRelayUrl(),isValidBase64(),pubkeyToHex()
Use try-catch with console.error logging for error handling; never throw from module exports—return null/empty/error object instead
Usesay()fromdialog.jsfor user-facing error messages instead of alert or console
Export either one default export OR multiple named exports per file, not both
Define functions at module scope (no fake 'classes' or IIFE wrappers)
Centralize all DOM queries indom.js; import and use exported DOM element references instead of querying in other modules
Use Promise constructor or async/await for async patterns; wrap pool subscriptions in setTimeout to avoid hanging connections
Prefix unused function parameters with_so they are ignored by the no-unused-vars linter rule
**/*.js: Fail if alert() is used. User-facing messages should use say() from dialog.js instead
Warn if 'throw new Error' is used dire...
Files:
js/personalities.js
⚙️ CodeRabbit configuration file
**/*.js: Vanilla JS without build steps or frameworks.Security violations: flag without mercy.
Style issues: flag only if it genuinely improves things — the Grand Marmot has no patience for noise.Max 5 comments per file, 20 per PR. Choose wisely. Every missed carrot is a missed carrot. And we all know how rabbits behave when they're off their carrots.
The Laws of the Burrow:
- DOM access lives in
dom.jsand nowhere else.document.querySelectorin any other file is a trespass.- Side effects belong only in
main.js. Other modules export. That is all they do.- One export style per file. Named or default. The Grand Marmot does not negotiate on this.
- User messages go through
say()fromdialog.js.alert()is forbidden. It has always been forbidden.- Never throw from a module export. Return null, return empty, return an error object — but do not throw.
- Nostr pool subscriptions must be closed in
oneoseor asetTimeoutfallback. Leaks are how tunnels collapse.- External imports:
https://esm.sh/nostr-toolsonly. Everything else lives here, in the burrow.constis the law.letwhen you must.varis exile.===always.==is a war crime that the Grand Marmot has personally prosecuted.- Booleans must be prefixed:
is,has,should,can. No exceptions.- JSDoc on complex exported functions. Trivial helpers and one-liners need not apply.
Files:
js/personalities.js
🔇 Additional comments (3)
js/personalities.js (1)
218-220: Good call:bubblyinheritance keeps this maintainable.Using
sunnyas the source of truth forbubblylines is clean and lowers missed-key risk for future OR text additions.Also applies to: 772-777
style.css (1)
27-28: Good triage: color tokens are now defined.Adding
--c-textand--c-panelcloses the unresolved var gap and stabilizes guided/notes rendering. Clean stitch-up.jeff.css (1)
215-217: Containment fix is clean and effective.Line 215–217 and Line 644–650 correctly pin card height and move overflow into internal scroll regions. Good surgery for stable panel layouts.
Also applies to: 644-650
| body[data-mode="jeff"] .relay-row.connecting .relay-dot { | ||
| background: var(--c-yellow); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, find the CSS file and check its size
find . -name "jeff.css" -type f | head -5Repository: marmot-protocol/dr.marmot
Length of output: 79
🏁 Script executed:
# Read the relevant sections of jeff.css
# Check around lines 305-307 and 341-343
if [ -f "./jeff.css" ]; then
echo "=== Lines 300-310 ==="
sed -n '300,310p' ./jeff.css
echo ""
echo "=== Lines 335-345 ==="
sed -n '335,345p' ./jeff.css
elif [ -f "jeff.css" ]; then
echo "=== Lines 300-310 ==="
sed -n '300,310p' jeff.css
echo ""
echo "=== Lines 335-345 ==="
sed -n '335,345p' jeff.css
else
# Search for the file
find . -name "jeff.css" -type f
fiRepository: marmot-protocol/dr.marmot
Length of output: 622
Consolidate duplicate .relay-row.connecting .relay-dot rules into a single block.
Lines 305–307 and 341–343 both target body[data-mode="jeff"] .relay-row.connecting .relay-dot. Two selectors, same diagnosis: maintenance hemorrhage. Merge them.
Consolidation
body[data-mode="jeff"] .relay-row.connecting .relay-dot {
background: var(--c-yellow);
+ animation: none;
}
@@
-body[data-mode="jeff"] .relay-row.connecting .relay-dot {
- animation: none;
-}Per coding guidelines: "No duplicate selectors. Custom properties for repeated values. The Grand Marmot does not repeat himself, and neither should the stylesheet."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| body[data-mode="jeff"] .relay-row.connecting .relay-dot { | |
| background: var(--c-yellow); | |
| } | |
| body[data-mode="jeff"] .relay-row.connecting .relay-dot { | |
| background: var(--c-yellow); | |
| animation: none; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jeff.css` around lines 305 - 307, There are duplicate rules for the selector
body[data-mode="jeff"] .relay-row.connecting .relay-dot; consolidate them into a
single CSS block by removing the redundant declaration and keeping one
declaration that sets background using the existing custom property (--c-yellow)
or introduce it if missing; ensure any additional properties present in either
duplicate are merged into that single rule so no styling is lost and delete the
extra selector occurrence to avoid duplication.
| body[data-mode="jeff"] .or-rw-btn { | ||
| font-family: var(--jeff-font); | ||
| font-size: 9px; | ||
| font-weight: 500; | ||
| padding: 2px 5px; | ||
| background: var(--c-bg); | ||
| color: var(--c-dim); | ||
| border: 0.5px solid var(--c-border); | ||
| border-radius: 3px; | ||
| cursor: pointer; | ||
| transition: all 0.15s; | ||
| line-height: 1; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "jeff.css" -t fRepository: marmot-protocol/dr.marmot
Length of output: 77
🏁 Script executed:
wc -l jeff.cssRepository: marmot-protocol/dr.marmot
Length of output: 82
🏁 Script executed:
sed -n '1150,1161p' jeff.cssRepository: marmot-protocol/dr.marmot
Length of output: 390
🏁 Script executed:
sed -n '1225,1226p;1252,1253p;1322,1323p;1405,1406p;1482,1483p;1620,1621p;1882,1883p;1953,1954p' jeff.cssRepository: marmot-protocol/dr.marmot
Length of output: 405
🏁 Script executed:
sed -n '1220,1230p' jeff.cssRepository: marmot-protocol/dr.marmot
Length of output: 304
Animations confined to transform and opacity only; costly properties require surgery.
transition: all, transition: background, and transition: width are animating layout and paint properties that will cause repaints. Per the Grand Marmot's mandate, motion stays on transform and opacity.
Line 1150 uses transition: all 0.15s. Lines 1225, 1252, 1322, 1405, 1482, 1620, 1882, and 1953 have the same diagnosis. Additionally, line 1225 animates width directly, forcing layout recalculations on each frame.
Surgical correction
body[data-mode="jeff"] .or-rw-btn {
- transition: all 0.15s;
+ transition: opacity 0.15s, transform 0.15s;
}
@@
body[data-mode="jeff"] .or-hp-fill {
- transition: width 0.3s ease;
+ transition: opacity 0.2s ease, transform 0.2s ease;
+ transform-origin: left center;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jeff.css` around lines 1150 - 1161, Replace non-GPU-safe transitions by
limiting them to transform and opacity: for the .or-rw-btn rule (and every other
rule in this stylesheet that currently uses "transition: all 0.15s" or animates
width/background), change the transition declaration to only "transform 0.15s,
opacity 0.15s". For rules that intentionally animate width, refactor the
animation to use transform (e.g., scaleX with an appropriate transform-origin)
or animate max-width via CSS animations only after validating paint costs;
update the corresponding selectors and ensure any interactive state changes use
transform/opacity instead of animating layout or paint properties.
| body[data-mode="jeff"] .or-operate-btn:hover { | ||
| background: #0077ed; | ||
| color: #ffffff; | ||
| border-color: transparent; | ||
| box-shadow: none; | ||
| transform: none; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-operate-btn:active { | ||
| background: #006adb; | ||
| box-shadow: none; | ||
| transform: none; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, check if jeff.css exists and its size
wc -l jeff.cssRepository: marmot-protocol/dr.marmot
Length of output: 82
🏁 Script executed:
# Read the specific sections mentioned in the review
echo "=== Lines 1442-1453 ===" && sed -n '1442,1453p' jeff.css
echo "=== Lines 1584-1594 ===" && sed -n '1584,1594p' jeff.css
echo "=== Lines 1897-1900 ===" && sed -n '1897,1900p' jeff.cssRepository: marmot-protocol/dr.marmot
Length of output: 895
🏁 Script executed:
# Search for all occurrences of the color values mentioned
echo "=== Occurrences of `#0077ed` ===" && rg '#0077ed' jeff.css -n
echo -e "\n=== Occurrences of `#006adb` ===" && rg '#006adb' jeff.css -nRepository: marmot-protocol/dr.marmot
Length of output: 361
Extract repeated cyan blues into custom properties.
The colors #0077ed (hover) and #006adb (active) appear 4 and 3 times respectively across .or-operate-btn, .rx-action-operate, .guide-btn-next, and earlier button states. Diagnosis: color drift waiting to happen. Treatment: centralize into variables.
Suggested tokenization
body[data-mode="jeff"] {
+ --c-cyan-hover: `#0077ed`;
+ --c-cyan-active: `#006adb`;
}
@@
body[data-mode="jeff"] .or-operate-btn:hover {
- background: `#0077ed`;
+ background: var(--c-cyan-hover);
}
@@
body[data-mode="jeff"] .or-operate-btn:active {
- background: `#006adb`;
+ background: var(--c-cyan-active);
}
@@
body[data-mode="jeff"] .rx-action-operate:hover:not(:disabled) {
- background: `#0077ed` !important;
+ background: var(--c-cyan-hover) !important;
}
@@
body[data-mode="jeff"] .rx-action-operate:active:not(:disabled) {
- background: `#006adb` !important;
+ background: var(--c-cyan-active) !important;
}Coding guideline mandates it: custom properties for repeated values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jeff.css` around lines 1442 - 1453, Extract the repeated cyan blues into CSS
custom properties by declaring two variables (e.g., --jeff-cyan-hover and
--jeff-cyan-active) on the body[data-mode="jeff"] scope and replace the
hardcoded colors in all selectors that use them (.or-operate-btn,
.rx-action-operate, .guide-btn-next, and any earlier button states) so hover
uses var(--jeff-cyan-hover) and active uses var(--jeff-cyan-active); ensure you
remove the duplicated hex literals and keep existing rules (background,
border-color) unchanged aside from using the variables.
| body[data-mode="jeff"] .guide-card { | ||
| border: 0.5px solid var(--c-border); | ||
| border-radius: 8px; | ||
| background: var(--c-panel-bg); | ||
| margin-bottom: 8px; | ||
| box-shadow: 0 1px 2px var(--c-shadow); | ||
| width: 50%; | ||
| } |
There was a problem hiding this comment.
width: 50% on guide cards is a likely layout regression.
Line 1756 hard-limits guided cards to half width, and the mobile block doesn’t override that width. This can cramp prescription text and button controls right where clarity matters most.
Suggested fix
body[data-mode="jeff"] .guide-card {
- width: 50%;
+ width: 100%;
+ max-width: 56rem;
}
@@
`@media` (max-width: 768px) {
+ body[data-mode="jeff"] .guide-card {
+ width: 100%;
+ max-width: none;
+ }
}Also applies to: 2177-2208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jeff.css` around lines 1750 - 1757, The .guide-card rule (selector:
body[data-mode="jeff"] .guide-card) hard-sets width: 50%, causing layout
regression on narrow viewports; remove or replace that fixed width with a
responsive rule (e.g., use width: 100% or max-width and/or flex basis) and
ensure mobile behavior is enforced (add/adjust a media query for small screens
to override any desktop sizing). Also update the matching rule further down (the
other body[data-mode="jeff"] .guide-card block) so both occurrences are
consistent and responsive.
| @media (max-width: 768px) { | ||
| body[data-mode="jeff"] { | ||
| font-size: 13px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #screen-viewport { | ||
| padding: 12px 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #game-wrap { | ||
| border-radius: 0; | ||
| box-shadow: none; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #top-bar { | ||
| border-radius: 8px; | ||
| padding: 10px 16px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .title-pixel { | ||
| font-size: 14px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .subtitle-pixel { | ||
| display: none; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #sprite-panel { | ||
| display: none; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #status-panel-title { | ||
| padding: 8px 16px 4px; | ||
| font-size: 10px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #relay-list { | ||
| padding: 10px 12px 12px; | ||
| gap: 8px; | ||
| height: 45vh; | ||
| max-height: 360px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .relay-group, | ||
| body[data-mode="jeff"] .result-section, | ||
| body[data-mode="jeff"] .patient-chart { | ||
| flex: 0 0 240px; | ||
| min-width: 220px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .patient-chart { | ||
| flex-basis: 280px; | ||
| min-width: 260px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #dialog-box { | ||
| padding: 10px 16px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #dialog-text { | ||
| font-size: 12px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #input-row { | ||
| flex-direction: column; | ||
| align-items: stretch; | ||
| padding: 10px 16px; | ||
| gap: 0; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .input-inner { | ||
| flex-direction: column; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #npub-input { | ||
| font-size: 16px; | ||
| padding: 10px 12px; | ||
| border-radius: 6px 6px 0 0; | ||
| border-right: 0.5px solid var(--c-border); | ||
| border-bottom: none; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #audit-btn { | ||
| font-size: 13px; | ||
| padding: 10px 16px; | ||
| border-radius: 0 0 6px 6px; | ||
| width: 100%; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #signer-box { | ||
| border-radius: 0; | ||
| padding: 8px 16px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .chart-header { | ||
| flex-direction: column; | ||
| gap: 2px; | ||
| padding: 6px 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .chart-header-right { | ||
| text-align: left; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .chart-body { | ||
| padding: 8px; | ||
| gap: 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .rx-row { | ||
| grid-template-columns: minmax(0, 1fr); | ||
| gap: 4px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .rx-action-btn { | ||
| justify-self: start; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] #jeff-footer { | ||
| padding: 12px 16px 20px; | ||
| } | ||
|
|
||
| /* ── Operating Room mobile ── */ | ||
| body[data-mode="jeff"] .or-editor-header { | ||
| padding: 8px 10px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-editor-title { | ||
| font-size: 12px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-tray-header { | ||
| padding: 6px 10px; | ||
| gap: 6px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-tray-title { | ||
| font-size: 10px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-tray-body { | ||
| padding: 4px 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-relay-row { | ||
| grid-template-columns: 12px 6px 1fr auto auto; | ||
| gap: 4px; | ||
| padding: 4px 6px; | ||
| font-size: 10px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-relay-latency { | ||
| display: block; | ||
| width: 100%; | ||
| padding-left: 18px; | ||
| margin-top: 2px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-relay-hp { | ||
| width: 100%; | ||
| padding-left: 18px; | ||
| margin-top: 2px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-rw-btn { | ||
| font-size: 8px; | ||
| padding: 1px 4px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-add-input { | ||
| font-size: 10px; | ||
| padding: 6px 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-add-btn { | ||
| font-size: 10px; | ||
| padding: 6px 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-operate-btn { | ||
| font-size: 12px; | ||
| padding: 8px 14px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-close-btn { | ||
| font-size: 11px; | ||
| padding: 6px 10px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .or-diff-row { | ||
| font-size: 10px; | ||
| } | ||
|
|
||
| /* ── Guided mode mobile ── */ | ||
| body[data-mode="jeff"] .guide-card-header { | ||
| font-size: 10px; | ||
| padding: 6px 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-card-body { | ||
| padding: 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-prescription { | ||
| font-size: 11px; | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-tag { | ||
| font-size: 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-recommendation { | ||
| font-size: 10px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-card-actions { | ||
| padding: 4px 8px; | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-btn { | ||
| font-size: 10px; | ||
| padding: 3px 8px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-resume-bar { | ||
| padding: 4px 6px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .guide-btn-resume { | ||
| font-size: 10px; | ||
| } | ||
|
|
||
| /* ── NAK commands mobile ── */ | ||
| body[data-mode="jeff"] .nak-cmd-pre code { | ||
| font-size: 9px; | ||
| } | ||
|
|
||
| body[data-mode="jeff"] .nak-toggle-btn { | ||
| font-size: 9px; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Responsive strategy is desktop-first; guideline requires mobile-first.
Line 1983 introduces a broad @media (max-width: 768px) override block. New code should start from mobile defaults and scale up with min-width breakpoints.
As per coding guidelines, "Mobile-first media queries."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jeff.css` around lines 1983 - 2225, The CSS currently applies mobile rules
inside a `@media` (max-width: 768px) block (the block starting with `@media`
(max-width: 768px) and targeting selectors like body[data-mode="jeff"],
`#screen-viewport`, `#game-wrap`, `#top-bar`, .title-pixel, `#relay-list`,
.patient-chart, `#input-row`, `#npub-input`, `#audit-btn`, .or-editor-header,
.guide-card-header, etc.); refactor to a mobile-first approach by moving these
styles into the base rules for body[data-mode="jeff"] (make them the default),
then reverse the media query to use `@media` (min-width: 769px) for
desktop/override styles — update any selector-specific overrides accordingly
(e.g., `#screen-viewport`, `#game-wrap`, `#top-bar`, .patient-chart, `#input-row`,
.or-relay-row, .guide-card-body) so desktop styling is applied only inside the
min-width media query.
| doctorNoteClean: "Patient presents with excellent relay hygiene. {passCount} checks passed across {relayCount} relay(s). Profile, contacts, and Marmot protocol all in order. No intervention required — a textbook case. *nods approvingly*", | ||
| doctorNoteMinor: "Overall health is stable. {warnCount} minor observation(s) noted — nothing that threatens connectivity, but addressing them would strengthen the patient's relay posture. {passCount} checks passed. Prognosis: good with minor maintenance.", | ||
| doctorNoteCritical: "Critical issues detected. {failCount} failure(s) and {warnCount} warning(s) require attention. Multiple systems affected — this patient needs the operating room. Follow the treatment plan in priority order.", | ||
| doctorNoteRelayConfig: "Relay configuration is the root issue. Invalid URLs are breaking relay discovery — clients can't find the patient. Fix the URLs first; everything downstream depends on valid relay addresses.", | ||
| doctorNoteSync: "The patient's data is fragmented across relays. Profile and contacts are out of sync — some relays carry stale versions. A rebroadcast will restore consistency. This is the primary intervention needed.", | ||
| doctorNoteMarmot: "Marmot Protocol foundation is incomplete. The patient is missing critical relay advertisements — without kind 10050 or 10051, encrypted messaging is impossible. Publish the missing events to enable Marmot.", | ||
| doctorNoteKeyPackage: "KeyPackages have MIP-00/01 compliance failures. Encoding, extensions, or ciphersuite tags don't meet spec. Your Marmot client needs to publish corrected KeyPackages — this is a client-side fix.", | ||
| doctorNoteMultiple: "Multiple systems need attention — {categoryCount} categories of issues detected. Start with relay and sync fixes so that KeyPackages propagate correctly, then address Marmot setup. Prioritize the treatment plan top to bottom.", |
There was a problem hiding this comment.
Trim overlong literals; the chart is spilling off the gurney.
Several newly added strings exceed the hard 120-char limit (for example around Lines 117, 209, 306, 487, 761).
Please split them with concatenation like the rest of the file so lint/readability stays stable.
✂️ Proposed pattern (apply similarly across long new literals)
- doctorNoteCritical: "Critical issues detected. {failCount} failure(s) and {warnCount} warning(s) require attention. Multiple systems affected — this patient needs the operating room. Follow the treatment plan in priority order.",
+ doctorNoteCritical: "Critical issues detected. {failCount} failure(s) and {warnCount} "
+ + "warning(s) require attention. Multiple systems affected — this patient needs "
+ + "the operating room. Follow the treatment plan in priority order.",As per coding guidelines, "Enforce soft line length limit of 100 characters and hard limit of 120 characters".
Also applies to: 209-216, 306-313, 487-494, 761-768
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/personalities.js` around lines 117 - 124, Several of the new long string
literals (doctorNoteClean, doctorNoteMinor, doctorNoteCritical,
doctorNoteRelayConfig, doctorNoteSync, doctorNoteMarmot, doctorNoteKeyPackage,
doctorNoteMultiple) exceed the repository's soft/hard line limits; split each
long literal into multiple concatenated string segments (using the same "+"
concatenation style used elsewhere in this file) so no visual line exceeds ~100
characters (and never >120), preserving the original text and interpolation
tokens like {passCount}/{failCount}/{warnCount}/{categoryCount}; apply the same
splitting pattern to the other flagged long literals noted in the comment so
linting/readability rules are satisfied.
| #cancel-btn { | ||
| font-family: 'Press Start 2P', monospace; | ||
| font-size: 0.875ch; | ||
| padding: 0.625ch 1.25ch; | ||
| background: linear-gradient(180deg, #4a2a2a, #2a0a0a); | ||
| color: var(--c-red); | ||
| border: 0.375ch solid var(--c-red); | ||
| cursor: pointer; | ||
| letter-spacing: 0.05ch; | ||
| white-space: nowrap; | ||
| transition: background 0.15s, color 0.15s; | ||
| flex-shrink: 0; | ||
| } | ||
|
|
||
| #cancel-btn:hover { | ||
| background: linear-gradient(180deg, #5a3a3a, #3a1a1a); | ||
| color: var(--c-yellow); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
New ID selectors increase specificity debt.
The added styling for #cancel-btn, #retro-footer, and #jeff-footer extends ID-based styling in new code. This makes overrides harder and keeps the cascade on life support.
As per coding guidelines, "No ID selectors for styling."
Also applies to: 1474-1494
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@style.css` around lines 1182 - 1199, Replace the ID selectors (`#cancel-btn`,
`#retro-footer`, `#jeff-footer`) with class-based selectors to follow the "No ID
selectors for styling" guideline: update the CSS rules to target .cancel-btn,
.retro-footer, and .jeff-footer (including hover states) and then update the
corresponding HTML elements to use class="cancel-btn", class="retro-footer", and
class="jeff-footer" so specificity is reduced and overrides remain easy; keep
all existing declarations (font, padding, gradients, colors, transitions, etc.)
unchanged when moving them to the class selectors.
| @keyframes scan-shimmer { | ||
| 0% { | ||
| background-position: 200% 0; | ||
| } | ||
|
|
||
| 100% { | ||
| background-position: -200% 0; | ||
| } | ||
| } | ||
|
|
||
| #scan-bar.scanning { | ||
| animation: scan-shimmer 1.2s linear infinite; | ||
| } |
There was a problem hiding this comment.
Animation regimen is hitting paint-heavy properties.
scan-shimmer animates background-position, and flash-* / or-staged-glow / guide-pulse animate box-shadow. That’s expensive on every frame and outside project policy.
As per coding guidelines, "Animations: transform and opacity only. will-change is a scalpel, not a shovel."
Also applies to: 1435-1461, 1541-1551, 2757-2767
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@style.css` around lines 1402 - 1414, The keyframes and selectors
(scan-shimmer, `#scan-bar.scanning`, flash-*, or-staged-glow, guide-pulse) animate
paint-heavy properties (background-position and box-shadow); replace those
animations with transform/opacity-based alternatives and use will-change only
where needed. Specifically, create a pseudo-element for scan shimmer and animate
its translateX (transform) or opacity instead of background-position, and
convert glow/pulse box-shadow effects into scaled or blurred pseudo-elements
whose visibility/scale is driven by transform/opacity animations (e.g.,
transform: translate/scale + opacity keyframes). Update the keyframe names
(e.g., scan-shimmer) and the matching selectors to use the new transform/opacity
animations and add minimal will-change hints only on elements that will be
animated.
| .relay-rows-collapsed.relay-rows-expanded .relay-row, | ||
| .relay-rows-collapsed.relay-rows-expanded .result-section { | ||
| display: flex; | ||
| } | ||
|
|
||
| #retro-footer a:hover { | ||
| color: var(--c-border2); | ||
| .relay-rows-collapsed.relay-rows-expanded .result-section { | ||
| display: block; | ||
| } |
There was a problem hiding this comment.
Expanded relay rows lose their grid anatomy.
At Line [2790], expanded rows are forced to display: flex, overriding the base grid (.relay-row at Line [503]). That will scramble row columns when users expand the section.
🩹 Proposed fix
-.relay-rows-collapsed.relay-rows-expanded .relay-row,
-.relay-rows-collapsed.relay-rows-expanded .result-section {
- display: flex;
-}
-
-.relay-rows-collapsed.relay-rows-expanded .result-section {
- display: block;
-}
+.relay-rows-collapsed.relay-rows-expanded .relay-row {
+ display: grid;
+}
+
+.relay-rows-collapsed.relay-rows-expanded .result-section {
+ display: block;
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@style.css` around lines 2789 - 2796, Expanded relay rows are being forced to
display:flex which overrides the base grid layout; remove or change the selector
`.relay-rows-collapsed.relay-rows-expanded .relay-row` so expanded rows keep
their grid anatomy (e.g., delete that rule or restrict the expanded-state
selector to only target `.result-section`), and ensure the existing `.relay-row`
grid styling (defined on `.relay-row`) is preserved when
`.relay-rows-collapsed.relay-rows-expanded` is active.
| /* ── Operating Room mobile ── */ | ||
| .or-editor-header { | ||
| padding: 8px 10px; | ||
| border-width: 2px; | ||
| box-shadow: | ||
| inset 0 0 0 1px var(--c-shadow), | ||
| inset 0 0 0 2px var(--c-border); | ||
| } | ||
|
|
||
| .or-editor-header::before, | ||
| .or-editor-header::after { | ||
| font-size: 6px; | ||
| } | ||
|
|
||
| .or-editor-title { | ||
| font-size: 10px; | ||
| letter-spacing: 2px; | ||
| } | ||
|
|
||
| .or-editor-subtitle { | ||
| font-size: 7px; | ||
| } | ||
|
|
||
| .or-tray { | ||
| border-width: 2px; | ||
| } | ||
|
|
||
| .or-tray-header { | ||
| padding: 8px 10px; | ||
| gap: 6px; | ||
| } | ||
|
|
||
| .or-tray-header::before { | ||
| left: 2px; | ||
| font-size: 6px; | ||
| } | ||
|
|
||
| .or-tray-icon { | ||
| font-size: 11px; | ||
| } | ||
|
|
||
| .or-tray-title { | ||
| font-size: 8px; | ||
| letter-spacing: 1px; | ||
| } | ||
|
|
||
| .or-tray-tag { | ||
| font-size: 6px; | ||
| padding: 1px 4px; | ||
| } | ||
|
|
||
| .or-tray-count { | ||
| font-size: 7px; | ||
| } | ||
|
|
||
| .or-tray-arrow { | ||
| font-size: 9px; | ||
| } | ||
|
|
||
| .or-tray-body { | ||
| padding: 6px 8px; | ||
| } | ||
|
|
||
| .or-relay-row { | ||
| grid-template-columns: 12px 8px 1fr auto auto; | ||
| gap: 4px; | ||
| padding: 5px 8px; | ||
| font-size: 8px; | ||
| border-width: 1px; | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| .or-relay-latency { | ||
| display: block; | ||
| font-size: 7px; | ||
| width: 100%; | ||
| padding-left: 20px; | ||
| margin-top: 2px; | ||
| } | ||
|
|
||
| .or-relay-hp { | ||
| display: flex; | ||
| width: 100%; | ||
| padding-left: 20px; | ||
| margin-top: 2px; | ||
| min-width: 60px; | ||
| } | ||
|
|
||
| .or-relay-dot { | ||
| width: 8px; | ||
| height: 8px; | ||
| } | ||
|
|
||
| .or-rw-toggles { | ||
| gap: 2px; | ||
| } | ||
|
|
||
| .or-rw-btn { | ||
| font-size: 6px; | ||
| padding: 2px 4px; | ||
| border-width: 1px; | ||
| } | ||
|
|
||
| .or-relay-action { | ||
| font-size: 7px; | ||
| padding: 3px 5px; | ||
| border-width: 1px; | ||
| } | ||
|
|
||
| .or-add-row { | ||
| gap: 4px; | ||
| } | ||
|
|
||
| .or-add-input { | ||
| font-size: 8px; | ||
| padding: 7px 8px; | ||
| border-width: 2px; | ||
| } | ||
|
|
||
| .or-add-btn { | ||
| font-size: 7px; | ||
| padding: 7px 10px; | ||
| border-width: 2px; | ||
| } | ||
|
|
||
| .or-add-error { | ||
| font-size: 6px; | ||
| } | ||
|
|
||
| .or-actions { | ||
| padding: 10px 8px; | ||
| gap: 6px; | ||
| border-width: 2px; | ||
| } | ||
|
|
||
| .or-staged-summary { | ||
| padding: 6px 8px; | ||
| border-width: 2px; | ||
| } | ||
|
|
||
| .or-staged-count { | ||
| font-size: 7px; | ||
| } | ||
|
|
||
| .or-staged-badge { | ||
| font-size: 6px; | ||
| } | ||
|
|
||
| .or-discard-btn { | ||
| font-size: 6px; | ||
| padding: 4px 8px; | ||
| border-width: 2px; | ||
| } | ||
|
|
||
| .or-operate-btn { | ||
| font-size: 10px; | ||
| padding: 10px 16px; | ||
| border-width: 2px; | ||
| letter-spacing: 1px; | ||
| box-shadow: | ||
| inset 0 0 0 1px rgba(56, 232, 88, 0.2), | ||
| 0 0 8px rgba(56, 232, 88, 0.25), | ||
| 0 2px 0 #041a04; | ||
| } | ||
|
|
||
| .or-close-btn { | ||
| font-size: 7px; | ||
| padding: 6px 10px; | ||
| border-width: 2px; | ||
| box-shadow: 0 2px 0 #080838; | ||
| } | ||
|
|
||
| .or-diff-preview { | ||
| border-width: 2px; | ||
| padding: 8px; | ||
| } | ||
|
|
||
| .or-diff-title { | ||
| font-size: 7px; | ||
| } | ||
|
|
||
| .or-diff-row { | ||
| font-size: 7px; | ||
| padding: 2px 6px; | ||
| } | ||
|
|
||
| .chart-operate-row { | ||
| flex-direction: column; | ||
| align-items: stretch; | ||
| gap: 6px; | ||
| } | ||
|
|
||
| .rx-action-operate { | ||
| font-size: 8px !important; | ||
| padding: 8px 12px !important; | ||
| border-width: 2px !important; | ||
| box-shadow: | ||
| 0 0 6px rgba(56, 232, 232, 0.2), | ||
| 0 2px 0 #040820 !important; | ||
| } | ||
|
|
||
| .chart-operate-hint { | ||
| font-size: 6px; | ||
| text-align: center; | ||
| } | ||
|
|
||
| /* ── Doctor's Notes mobile ── */ | ||
| .doctor-note-text { | ||
| font-size: 9px; | ||
| } | ||
|
|
||
| .dx-hint { | ||
| font-size: 7px; | ||
| padding-left: 20px; | ||
| } | ||
|
|
||
| /* ── Guided mode mobile ── */ | ||
| .guide-card-header { | ||
| font-size: 9px; | ||
| padding: 6px 8px; | ||
| } | ||
|
|
||
| .guide-card-body { | ||
| padding: 8px; | ||
| } | ||
|
|
||
| .guide-prescription { | ||
| font-size: 9px; | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| .guide-tag { | ||
| font-size: 6px; | ||
| } | ||
|
|
||
| .guide-recommendation { | ||
| font-size: 8px; | ||
| } | ||
|
|
||
| .guide-card-actions { | ||
| padding: 6px 8px; | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| .guide-btn { | ||
| font-size: 7px; | ||
| padding: 4px 8px; | ||
| } | ||
|
|
||
| .guide-resume-bar { | ||
| padding: 4px 8px; | ||
| } | ||
|
|
||
| .guide-btn-resume { | ||
| font-size: 7px; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Mobile overrides are still desktop-first patches.
The new Operating Room and guide mobile styles are appended under @media (max-width: 768px) instead of using mobile defaults and scaling up. It works, but it’s against the house protocol.
As per coding guidelines, "Mobile-first media queries."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@style.css` around lines 3140 - 3396, The CSS places mobile rules inside a
`@media` (max-width: 768px) block (desktop-first); refactor to mobile-first by
moving the mobile-specific declarations for the Operating Room and Guided Mode
components (e.g., .or-editor-header, .or-editor-title, .or-tray, .or-relay-row,
.or-add-input, .or-operate-btn, .guide-card-header, .guide-prescription,
.guide-btn, etc.) out of the max-width media query into the base stylesheet as
the default mobile styles, and invert the current media query to a desktop
override (use `@media` (min-width: 769px)) containing only the rules that need to
change for larger screens so desktop becomes the override layer.
1 participant
The marmot colony's medical wing gets a full renovation — a new Operating Room for relay management with auto-staging from prescriptions, a guided step-by-step treatment flow with FIX IT buttons, doctor's recommendations at three touchpoints (post-audit notes, per-finding hints, pre-OR briefing), and five one-click fix actions (rebroadcast, unify relays, delete orphaned KPs, purge NIP-04, reduce relay bloat). Eleven UI/UX improvements round it out: click-to-advance dialog with scroll-back, always-visible NIP-07 sign-in, grouped STATUS EFFECTS with collapsible pass section, action-specific button labels, cancel-during-audit, full-width OR banner, mobile relay health visibility, sticky guide resume bar, readable finding hints, and auto-collapsed relay rows when the patient chart renders.
Summary
js/operate/): Full relay list editor across k10002/k10050/k10051/k3 with health diagnostics, staged changes, read/write toggles, and procedure execution via NIP-07js/operate/autostage.js): Pre-stages fixes from audit prescriptions on OR entry (invalid URL removal, relay unification, inbox relay suggestions, orphaned KP fixes, read/write corrections)js/operate/guide.js): Step-by-step prescription walker with FIX IT/SKIP/NEXT controls, prescription-to-action mapping, and manual mode togglegenerateDoctorNotes()with 8 verdict-based note keys,FINDING_HINTSwith 30+ per-finding recommendations, personality-driven text across all 8 doctor stylesjs/actions.js):deleteOrphanedKeyPackages()anddeleteDeprecatedKind4()with relay scanning, kind 5 deletion signing, and batch publishing