docs: cut the comments back to what a reader will still need

Review found the comment load indefensible, worst on backend code at 43%
of added lines. Every comment this branch added or touched is now at most
two lines, and the ones that only restated the code, narrated a decision
already visible in the diff, or justified the PR to its reviewer are
gone. What survives is the reasoning a reader cannot recover from the
code: why markFilesRemoved keys on the absence of a source, why the
divider boundary is frozen as an id, why the contexts are read raw, why
"/" is not a destination.

Two were not merely long but wrong, describing a retry this PR no longer
has: the usePolicyAutoRun gate cited notificationActions.adopt, and its
sibling cited "Decrypt and retry".

857 comment lines to 446 across the branch, backend code 273 to 121.

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionException.java

@@ -4,20 +4,14 @@ import org.springframework.http.HttpStatus;
 
 import lombok.Getter;
 
-/**
- * Why an action could not be dispatched. Thrown carrying a {@link Reason} rather than an HTTP
- * status, so the service stays web-agnostic and only a controller ever reaches for {@link
- * #statusOf}.
- */
+/** Carries a {@link Reason} rather than an HTTP status, so the service stays web-agnostic. */
 @Getter
 public class FailureActionException extends RuntimeException {
 
     public enum Reason {
         /**
-         * No such event, it belongs to another team, or the caller's team did not resolve. One
-         * reason for all three, so the response does not vary with which it was. Unrelated to
-         * {@link FailureKind#UNKNOWN}, which is an unclassified failure rather than a refused
-         * action.
+         * No such event, another team's, or an unresolved team: one reason, so the answer cannot
+         * vary.
          */
         EVENT_NOT_FOUND,
 
@@ -25,16 +19,11 @@ public class FailureActionException extends RuntimeException {
         ACTION_NOT_RECOGNISED,
 
         /**
-         * The action exists but this kind does not declare it, so an incoherent pairing (releasing
-         * a document whose destination is what failed) cannot be dispatched even by hand. Reachable
-         * as of the kinds that stopped offering {@link FailureActionId#ACKNOWLEDGE}.
+         * The action exists but this kind does not offer it, so it cannot be dispatched by hand.
          */
         ACTION_NOT_DECLARED,
 
-        /**
-         * The kind offers it, but the client is what runs it (see {@link
-         * FailureActionId.Execution#CLIENT}), so it is refused rather than half-performed.
-         */
+        /** Offered, but the client is what runs it, so refused rather than half-performed. */
         ACTION_NOT_DISPATCHABLE,
 
         /** The event is already closed, so no further transition is possible. */
@@ -47,20 +36,14 @@ public class FailureActionException extends RuntimeException {
         this(reason, message, null);
     }
 
-    /** For a refusal that follows from a lower-level failure, so its stack is not dropped. */
     public FailureActionException(Reason reason, String message, Throwable cause) {
         super(message, cause);
         this.reason = reason;
     }
 
     /**
-     * The HTTP status each refusal reason answers with. Lives with the reasons it maps, so the two
-     * surfaces that dispatch actions, the failure queue and the notification bell, cannot drift
-     * apart and a client's error handling does not depend on which one it called.
-     *
-     * <p>A closed row is a conflict rather than a bad request: the request was well-formed and
-     * would have been valid a moment earlier. A missing row is a 404 whether it never existed,
-     * belongs to another team or is a colleague's, so trying does not confirm which.
+     * Lives with the reasons it maps, so every surface that dispatches an action answers alike. A
+     * closed row is a conflict, not a bad request: it was well-formed and valid a moment earlier.
      */
     public static HttpStatus statusOf(Reason reason) {
         return switch (reason) {

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionId.java

@@ -3,42 +3,27 @@ package stirling.software.proprietary.failure;
 import lombok.Getter;
 
 /**
- * The actions a {@link FailureKind} may declare, and which side of the wire runs each one.
- *
- * <p>The dispositions are the server's: they change how the event is shown and touch nothing else,
- * which is what makes them valid for every kind including {@link FailureKind#UNKNOWN}. Everything
- * else is the client's, because the server holds an opaque id for the document and nothing more.
- *
- * <p>Client actions are still declared here rather than invented by each client, so the server
- * keeps deciding what a kind offers, in what order and under what label.
+ * The actions a {@link FailureKind} may declare. Client actions are declared here rather than
+ * invented per client, so the server keeps deciding what a kind offers, in what order and labelled
+ * how.
  */
 @Getter
 public enum FailureActionId {
 
-    /**
-     * "Seen, and I own it." No kind offers this any more, but rows shipped before that are already
-     * {@code ACKNOWLEDGED} and must stay readable and closable.
-     */
+    /** No kind offers this any more, but rows already {@code ACKNOWLEDGED} must stay closable. */
     ACKNOWLEDGE(Execution.SERVER, "Acknowledge"),
 
-    /** "No remediation will happen; close it." See {@link DismissAction}. */
     DISMISS(Execution.SERVER, "Dismiss"),
 
-    /** Open the document this incident is about. Only its owner's client can resolve the id. */
+    /** Only the owner's client can resolve the id. */
     VIEW_FILE(Execution.CLIENT, "View file"),
 
-    /** Open the run behind it, for whoever reviews the team rather than owns the document. */
     VIEW_IN_PROCESSOR(Execution.CLIENT, "View in processor");
 
-    /**
-     * Which side of the wire runs the action. The dispatch endpoint refuses a {@code CLIENT} id, so
-     * "the client does this one" is enforced rather than merely documented.
-     */
+    /** Dispatch refuses a {@code CLIENT} id, so this is enforced rather than merely documented. */
     public enum Execution {
 
-        /**
-         * Dispatchable, and {@link FailureActionRegistry} requires a {@link FailureAction} bean.
-         */
+        /** {@link FailureActionRegistry} requires a {@link FailureAction} bean for these. */
         SERVER,
 
         /**
@@ -49,7 +34,7 @@ public enum FailureActionId {
 
     private final Execution execution;
 
-    /** English fallback, used when the client has no translation for the action's label key. */
+    /** English fallback, for a client with no translation for the label key. */
     private final String defaultLabel;
 
     FailureActionId(Execution execution, String defaultLabel) {
@@ -57,7 +42,7 @@ public enum FailureActionId {
         this.defaultLabel = defaultLabel;
     }
 
-    /** Whether the server runs it itself, which is also whether it can be dispatched. */
+    /** Also whether it can be dispatched. */
     public boolean runsOnServer() {
         return execution == Execution.SERVER;
     }

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureActionRegistry.java

@@ -17,8 +17,8 @@ import lombok.extern.slf4j.Slf4j;
  * point: because kinds declare action ids as data, one could name an action nobody implements,
  * which would otherwise show up as a button that 400s rather than as a failed boot.
  *
- * <p>Only {@link FailureActionId.Execution#SERVER} ids belong here. A client action has no bean by
- * design, and a bean for one is refused outright because dispatch could never reach it.
+ * <p>Only {@link FailureActionId.Execution#SERVER} ids belong here: a bean for a client action is
+ * refused, because dispatch could never reach it.
  */
 @Slf4j
 @Service
@@ -49,10 +49,7 @@ public class FailureActionRegistry {
         }
     }
 
-    /**
-     * Fail fast if any kind declares a server action with no handler, naming every gap rather than
-     * the first, so one boot tells you everything that is missing.
-     */
+    /** Names every gap rather than the first, so one boot tells you everything that is missing. */
     @PostConstruct
     void verifyEveryDeclaredActionHasAHandler() {
         List<String> gaps =

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureAudience.java

@@ -1,20 +1,15 @@
 package stirling.software.proprietary.failure;
 
 /**
- * Who an offered action is for. The read scope has already decided the caller may see the incident;
- * this decides which of its actions are theirs to take.
- *
- * <p>The distinction is possession, not seniority: a reviewer clearing up after a colleague cannot
- * supply that colleague's password or reach a document only their browser holds.
+ * Who an offered action is for, the read scope having already decided they may see the incident.
+ * The distinction is possession, not seniority: a reviewer cannot reach a document only its owner
+ * holds.
  */
 public enum FailureAudience {
-
-    /** The person whose work failed, whose own client still holds the document. */
     OWNER,
 
     /** Anyone who triages the team's incidents, whoever hit them. */
     TEAM_REVIEWER,
 
-    /** Everyone the incident is shown to at all. */
     ANYONE_WHO_SEES
 }

app/proprietary/src/main/java/stirling/software/proprietary/failure/FailureKind.java

@@ -24,14 +24,8 @@ import lombok.Getter;
  * The registry of failure kinds, described as data: a stable id, i18n keys and an English fallback
  * like {@code ExceptionUtils.ErrorCode}, plus the facets a review surface needs.
  *
- * <p>Actions are declared here but run elsewhere: a server action in a {@link FailureAction} bean
- * resolved by id, a client action in the browser that holds the document. Either way a new kind
- * ships as a registry entry plus copy. Two members today: {@link #UNKNOWN} gives every failed run a
- * record, and kinds get promoted out of it as production shows what occurs.
- *
- * <p>Each offer also says who it is for, because the same incident is read by the person who hit it
- * and by whoever reviews after them: only the owner holds the document, only a reviewer wants the
- * run.
+ * <p>A new kind ships as a registry entry plus copy. Each offer also says who it is for, since one
+ * incident is read both by whoever hit it and by whoever reviews after them.
  */
 @Getter
 public enum FailureKind {
@@ -42,8 +36,6 @@ public enum FailureKind {
             FailureScope.FILE,
             errorCodes("E004"),
             fallback("This document is password-protected, so the pipeline could not read it."),
-            // Only the owner holds the document, so the file is theirs to open; a reviewer
-            // gets the run instead, and anyone who sees the row may close it.
             offer(VIEW_FILE, OWNER),
             offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
             offer(DISMISS, ANYONE_WHO_SEES)),
@@ -55,11 +47,8 @@ public enum FailureKind {
             FailureScope.RUN,
             noErrorCodes(),
             fallback("This run failed for a reason Stirling does not yet recognise."),
-            // Nothing here is known to be fixable, so the offers are the places to look:
-            // the owner their document, a reviewer the run, and anyone may close the row.
-            // Declared in the same order as every other kind, because declaration order is
-            // display order: the document leads wherever it is offered, so a reader is not
-            // asked to re-learn which button leads from one failure to the next.
+            // Same order as every other kind: declaration order is display order, so the document
+            // leads wherever it is offered rather than moving between failures.
             offer(VIEW_FILE, OWNER),
             offer(VIEW_IN_PROCESSOR, TEAM_REVIEWER),
             offer(DISMISS, ANYONE_WHO_SEES));
@@ -110,19 +99,15 @@ public enum FailureKind {
     }
 
     /**
-     * One action this kind offers: who it is for, and the key to label it by. One ordered list
-     * rather than ids plus parallel maps of audiences and label overrides, which could disagree
-     * with each other.
+     * One ordered list rather than ids plus parallel maps of audiences and labels, which could
+     * disagree with each other.
      *
      * @param labelKeySuffix key under {@code portal.failures.action.}, or null for the generic
      *     label
      */
     private record Offer(FailureActionId id, FailureAudience audience, String labelKeySuffix) {}
 
-    /**
-     * An action this kind offers, for whoever can actually take it, labelled by the shared wording.
-     * Declaration order is display order.
-     */
+    /** Declaration order is display order. */
     private static Offer offer(FailureActionId id, FailureAudience audience) {
         return new Offer(id, audience, null);
     }

app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventRepository.java

@@ -98,16 +98,8 @@ public interface FileRunEventRepository extends JpaRepository<FileRunEventEntity
      * Close the incidents about documents their owner deleted from the editor: the queue is what
      * needs attention, and a document that no longer exists needs none.
      *
-     * <p>Restricted to that owner's own rows. File ids are minted by the client, so scoping on team
-     * alone would let one caller close a colleague's incidents by naming ids.
-     *
-     * <p>Scoped by the ABSENCE OF A SOURCE rather than by origin, which is what tells the two id
-     * spaces in {@code fileId} apart. An editor report and an attended policy run both carry the id
-     * the client minted for its own document, so both are closable by the client that holds it; a
-     * source-fed run carries a one-way hash of a path or key that was never on any device, so no
-     * client can legitimately name it. Keying on {@code origin = TOOL} instead left a user's own
-     * policy failures in the queue after they deleted the very document those failures were about,
-     * because the run had been recorded by the processor rather than reported by the editor.
+     * <p>Scoped by the absence of a source rather than by origin: a source-fed run's {@code fileId}
+     * is a hash no client can name. Narrowed to the owner's own rows, since clients mint the ids.
      */
     @Modifying(clearAutomatically = true)
     @Transactional

app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventService.java

@@ -22,8 +22,7 @@ import stirling.software.proprietary.policy.config.PolicyManagementAuthority;
  * enabled so single-user deployments keep working. When the team cannot be resolved the caller
  * reads nothing; see {@link #readScope()}.
  *
- * <p>Seeing an incident and being able to act on it are separate questions: the read scope decides
- * the first, {@link #availableActions} the second.
+ * <p>The read scope decides who sees an incident; {@link #availableActions} decides who may act.
  */
 @Slf4j
 @Service
@@ -135,8 +134,8 @@ public class FileRunEventService {
         // failures they caused. Someone who fixes their own problem should not have to ask a leader
         // to clear the row.
         //
-        // Audience decides what is offered, not what may be dispatched, so this read scope is the
-        // whole gate. A server action aimed at OWNER alone would need its own guard here.
+        // Audience decides what is offered, not what may be dispatched, so this scope is the whole
+        // gate. A server action aimed at OWNER alone would need its own guard here.
         FileRunEvent event = requireVisible(eventId);
 
         FailureActionId resolvedId = parseActionId(actionId);
@@ -148,8 +147,7 @@ public class FileRunEventService {
                     FailureActionException.Reason.ACTION_NOT_DECLARED,
                     "Kind " + event.kind().getId() + " does not offer action " + resolvedId);
         }
-        // Declared, and still not the server's to run: without this a client could post VIEW_FILE
-        // and be answered as though something had happened, when nothing here has the file.
+        // Without this a client could post VIEW_FILE and be answered as though something happened.
         if (!resolvedId.runsOnServer()) {
             throw new FailureActionException(
                     FailureActionException.Reason.ACTION_NOT_DISPATCHABLE,
@@ -173,10 +171,7 @@ public class FileRunEventService {
         return action.execute(event, inputs == null ? Map.of() : inputs, currentActor());
     }
 
-    /**
-     * The event, if this caller may act on it at all. Reported as "no such event" rather than a
-     * refusal, so a member cannot learn that a colleague's incident exists by trying to close it.
-     */
+    /** "No such event" rather than a refusal, so trying does not confirm a colleague's exists. */
     private FileRunEvent requireVisible(String eventId) {
         ReadScope scope = readScope();
         if (!scope.permitted()) {
@@ -192,7 +187,6 @@ public class FileRunEventService {
                 FailureActionException.Reason.EVENT_NOT_FOUND, "No such event: " + eventId);
     }
 
-    /** Whose incident this is, as far as the caller is concerned. See {@link Ownership}. */
     public Ownership ownershipOf(FileRunEvent event) {
         if (event.actor() == null) {
             return Ownership.UNOWNED;
@@ -202,22 +196,18 @@ public class FileRunEventService {
     }
 
     /**
-     * Which of an event's declared actions this caller is offered, and which are usable right now,
-     * so the client never renders a button that would be refused. An action outside the caller's
-     * audience is dropped rather than disabled: a greyed-out "Decrypt and retry" would read as a
-     * permission problem when it is simply not their document.
+     * Offers resolved for one caller, so no client renders a button that would be refused. Outside
+     * their audience is dropped, not disabled: greyed out would read as a permission problem.
      */
     public List<AvailableAction> availableActions(FileRunEvent event) {
         Ownership ownership = ownershipOf(event);
         boolean reviewsTeam = reviewsTeam();
         boolean closed = event.status().terminal();
-        // Nobody's client is holding the document, so an owner action has nothing to act on. A
-        // login-disabled deployment is excluded: its rows are unowned only because it has no users,
-        // and its one operator owns everything they can see.
+        // Login disabled is excluded: its rows are unowned only for want of users, and its one
+        // operator owns everything they can see.
         boolean unattended = enforced() && ownership == Ownership.UNOWNED;
-        // No document reference at all, so an owner action has nothing to name even when the owner
-        // is right here holding the file. Answered here rather than left to the client, which would
-        // otherwise report "not on this device" about a document the row never identified.
+        // Answered here, or the client reports "not on this device" about a document the row never
+        // identified in the first place.
         boolean documentless = event.fileId() == null || event.fileId().isBlank();
         return event.kind().getOfferedActions().stream()
                 .filter(offer -> offeredTo(offer.audience(), ownership, reviewsTeam))
@@ -235,10 +225,7 @@ public class FileRunEventService {
         return new AvailableAction(offer.id(), offer.labelKey(), reason == null, reason);
     }
 
-    /**
-     * Why an offer cannot be taken right now, or null when it can. Closed wins over everything,
-     * then the owner-only reasons, most specific first.
-     */
+    /** Closed wins over everything, then the owner-only reasons, most specific first. */
     private static String disabledReasonFor(
             FailureAudience audience, boolean closed, boolean unattended, boolean documentless) {
         if (closed) {
@@ -253,11 +240,7 @@ public class FileRunEventService {
         return documentless ? DOCUMENTLESS_REASON_KEY : null;
     }
 
-    /**
-     * Whether an offer aimed at {@code audience} is this caller's to take. An unattended incident
-     * has no owner, so its reviewer inherits the owner's actions; otherwise nobody is offered them
-     * at all.
-     */
+    /** An unattended incident has no owner, so its reviewer inherits the owner's actions. */
     private static boolean offeredTo(
             FailureAudience audience, Ownership ownership, boolean reviewsTeam) {
         return switch (audience) {
@@ -268,7 +251,7 @@ public class FileRunEventService {
         };
     }
 
-    /** Whether the caller triages the whole team's incidents. Login disabled has no roles. */
+    /** Login disabled has no roles, so its one operator triages everything. */
     private boolean reviewsTeam() {
         return !enforced() || policyManagementAuthority.canEditPolicies();
     }
@@ -343,7 +326,6 @@ public class FileRunEventService {
         return applicationProperties.getSecurity().isEnableLogin();
     }
 
-    /** One action as offered to one caller about one event, with its availability resolved. */
     public record AvailableAction(
             FailureActionId id, String labelKey, boolean enabled, String disabledReasonKey) {}
 }

app/proprietary/src/main/java/stirling/software/proprietary/failure/FileRunEventView.java

@@ -61,9 +61,8 @@ public record FileRunEventView(
     }
 
     /**
-     * One button, as offered to this caller about this row. {@code defaultLabel} and {@code
-     * execution} are here for the reason {@code defaultTitle} is on the row: a client can then
-     * render, and route, an action it was never built with. Declaration order is display order.
+     * {@code defaultLabel} and {@code execution} let a client render and route an action it was
+     * never built with. Declaration order is display order.
      */
     public record ActionView(
             String id,
@@ -73,7 +72,6 @@ public record FileRunEventView(
             boolean enabled,
             String disabledReasonKey) {
 
-        /** Public because the notification bell projects the same resolved offers. */
         public static ActionView of(FileRunEventService.AvailableAction action) {
             return new ActionView(
                     action.id().name(),

app/proprietary/src/main/java/stirling/software/proprietary/failure/Ownership.java

@@ -1,21 +1,17 @@
 package stirling.software.proprietary.failure;
 
 /**
- * Whose incident this is, from the point of view of whoever is reading it. Derived on read and
- * never persisted: the same row is {@code MINE} to the member who hit it and {@code THEIRS} to the
- * leader reviewing after them, so a stored answer would be wrong for everyone but one person.
+ * Whose incident this is, from the reader's point of view. Derived on read, never persisted: one
+ * row is {@code MINE} to whoever hit it and {@code THEIRS} to the leader reviewing after them.
  */
 public enum Ownership {
-
-    /** The caller's own failure: their work, and their client holding the document. */
     MINE,
 
     /** A colleague's, visible because the caller reviews the team. */
     THEIRS,
 
     /**
-     * Nobody's. An unattended run failed, with a folder, bucket or webhook as its only attribution,
-     * so there is no owner to hand the resolution to.
+     * An unattended run: a folder, bucket or webhook is its only attribution, so there is no owner.
      */
     UNOWNED
 }

app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationController.java

@@ -14,13 +14,8 @@ import io.swagger.v3.oas.annotations.tags.Tag;
 import lombok.RequiredArgsConstructor;
 
 /**
- * The caller's notifications. Open to any authenticated user, unlike the failure endpoints it draws
- * on: each source scopes its own rows and resolves its own actions, so a member is told about their
- * own failures and a leader about their team's.
- *
- * <p>Read-only: every action a notification offers is one the client runs on its own device, so
- * there is nothing to post back here yet. Ids are still prefixed with their source, so the bell is
- * never given the producing row's id; see {@link NotificationSource}.
+ * Open to any authenticated user, unlike the failure endpoints it draws on: each source scopes its
+ * own rows. Read-only, because every action a notification offers runs on the client's own device.
  */
 @RestController
 @RequestMapping("/api/v1/notifications")
@@ -29,7 +24,6 @@ import lombok.RequiredArgsConstructor;
 @Tag(name = "Notifications", description = "Things worth telling the caller about")
 public class NotificationController {
 
-    /** One page of a bell. Enough to fill a panel; the badge counts what it is given. */
     private static final int DEFAULT_LIMIT = 20;
 
     private static final int MAX_LIMIT = 100;
@@ -47,8 +41,6 @@ public class NotificationController {
         return new NotificationsResponse(notifications.list(capped));
     }
 
-    /**
-     * Wrapped rather than a bare array so paging or a total can be added without breaking clients.
-     */
+    /** Wrapped so paging or a total can be added without breaking clients. */
     public record NotificationsResponse(List<NotificationView> notifications) {}
 }

app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationService.java

@@ -11,13 +11,8 @@ import stirling.software.proprietary.failure.FileRunEventService;
 import stirling.software.proprietary.failure.FileRunEventView;
 
 /**
- * Assembles the caller's notifications from whatever produces them. Derived on read rather than
- * stored: there is one source today, and a table would need a write path, a retention story and a
- * per-user read model before it earned itself.
- *
- * <p>Who sees what is decided by each source rather than here. {@link FileRunEventService} already
- * scopes its reads and resolves each row's actions against that reader, so this cannot widen either
- * by accident.
+ * Derived on read rather than stored: one source today, and a table would need a write path,
+ * retention and a per-user read model first. Each source scopes its own rows, so this cannot widen.
  */
 @Service
 @RequiredArgsConstructor
@@ -25,17 +20,12 @@ public class NotificationService {
 
     private final FileRunEventService fileRunEvents;
 
-    /**
-     * The caller's notifications, newest first. Only open failures: a dismissed or resolved one has
-     * been dealt with and is not news.
-     */
+    /** Newest first, and only open failures: one already dealt with is not news. */
     public List<NotificationView> list(int limit) {
         return fileRunEvents.list(null, null, limit).stream().map(this::fromFailure).toList();
     }
 
-    /**
-     * One failure as a notification, with its row id prefixed on the way out and never sent bare.
-     */
+    /** Prefixes the row id on the way out, so it is never sent bare. */
     private NotificationView fromFailure(FileRunEvent event) {
         return new NotificationView(
                 NotificationSource.FAILURE.qualify(event.id()),
@@ -54,7 +44,6 @@ public class NotificationService {
                 event.occurrences(),
                 event.createdAt(),
                 event.lastSeenAt(),
-                // The failure surface's own offers, filtered to the ones the client itself runs.
                 // A disposition such as Dismiss belongs to the review surface, not the bell.
                 fileRunEvents.availableActions(event).stream()
                         .filter(action -> !action.id().runsOnServer())

app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationSource.java

@@ -3,25 +3,18 @@ package stirling.software.proprietary.notification;
 import java.util.Locale;
 
 /**
- * Which subsystem produced a notification. One member today; the point of the field is that a
- * client already branches on it, so a second source needs no client change to be ignorable.
- *
- * <p>Every notification id is prefixed with its source, so a client never holds the producing row's
- * own id and cannot hand it to that source's endpoints by accident.
+ * Which subsystem produced a notification. Every id is prefixed with it, so a client never holds
+ * the producing row's own id and cannot reach that source's endpoints by accident.
  */
 public enum NotificationSource {
-
-    /** A recorded run failure: see {@code stirling.software.proprietary.failure}. */
     FAILURE;
 
     private static final char SEPARATOR = ':';
 
-    /** The prefix an id from this source carries, including the separator. */
     public String prefix() {
         return name().toLowerCase(Locale.ROOT) + SEPARATOR;
     }
 
-    /** One of this source's own row ids, as the client sees it. */
     public String qualify(String sourceRowId) {
         return prefix() + sourceRowId;
     }

app/proprietary/src/main/java/stirling/software/proprietary/notification/NotificationView.java

@@ -10,35 +10,8 @@ import stirling.software.proprietary.failure.FileRunEventView;
 import stirling.software.proprietary.failure.Ownership;
 
 /**
- * One thing worth telling the caller about, flattened to what a bell needs to render it and act on
- * it. Sources project onto this rather than exposing their own shape, so a client renders and
- * triages a notification without knowing which subsystem produced it.
- *
- * <p>The facets read as failure vocabulary because failures are the only source so far. A later
- * source projects onto the same fields or leaves them null; what a client must not do is infer the
- * source from which fields are populated, which is what {@code source} is for.
- *
- * @param id unique across every source, so a client can key and de-duplicate on it alone. Prefixed
- *     with its source, and the only handle the notification endpoints accept
- * @param source which subsystem produced it, for grouping and for the click-through
- * @param kindId the source's own id for what happened, so a client can group or route on it
- * @param origin what was running when it happened, e.g. one tool or a policy
- * @param ownership whether this is the caller's own to act on, derived per read
- * @param severity how loudly to show it
- * @param status where the underlying row stands, so a bell can show what has already been dealt
- *     with
- * @param titleKey i18n key for the headline, with {@code defaultTitle} as the English fallback
- * @param detail the one-line body, as the source recorded it
- * @param fileId opaque reference to the document, never a name. Two id spaces share it, and {@code
- *     sourceId} is what tells them apart; see {@code PolicyEngine#runPolicy}
- * @param sourceId which folder, bucket or webhook fed the failing run, and null for an attended run
- *     the caller started themselves
- * @param policyId which policy the failing run belonged to; null for work with no policy around it
- * @param occurrences how many times this same thing has happened, at least 1
- * @param createdAt when it first happened; the client orders and marks unread on this
- * @param lastSeenAt when it last happened, equal to {@code createdAt} for a one-off
- * @param actions the same resolved offers the failure surface renders, already filtered to this
- *     caller, so the bell needs no rules of its own
+ * A source's row flattened to what a bell renders. {@code fileId} is an opaque reference, never a
+ * name, and two id spaces share it: {@code sourceId} tells them apart.
  */
 public record NotificationView(
         String id,

app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyController.java

@@ -680,13 +680,9 @@ public class PolicyController {
     }
 
     /**
-     * The document reference to record against a failure of this run: the caller's own opaque id,
-     * but only when the run carries exactly one primary document. A recorded incident has a single
-     * file reference, so naming one document out of several would attribute the failure to
-     * whichever happened to be bound first, which is worse than naming none.
-     *
-     * <p>Counted off the resolved inputs rather than the raw multipart list, so an empty part
-     * cannot make a single-document run look like two.
+     * Only for a single-document run: an incident holds one file reference, so naming one of
+     * several would attribute the failure to whichever bound first. Counted off resolved inputs,
+     * not parts.
      */
     private static String documentReferenceFor(PolicyRunFiles files, PolicyInputs inputs) {
         String fileId = files.getFileId();

app/proprietary/src/main/java/stirling/software/proprietary/policy/controller/PolicyRunFiles.java

@@ -31,14 +31,8 @@ public class PolicyRunFiles {
     private List<NamedAsset> assets = new ArrayList<>();
 
     /**
-     * The caller's own opaque reference to the document it is running on, recorded against any
-     * failure of this run so the client that filed it can resolve the row back to that document.
-     * Without it an attended failure names no document, and every action that needs the bytes is
-     * unreachable for the one person holding them.
-     *
-     * <p>Opaque by contract and never a name: the server stores it, hands it back and reads nothing
-     * out of it. Only honoured for a single-document run, see {@code
-     * PolicyController#documentReferenceFor}.
+     * Recorded against any failure of this run, so the client can resolve the row back to its
+     * document. Opaque by contract, never a name, and only honoured for a single-document run.
      */
     @Schema(
             description =

app/proprietary/src/main/java/stirling/software/proprietary/policy/engine/PolicyEngine.java

@@ -151,12 +151,8 @@ public class PolicyEngine {
      * As {@link #runPolicy(Policy, PolicyInputs, PolicyProgressListener)}, recording which source
      * fed the run and its opaque reference to the document. The first says where an unattended
      * failure came from; the second says which document, and is what lets the same document failing
-     * again fold into one incident.
-     *
-     * <p>An attended run (a user's upload) has no source, and that absence is the discriminator for
-     * the two id spaces {@code fileIdentity} carries: with no source it is the reference the client
-     * minted for its own document, otherwise a source's one-way hash of a path or key. Either way
-     * this engine only carries it, and reads nothing out of it.
+     * again fold into one incident. With no source {@code fileIdentity} is the client's own
+     * reference, with one it is that source's hash; this engine only carries it either way.
      */
     public PolicyRunHandle runPolicy(
             Policy policy,

app/proprietary/src/main/java/stirling/software/proprietary/policy/engine/PolicyRunner.java

@@ -121,13 +121,8 @@ public class PolicyRunner {
      * The supplied documents are still counted against the virtual {@link EditorSource}, scoped to
      * the policy's team, so the Sources overview reports the whole team's editor throughput.
      *
-     * <p>Attended: no source fed this run, which is what later tells its recorded failures apart
-     * from an unattended sweep's.
-     *
-     * @param documentReference the caller's own opaque reference to the single document it is
-     *     running on, or null when it supplied none (or supplied several). Passed through
-     *     untouched, and lands where an unattended run's {@link ResolvedInput#forFile} identity
-     *     does; see {@code PolicyEngine#runPolicy} for how the two are told apart.
+     * @param documentReference the caller's own opaque reference to the single document it runs on,
+     *     or null when it supplied none or several. Passed through untouched.
      */
     public PolicyRunHandle runWith(
             Policy policy,

app/proprietary/src/test/java/stirling/software/proprietary/failure/CheckConstrainedEnumsTest.java

@@ -9,11 +9,8 @@ import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
 /**
- * Pins the five enums that {@code file_run_events} stores as strings behind CHECK constraints. The
- * shipped migration spells out the permitted values, so adding one to {@code status}, {@code
- * origin}, {@code stage}, {@code severity} or {@code scope} is a schema change dressed up as a Java
- * change: it compiles, then fails against a real database on the row that most needed recording.
- * Reordering is free, since values are persisted by name.
+ * Pins the five enums {@code file_run_events} stores behind CHECK constraints: adding a value is a
+ * schema change dressed as a Java one, compiling here and failing against a real database.
  */
 class CheckConstrainedEnumsTest {
 
@@ -36,8 +33,7 @@ class CheckConstrainedEnumsTest {
     @Test
     @DisplayName("the facets added since are derived, not stored")
     void nothingAddedToTheModelReachedTheTable() throws Exception {
-        // Audience, slot, execution and ownership are resolved per reader, so a column for any of
-        // them would hold the wrong answer for everybody but one person.
+        // Resolved per reader, so a column would hold the wrong answer for all but one person.
         List<Class<?>> persisted =
                 Arrays.stream(FileRunEventEntity.class.getDeclaredFields())
                         .filter(field -> !field.isSynthetic())
@@ -50,8 +46,7 @@ class CheckConstrainedEnumsTest {
                         FailureActionId.class,
                         FailureActionId.Execution.class,
                         Ownership.class);
-        // kind_id stays a plain varchar with no CHECK, which is what lets a new kind ship without a
-        // migration while the five columns above cannot.
+        // A plain varchar with no CHECK, which is what lets a new kind ship without a migration.
         assertThat(FileRunEventEntity.class.getDeclaredField("kindId").getType())
                 .isEqualTo(String.class);
     }

app/proprietary/src/test/java/stirling/software/proprietary/failure/FailureKindTest.java

@@ -32,10 +32,7 @@ import stirling.software.common.util.ExceptionUtils;
  */
 class FailureKindTest {
 
-    /**
-     * One expected offer in full, rather than four separate extracting() assertions, so a
-     * declaration that pairs the right action with the wrong audience cannot pass.
-     */
+    /** In full, so a declaration pairing the right action with the wrong audience cannot pass. */
     private static FailureKind.OfferedAction offered(
             FailureActionId id, FailureAudience audience, String labelKeySuffix) {
         return new FailureKind.OfferedAction(
@@ -76,10 +73,9 @@ class FailureKindTest {
         @ParameterizedTest
         @EnumSource(FailureKind.class)
         void declaresItsActionsInTheSameOrderAsEveryOtherKind(FailureKind kind) {
-            // Declaration order is display order, and the first offer a reader can use is the one
-            // rendered as the row's primary. Two kinds listing the same actions in different orders
-            // therefore flip the solid button between rows, which reads as a bug rather than as
-            // emphasis. Asserted as a shared ranking so a kind added later cannot reintroduce it.
+            // Declaration order is display order and the first usable offer is the row's primary,
+            // so
+            // two kinds disagreeing would flip the solid button between rows.
             List<FailureActionId> ranking =
                     List.of(
                             FailureActionId.VIEW_FILE,
@@ -126,8 +122,7 @@ class FailureKindTest {
         @ParameterizedTest
         @EnumSource(FailureKind.class)
         void everyOfferSaysWhoItIsFor(FailureKind kind) {
-            // Read per row to decide what a caller is shown, so a missing one would be a button
-            // offered to whoever the null case happened to let through.
+            // Read per row to decide what a caller is shown, so a null would leak a button.
             for (FailureKind.OfferedAction offer : kind.getOfferedActions()) {
                 assertThat(offer.audience())
                         .as("%s offers %s", kind.getId(), offer.id())
@@ -138,8 +133,8 @@ class FailureKindTest {
         @ParameterizedTest
         @EnumSource(FailureKind.class)
         void offersEachActionAtMostOnce(FailureKind kind) {
-            // Declaration order is the client's tie-break, so the same action twice would be two
-            // buttons with one meaning, and labelKeyFor would silently answer for the first.
+            // The same action twice would be two buttons with one meaning, and labelKeyFor would
+            // answer for the first.
             assertThat(kind.getActions()).doesNotHaveDuplicates();
         }
 
@@ -238,8 +233,7 @@ class FailureKindTest {
 
         @Test
         void offersItsOwnerTheirDocumentAndTheRunToWhoeverReviews() {
-            // Nothing here is known to be fixable, so the offers are the places to look: the
-            // owner their document, a reviewer the run, and anyone may close the row.
+            // Nothing here is known to be fixable, so the offers are just the places to look.
             assertThat(FailureKind.UNKNOWN.getOfferedActions())
                     .containsExactly(
                             offered(FailureActionId.VIEW_FILE, OWNER, "viewFile"),
@@ -301,8 +295,7 @@ class FailureKindTest {
 
         @Test
         void offersTheDocumentToItsOwnerAndTheRunToItsReviewer() {
-            // The whole point of the audiences: only the owner holds the document, so a reviewer
-            // is offered the run and a way to close the row instead.
+            // The point of the audiences: only the owner holds the document.
             assertThat(FailureKind.INPUT_PASSWORD_PROTECTED.getOfferedActions())
                     .containsExactly(
                             offered(FailureActionId.VIEW_FILE, OWNER, "viewFile"),
@@ -315,8 +308,8 @@ class FailureKindTest {
 
         @Test
         void noKindOffersAcknowledgeAnyMore() {
-            // Kept in the vocabulary because rows are already ACKNOWLEDGED, and those must stay
-            // readable. Nothing offers it, so nothing can dispatch it either.
+            // Kept in the vocabulary for rows already ACKNOWLEDGED; offered by nothing, so
+            // dispatchable by nothing.
             for (FailureKind kind : FailureKind.values()) {
                 assertThat(kind.declares(FailureActionId.ACKNOWLEDGE))
                         .as("%s offers ACKNOWLEDGE", kind.getId())
@@ -326,8 +319,7 @@ class FailureKindTest {
 
         @Test
         void everyKindLabelsItsActionsWithTheSharedWordingToday() {
-            // The per-kind override still exists for wording that reads badly in context; nothing
-            // needs it now that Dismiss sits in an overflow menu, where the shared word is right.
+            // The per-kind override still exists for wording that reads badly in context.
             for (FailureKind kind : FailureKind.values()) {
                 for (FailureActionId action : kind.getActions()) {
                     assertThat(kind.labelKeyFor(action))

app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventServiceTest.java

@@ -85,9 +85,7 @@ class FileRunEventServiceTest {
     class Acknowledge {
 
         /**
-         * No kind offers ACKNOWLEDGE any more, so it cannot be dispatched; the bean stays for rows
-         * that are already {@code ACKNOWLEDGED}. Exercised directly so their transition stays
-         * covered.
+         * No kind offers it, so it cannot be dispatched; exercised directly for rows that have it.
          */
         private FileRunEvent acknowledge(FileRunEvent event, String actor) {
             return new AcknowledgeAction(store).execute(event, Map.of(), actor);
@@ -128,8 +126,6 @@ class FileRunEventServiceTest {
 
         @Test
         void anAlreadyAcknowledgedRowStaysReadableAndClosable() {
-            // The reason the id and its bean stay: a row in this state predates the retry actions
-            // and must not become a row nobody can act on.
             FileRunEvent event = given(FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
             acknowledge(event, ACTOR);
 
@@ -232,8 +228,7 @@ class FileRunEventServiceTest {
 
         @Test
         void anActionTheClientRunsIsRefusedRatherThanPretendedTo() {
-            // UNKNOWN offers VIEW_FILE, and the server has neither the document nor the tool.
-            // Answering 200 here would tell the client something happened when nothing did.
+            // Answering 200 would tell the client something happened when nothing did.
             FileRunEvent event = given(FailureKind.UNKNOWN, TEAM, "f1");
 
             assertThatThrownBy(() -> service.dispatch(event.id(), "VIEW_FILE", Map.of()))
@@ -247,8 +242,7 @@ class FileRunEventServiceTest {
 
         @Test
         void everyClientActionIsRefusedWhicheverKindDeclaresIt() {
-            // Asserted over the vocabulary rather than one id, so an action added on the client
-            // side cannot arrive dispatchable because nobody thought to test it.
+            // Over the whole vocabulary, so a client action added later cannot arrive dispatchable.
             for (FailureKind kind : FailureKind.values()) {
                 FileRunEvent event = given(kind, TEAM, "f-" + kind.getId());
                 for (FailureActionId action : kind.getActions()) {
@@ -337,8 +331,7 @@ class FileRunEventServiceTest {
 
         @Test
         void theSameRowIsMineToOnePersonAndTheirsToAnother() {
-            // Why it is derived and not stored: one stored answer would be wrong for everyone but
-            // the person it was stored for.
+            // Why it is derived: a stored answer would be wrong for everyone but one person.
             FileRunEvent event = givenHitBy(ACTOR, FailureKind.UNKNOWN, TEAM, "f1");
             assertThat(service.ownershipOf(event)).isEqualTo(Ownership.MINE);
 
@@ -360,8 +353,7 @@ class FileRunEventServiceTest {
 
         @Test
         void theOwnerIsOfferedTheirDocumentAndNotTheReviewersView() {
-            // A member reading their own password failure: the document is theirs to open, and the
-            // processor view is for whoever reviews the team rather than owns the file.
+            // The document is theirs to open; the processor view is for whoever reviews the team.
             when(authority.canEditPolicies()).thenReturn(false);
             FileRunEvent mine = givenHitBy(ACTOR, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
 
@@ -373,8 +365,7 @@ class FileRunEventServiceTest {
 
         @Test
         void aReviewerReadingAColleaguesIsNotOfferedTheDocumentTheyDoNotHave() {
-            // Dropped rather than disabled: a greyed-out "View file" would read as the
-            // reviewer's permission problem, when it is simply not their document.
+            // Dropped, not disabled: greyed out would read as their permission problem.
             FileRunEvent theirs =
                     givenHitBy(
                             "colleague@example.com",
@@ -401,8 +392,8 @@ class FileRunEventServiceTest {
 
         @Test
         void inheritedOwnerActionsComeBackDisabledWithTheReasonWhy() {
-            // The file was fed by a folder, bucket or webhook, and re-running it from there is work
-            // that has not landed. Said out loud rather than offered as a button that does nothing.
+            // No browser holds a source-fed file, so it is stated rather than offered as a dead
+            // button.
             FileRunEvent unattended =
                     givenHitBy(null, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
 
@@ -434,9 +425,8 @@ class FileRunEventServiceTest {
 
         @Test
         void theOwnersActionsAreDisabledWhenTheRowNamesNoDocument() {
-            // A row with no document id has nothing to name even for the owner holding the file.
-            // Answered here rather than left to the client, which would otherwise report it "not on
-            // this device" while it sits in their own workbench.
+            // Answered here, or the client calls it "not on this device" while it sits in their
+            // own workbench.
             FileRunEvent documentless =
                     givenHitBy(ACTOR, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, null);
 
@@ -465,8 +455,7 @@ class FileRunEventServiceTest {
 
         @Test
         void aMemberIsNotOfferedTheOwnerActionsOnAnUnattendedRow() {
-            // The inheritance is the reviewer's, not everybody's. A member has no claim on a run
-            // nobody attended, and cannot see one in the first place.
+            // The inheritance is the reviewer's: a member has no claim on a run nobody attended.
             when(authority.canEditPolicies()).thenReturn(false);
             FileRunEvent unattended =
                     givenHitBy(null, FailureKind.INPUT_PASSWORD_PROTECTED, TEAM, "f1");
@@ -476,8 +465,8 @@ class FileRunEventServiceTest {
 
         @Test
         void aLoginDisabledOperatorKeepsTheirOwnActions() {
-            // Its rows are unowned because it has no users, not because nothing attended them, and
-            // the one operator is holding the document. Disabling their retry would be wrong.
+            // Unowned for want of users, not because nothing attended: the one operator holds the
+            // file.
             ApplicationProperties props = new ApplicationProperties();
             props.getSecurity().setEnableLogin(false);
             FileRunEventService unsecured =
@@ -679,16 +668,14 @@ class FileRunEventServiceTest {
             complete.verifyEveryDeclaredActionHasAHandler();
 
             for (FailureActionId id : FailureActionId.values()) {
-                // Only the server's actions need a handler: the rest are run by the client, which
-                // is why the boot check no longer asks about them.
+                // Only server actions need a handler, which is why the boot check ignores the rest.
                 assertThat(complete.find(id).isPresent()).isEqualTo(id.runsOnServer());
             }
         }
 
         @Test
         void doesNotAskForAHandlerForAnActionTheClientRuns() {
-            // Every kind declares client actions, so a registry holding only the server's two must
-            // still boot; otherwise every client action would need an empty handler beside it.
+            // Otherwise every client action would need an empty handler beside it.
             FailureActionRegistry serverOnly =
                     new FailureActionRegistry(
                             List.of(new AcknowledgeAction(store), new DismissAction(store)));
@@ -699,16 +686,14 @@ class FileRunEventServiceTest {
 
         @Test
         void refusesAHandlerForAnActionTheClientRuns() {
-            // It could never be reached: dispatch refuses the id before resolving a handler. A bean
-            // that is never called is worse than no bean, because it reads as though it were.
+            // Dispatch refuses the id before resolving a handler, so the bean reads as live and is
+            // not.
             assertThatThrownBy(() -> new FailureActionRegistry(List.of(new ClientSideAction())))
                     .isInstanceOf(IllegalStateException.class)
                     .hasMessageContaining("VIEW_FILE");
         }
 
-        /**
-         * A handler for an action the client runs, which is exactly what must not be registered.
-         */
+        /** A handler for a client action, which is exactly what must not be registered. */
         private static final class ClientSideAction implements FailureAction {
 
             @Override

app/proprietary/src/test/java/stirling/software/proprietary/failure/FileRunEventStoreDbTest.java

@@ -241,9 +241,8 @@ class FileRunEventStoreDbTest {
                 store.record(
                         RecordFailure.forEditor(
                                 FailureKind.UNKNOWN, TEAM, "owner@example.com", "f-1", "boom"));
-        // A policy the user's own upload triggered. Recorded by the processor, but about the very
-        // document they just deleted, and carrying the id their client minted for it. Keying on
-        // origin left these behind: the user deleted the file and the failure stayed in the queue.
+        // Recorded by the processor, about the document they just deleted. Keying on origin left
+        // these in the queue.
         FileRunEvent myPolicyRun =
                 store.record(failure(FailureKind.UNKNOWN, TEAM, "owner@example.com", "f-1"));
         FileRunEvent theirs =
@@ -278,11 +277,8 @@ class FileRunEventStoreDbTest {
     @Test
     @DisplayName("a source-fed incident survives a client naming its file id")
     void markFilesRemovedLeavesSourceFedRowsAlone() {
-        // The two id spaces meet here. A source-fed run's fileId is a one-way hash of a path or key
-        // that was never on any device, so no client can legitimately claim to have deleted it.
-        // With login disabled the actor is null on both sides, so the actor clause matches and the
-        // absence of a source is the only thing standing between a local delete and a sweep's
-        // incidents.
+        // With login disabled the actor is null on both sides, so the absence of a source is all
+        // that stands between a local delete and a sweep's incidents.
         FileRunEvent sweep =
                 store.record(
                         new RecordFailure(

app/proprietary/src/test/java/stirling/software/proprietary/failure/NotificationProjectionTest.java

@@ -22,9 +22,8 @@ import stirling.software.proprietary.notification.NotificationView;
 import stirling.software.proprietary.policy.config.PolicyManagementAuthority;
 
 /**
- * What the bell is given to render. Lives beside the failure tests because the invariants are
- * failure invariants: the bell is never handed a raw event id, and it is offered only the actions
- * the client itself runs, resolved for this reader by the same service that scopes the queue.
+ * What the bell is given to render: never a raw event id, and only the actions the client itself
+ * runs, resolved for this reader by the same service that scopes the queue.
  */
 @ExtendWith(MockitoExtension.class)
 class NotificationProjectionTest {
@@ -95,12 +94,10 @@ class NotificationProjectionTest {
             assertThat(notification.status()).isEqualTo(FileRunEventStatus.NEW);
             assertThat(notification.fileId()).isEqualTo("f-1");
             assertThat(notification.policyId()).isNull();
-            // No source fed it, which is how the client knows the fileId above is one of its own
-            // references and worth looking up locally.
+            // How the client knows the fileId above is one of its own and worth looking up.
             assertThat(notification.sourceId()).isNull();
             assertThat(notification.defaultTitle()).isNotBlank();
-            // The failure queue's own resolved offers, minus the ones the server runs: a bell
-            // offering different client actions from the queue would be a bell that lies.
+            // The queue's own offers minus the server's: a bell offering different ones would lie.
             assertThat(notification.actions())
                     .containsExactlyElementsOf(
                             FileRunEventView.of(mine, failures.availableActions(mine))
@@ -115,8 +112,7 @@ class NotificationProjectionTest {
 
         @Test
         void offersNoActionTheServerRunsBecauseDispositionsBelongToTheQueue() {
-            // The bell routes the user to the right surface; deciding a failure's fate (dismissing
-            // it) happens on the review surface, so no disposition button reaches the panel.
+            // Deciding a failure's fate belongs to the review surface, not the panel.
             given(FailureKind.INPUT_PASSWORD_PROTECTED, ACTOR, "f-1");
 
             assertThat(controller.list(null).notifications().getFirst().actions())
@@ -126,8 +122,8 @@ class NotificationProjectionTest {
 
         @Test
         void namesTheSourceThatFedAnUnattendedRunSoItsFileIdIsNotMistakenForAClientsOwn() {
-            // Only a source-fed run's fileId is a hash, so the source is the discriminator: without
-            // it a client would look up a hash it can never resolve and call the document missing.
+            // Without the source a client looks up a hash it can never resolve and calls it
+            // missing.
             store.record(
                     RecordFailure.forRun(
                             FailureKind.INPUT_PASSWORD_PROTECTED,
@@ -147,8 +143,7 @@ class NotificationProjectionTest {
 
         @Test
         void aColleaguesNotificationOffersTheReviewersActionsOnly() {
-            // The bell shows a leader their team's failures, so the audience filtering has to reach
-            // it: no offering someone a document they do not have.
+            // A leader sees the team's failures, so audience filtering has to reach the bell too.
             given(FailureKind.INPUT_PASSWORD_PROTECTED, "colleague@example.com", "f-1");
 
             assertThat(controller.list(null).notifications().getFirst().actions())

app/proprietary/src/test/java/stirling/software/proprietary/failure/PolicyFailureOwnershipTest.java

@@ -55,15 +55,9 @@ import stirling.software.proprietary.policy.store.PolicyStore;
 import tools.jackson.databind.json.JsonMapper;
 
 /**
- * What a reader is offered on a real recorded row, with every collaborator between the failing tool
- * call and the offered actions being the real one. That {@code actor} names whoever triggered the
- * run is settled upstream by {@code PolicyFailureAttributionTest}; what is pinned here is what the
- * reader's relationship to that actor then entitles them to.
- *
- * <p>Ownership is derived per reader rather than stored, so the same row answers differently to the
- * person holding the document and to whoever reviews after them. Both directions are asserted,
- * because an action offered to the wrong one is either a button that cannot work or a document
- * handed to someone who should not have it.
+ * What a reader is offered on a real recorded row, every collaborator being the real one. Both
+ * directions are asserted: offered to the wrong reader is either a dead button or a leaked
+ * document.
  */
 @ExtendWith(MockitoExtension.class)
 class PolicyFailureOwnershipTest {
@@ -150,9 +144,7 @@ class PolicyFailureOwnershipTest {
                 TEAM);
     }
 
-    /**
-     * Run the shared policy so its single tool step fails, as {@code triggeredBy} (null = sweep).
-     */
+    /** Fails the policy's single tool step as {@code triggeredBy} (null = sweep). */
     private void runAndFail(String triggeredBy, String sourceId, String fileIdentity)
             throws Exception {
         when(internalApiClient.post(eq(ROTATE), any())).thenThrow(new RuntimeException("boom"));
@@ -183,10 +175,8 @@ class PolicyFailureOwnershipTest {
     }
 
     /**
-     * Read the single recorded row as {@code reader}, who is a plain member of the team. Lenient
-     * because a leader's read scope and an UNOWNED ownership check both answer without asking who
-     * is reading, so whether the name is consulted is the behaviour under test rather than a
-     * mistake.
+     * Lenient because a leader's scope and an UNOWNED check both answer without asking who reads,
+     * so whether the name is consulted is the behaviour under test.
      */
     private FileRunEvent asMember(String reader) {
         lenient().when(userService.getCurrentUsername()).thenReturn(reader);
@@ -231,9 +221,7 @@ class PolicyFailureOwnershipTest {
         void thePolicyOwnerIsNotHandedADocumentSheNeverTouched() throws Exception {
             runAndFail("bob", null, "bob-doc-1");
 
-            // Alice owns the policy and pays for the run, but her browser has no copy of Bob's
-            // file. Offering her the document produced a button that vanished client-side with no
-            // explanation.
+            // She owns the policy and pays for the run, and still has no copy of Bob's file.
             FileRunEvent theirs = asReviewer("alice");
             assertThat(service.ownershipOf(theirs)).isEqualTo(Ownership.THEIRS);
             assertThat(offeredTo(theirs)).doesNotContain(FailureActionId.VIEW_FILE);
@@ -243,8 +231,7 @@ class PolicyFailureOwnershipTest {
         void theReviewerIsStillOfferedWhatReviewingNeeds() throws Exception {
             runAndFail("bob", null, "bob-doc-1");
 
-            // Not her document, but still her team's incident: she gets the run and a way to close
-            // the row.
+            // Not her document, still her team's incident.
             assertThat(offeredTo(asReviewer("alice")))
                     .contains(FailureActionId.VIEW_IN_PROCESSOR, FailureActionId.DISMISS);
         }
@@ -260,9 +247,7 @@ class PolicyFailureOwnershipTest {
 
             FileRunEvent unattended = asReviewer("alice");
             assertThat(service.ownershipOf(unattended)).isEqualTo(Ownership.UNOWNED);
-            // The inheritance and its copy are reachable for the exact scenario they exist for: no
-            // browser holds this document, so the offer is stated and disabled rather than silently
-            // dropped.
+            // No browser holds this document, so the offer is stated and disabled, not dropped.
             assertThat(offeredTo(unattended)).contains(FailureActionId.VIEW_FILE);
             assertThat(service.availableActions(unattended))
                     .filteredOn(action -> action.id() == FailureActionId.VIEW_FILE)
@@ -277,8 +262,8 @@ class PolicyFailureOwnershipTest {
 
         @Test
         void thePolicyOwnerDoesNotInheritItAsHerOwn() throws Exception {
-            // Alice is billed for the sweep, and that must not become ownership: she is offered the
-            // owner actions here only as the team's reviewer, and disabled, not as MINE.
+            // Being billed for the sweep must not become ownership: she gets these as reviewer
+            // only.
             runAndFail(null, "src-watched-folder", "file-hash-1");
 
             assertThat(service.ownershipOf(asReviewer("alice"))).isNotEqualTo(Ownership.MINE);

app/proprietary/src/test/java/stirling/software/proprietary/failure/PolicyFailureRecorderTest.java

@@ -383,12 +383,8 @@ class PolicyFailureRecorderTest {
 
         @Test
         void theSameDocumentFailingInTwoAttendedRunsIsOneIncident() {
-            // WHAT THE DOCUMENT REFERENCE BUYS. A password-protected input is FILE-scoped, and
-            // every
-            // upload is a new run, so with no reference the run id stands in for the document and
-            // the
-            // same broken file re-uploaded reads as a second incident instead of a second
-            // occurrence.
+            // Every upload is a new run, so with no reference the run id stands in for the document
+            // and the same broken file reads as a second incident rather than a second occurrence.
             when(policyStore.get("policy-1")).thenReturn(Optional.of(policy("policy-1", TEAM)));
 
             recorder.recordRunFailure(
@@ -403,8 +399,7 @@ class PolicyFailureRecorderTest {
 
         @Test
         void twoDocumentsFailingTheSameWayStaySeparateIncidents() {
-            // The other half of the rule: folding is per document, so two broken uploads are still
-            // two rows and neither is credited with the other's occurrence.
+            // Folding is per document, so neither row is credited with the other's occurrence.
             when(policyStore.get("policy-1")).thenReturn(Optional.of(policy("policy-1", TEAM)));
 
             recorder.recordRunFailure(

app/proprietary/src/test/java/stirling/software/proprietary/failure/RecordFailurePrivacyTest.java

@@ -57,10 +57,8 @@ class RecordFailurePrivacyTest {
 
     @Test
     void theRunRequestThatSuppliesADocumentReferenceCarriesNoNameEither() {
-        // An attended run sends its own opaque reference to the document so a failure of it can be
-        // resolved back to that document. The same discipline applies at the door as in the row: an
-        // id and nothing else, or a document name reaches the wire on the way to a table that
-        // deliberately has nowhere to put it.
+        // The same discipline at the door as in the row: an id and nothing else, or a document name
+        // reaches a table that deliberately has nowhere to put it.
         assertThat(List.of(PolicyRunFiles.class.getDeclaredFields()))
                 .extracting(Field::getName)
                 .contains("fileId")

app/proprietary/src/test/java/stirling/software/proprietary/policy/controller/PolicyControllerTest.java

@@ -86,8 +86,7 @@ class PolicyControllerTest {
 
     @Mock private ProcessedLedger processedLedger;
 
-    // Real, not mocked: the run endpoints spool their uploads through it, and a test that supplies
-    // files needs them to actually land somewhere.
+    // Real, not mocked: the run endpoints spool uploads through it.
     private final TempFileManager tempFileManager =
             new TempFileManager(new TempFileRegistry(), new ApplicationProperties());
 
@@ -669,9 +668,7 @@ class PolicyControllerTest {
     @DisplayName("runStoredPolicy")
     class RunStoredPolicy {
 
-        /**
-         * The run files an editor sends: the documents, plus its own id for a single one of them.
-         */
+        /** What an editor sends: the documents, plus its own id for a single one of them. */
         private PolicyRunFiles filesWith(String fileId, int documents) {
             PolicyRunFiles files = new PolicyRunFiles();
             files.setFileId(fileId);
@@ -689,7 +686,6 @@ class PolicyControllerTest {
             return files;
         }
 
-        /** The document reference the run was actually started with. */
         private String documentReferenceOf(PolicyRunFiles files) throws Exception {
             Policy p = policy("a", 1L);
             when(policyStore.get("a")).thenReturn(Optional.of(p));
@@ -724,8 +720,8 @@ class PolicyControllerTest {
         @Test
         @DisplayName("records the caller's own id for a single-document run")
         void carriesTheCallersDocumentReference() throws Exception {
-            // The point of the whole field: a failure of this run names a document the client that
-            // started it can resolve, so the actions that need the bytes are reachable.
+            // The point of the field: a failure names a document the client that started it can
+            // resolve.
             assertThat(documentReferenceOf(filesWith("editor-file-1", 1)))
                     .isEqualTo("editor-file-1");
         }
@@ -733,15 +729,14 @@ class PolicyControllerTest {
         @Test
         @DisplayName("records nothing when the run carries several documents")
         void refusesToGuessWhichOfSeveralDocumentsItIs() throws Exception {
-            // One incident, one file reference: naming one of several would attribute the
-            // failure to whichever document happened to be bound first.
+            // One incident, one reference: naming one of several would attribute it to whichever
+            // bound first.
             assertThat(documentReferenceOf(filesWith("editor-file-1", 3))).isNull();
         }
 
         @Test
         @DisplayName("records nothing when the caller sent no id")
         void toleratesACallerThatSendsNoReference() throws Exception {
-            // Every other client (and every older one) keeps working exactly as before.
             assertThat(documentReferenceOf(filesWith(null, 1))).isNull();
         }
 

frontend/editor/src/core/components/notifications/BellIcon.tsx

@@ -1,7 +1,6 @@
 /**
- * Outlined bell. The bundled Material Symbols set only carries the filled variant, and a filled
- * bell reads as a permanently-ringing one, so this mirrors the portal's own {@code BellIcon}
- * rather than using it: core cannot import from portal.
+ * The bundled Material Symbols set only carries the filled variant, which reads as permanently
+ * ringing. Mirrors the portal's own icon rather than importing it: core cannot reach into portal.
  */
 export function BellIcon({ size = 18 }: { size?: number }) {
   return (

frontend/editor/src/core/components/notifications/NotificationBell.test.tsx

@@ -16,9 +16,8 @@ const render = (ui: Parameters<typeof baseRender>[0]) =>
   baseRender(ui, { wrapper: MantineProvider });
 
 /**
- * The bell renders whatever the server sends, and does with each row's actions only what the registry
- * for this build says it can. Two things are its own and worth pinning: which notifications the user has
- * already looked at, and how a row behaves around an action (message on failure, re-read on success).
+ * Two things are the bell's own and worth pinning: which notifications the user has already looked
+ * at, and how a row behaves around an action.
  */
 
 const fetchNotifications = vi.fn();
@@ -27,8 +26,7 @@ vi.mock("@app/services/notifications", () => ({
   fetchNotifications: (...args: unknown[]) => fetchNotifications(...args),
 }));
 
-// The document lookups read IndexedDB, which jsdom has none of. Answered here so a row's
-// availability is a fact of the test rather than of the environment.
+// IndexedDB, which jsdom has none of. Answered here so availability is a fact of the test.
 const h = vi.hoisted(() => ({
   hasLocalFile: true,
   specs: {} as Record<
@@ -45,16 +43,14 @@ vi.mock("@app/services/localFilePresence", () => ({
   hasLocalFile: () => Promise.resolve(h.hasLocalFile),
 }));
 
-// Stands in for the layer that owns the destinations. Core's own registry is empty, so without
-// this there are no client actions to test.
+// Core's own registry is empty, so without this there are no client actions to test.
 vi.mock("@app/components/notifications/notificationActions", () => ({
   useNotificationActions: () => h.specs,
 }));
 
 vi.mock("react-i18next", () => ({
   useTranslation: () => ({
-    // Mirrors i18next closely enough for this component: a string fallback, or an options object
-    // carrying defaultValue plus the values it interpolates.
+    // A string fallback, or an options object with defaultValue plus what it interpolates.
     t: (key: string, fallback?: unknown) => {
       if (typeof fallback === "string") return fallback;
       if (fallback && typeof fallback === "object") {
@@ -152,8 +148,7 @@ describe("NotificationBell", () => {
     render(<NotificationBell />);
     await openPanel();
 
-    // Opening is what marks them read: waiting for the close would leave the badge lit
-    // while the user is looking at the list.
+    // Opening marks them read: waiting for the close would leave the badge lit.
     await waitFor(() => expect(screen.queryByText("2")).toBeNull());
   });
 
@@ -172,8 +167,7 @@ describe("NotificationBell", () => {
   });
 
   it("keeps the division on screen after opening marks them read", async () => {
-    // The boundary is frozen on open. Read live it would collapse the moment the badge cleared,
-    // taking the divider with it while the user was still looking at the list.
+    // Frozen on open: read live it would collapse the moment the badge cleared.
     window.localStorage.setItem("stirling.notifications.lastSeenId", "b");
     fetchNotifications.mockResolvedValue([
       notification("a"),
@@ -229,8 +223,7 @@ describe("NotificationBell", () => {
   });
 
   it("treats everything as unread when the last seen one is gone", async () => {
-    // Dismissed or expired: we cannot tell how far the user got, so show them rather than
-    // silently marking the lot read.
+    // We cannot tell how far the user got, so show them rather than marking the lot read.
     window.localStorage.setItem(
       "stirling.notifications.lastSeenId",
       "vanished",
@@ -273,7 +266,7 @@ describe("NotificationBell", () => {
     render(<NotificationBell />);
     await openPanel();
 
-    // Named for the row they belong to: every button in the list says the same thing.
+    // Named for their row: every button in the list says the same thing.
     for (const id of ["VIEW_IN_PROCESSOR", "VIEW_FILE"])
       expect(
         screen.getByRole("button", { name: `${id}: Unrecognised failure` }),
@@ -382,8 +375,7 @@ describe("NotificationBell", () => {
   });
 
   it("claims nothing about a device for a row it never looks up", async () => {
-    // A source-fed run's fileId is a server-side identity that was never on any device, so it is not
-    // probed. Absent lookups must not read as an absent document, and the server said nothing either.
+    // Never on any device, so never probed, and an absent lookup is not an absent document.
     h.hasLocalFile = false;
     fetchNotifications.mockResolvedValue([
       notification("a", "Password-protected document", {
@@ -403,8 +395,7 @@ describe("NotificationBell", () => {
   });
 
   it("renders no button for an action the server would refuse, and says why in words", async () => {
-    // A greyed button on a failure it can never work for is false hope, so the next action takes the
-    // row and the reason becomes its note.
+    // A greyed button that can never work is false hope, so the reason becomes the row's note.
     h.specs = {
       VIEW_FILE: { available: () => true, run: vi.fn() },
       VIEW_IN_PROCESSOR: { available: () => true, run: vi.fn() },
@@ -457,7 +448,7 @@ describe("NotificationBell", () => {
     render(<NotificationBell />);
     await openPanel();
 
-    // The message and its reading aids remain, so the row still reads as a row.
+    // The message and its chips remain, so the row still reads as a row.
     expect(screen.getByText("Unrecognised failure")).toBeTruthy();
     expect(
       screen.getByText("Not available for this notification."),

frontend/editor/src/core/components/notifications/NotificationBell.tsx

@@ -28,11 +28,8 @@ import type { NotificationDocumentState } from "@app/hooks/useNotifications";
 import "@app/components/notifications/NotificationBell.css";
 
 /**
- * The bell and its panel. Lives in core because both shells mount it and the dependency only runs
- * one way: the portal may import from core, never the reverse.
- *
- * <p>Renders whatever the server sends without knowing which subsystem produced it, or what any of its
- * actions mean, so a new source or failure kind needs no change here.
+ * Renders whatever the server sends without knowing which subsystem produced it or what its actions
+ * mean, so a new source or failure kind needs no change here. In core because both shells mount it.
  */
 export function NotificationBell() {
   const { t } = useTranslation();
@@ -43,16 +40,12 @@ export function NotificationBell() {
   const container = useRef<HTMLDivElement>(null);
   const headingId = useId();
   /**
-   * The first notification the user had already seen when they opened the panel, which is where the
-   * new ones stop. Held as an id rather than a count because opening marks everything read, so a
-   * live count would collapse to zero and take the divider with it while they were reading.
-   *
-   * An id also survives the list changing underneath: one arriving on a poll lands above the
-   * divider, where it belongs, instead of shifting a frozen index onto the wrong row.
+   * Where the new ones stop, frozen on open. An id rather than a count because opening marks
+   * everything read, and because one arriving on a poll must land above the divider, not shift it.
    */
   const [firstSeenId, setFirstSeenId] = useState<string | null>(null);
-  // Fixed to the viewport, positioned from the trigger. The workbench bar clips its overflow, so
-  // an absolutely positioned panel is cut off by its own toolbar.
+  // Fixed to the viewport: the workbench bar clips its overflow, so an absolutely positioned panel
+  // would be cut off by its own toolbar.
   const [anchor, setAnchor] = useState<{ top: number; right: number } | null>(
     null,
   );
@@ -76,12 +69,11 @@ export function NotificationBell() {
     };
   }, [open]);
 
-  // Opening is what marks them read, not closing: the user has seen them by then, and waiting
-  // until close would leave the badge lit while they are looking at the list.
+  // Opening marks them read, not closing: waiting would leave the badge lit while they read.
   const toggle = () => {
     setOpen((wasOpen) => {
       if (!wasOpen) {
-        // Read the boundary before marking, or there is nothing left to read.
+        // Before marking, or there is nothing left to read.
         setFirstSeenId(notifications[unreadCount]?.id ?? null);
         markAllSeen();
       }
@@ -90,12 +82,8 @@ export function NotificationBell() {
   };
 
   /**
-   * How many of the listed notifications count as new, for this reading of the panel. Everything
-   * above it is new, everything from it down is earlier.
-   *
-   * No boundary id means everything was new when the panel opened, so the whole list is. A boundary
-   * that has since left the list (dismissed elsewhere, its document deleted) leaves nothing to
-   * divide on, and reads as "none new" rather than guessing at a row.
+   * How many count as new. No boundary id means all of them were; one that has since left the list
+   * leaves nothing to divide on, so it reads as none rather than guessing at a row.
    */
   const boundaryIndex = firstSeenId
     ? notifications.findIndex((notification) => notification.id === firstSeenId)
@@ -165,8 +153,8 @@ export function NotificationBell() {
                       />
                     </li>
                   )}
-                  {/* Only where there is something on both sides of it: a lone "Earlier" heading
-                      over the whole list says nothing the empty badge has not already said. */}
+                  {/* Only with something on both sides: a lone "Earlier" over everything says
+                      nothing the empty badge has not. */}
                   {index === dividedAt && dividedAt > 0 && (
                     <li aria-hidden>
                       <DividerWithText
@@ -192,9 +180,8 @@ export function NotificationBell() {
 }
 
 /**
- * The row's one line of explanation, if it has earned one. The server's reason for what it withheld
- * wins, being about this failure rather than this browser; otherwise only what we actually know, so a
- * row we never look up is never called absent.
+ * The server's reason wins, being about the failure rather than this browser. Otherwise only what we
+ * actually looked up, so a row we never probed is never called absent.
  */
 function noteFor(
   notification: AppNotification,
@@ -232,10 +219,7 @@ interface NotificationItemProps {
   onDismissPanel: () => void;
 }
 
-/**
- * One row. Its own component because the message its last attempt came back with and whether that
- * message is expanded are per-row state the panel cannot hold.
- */
+/** Its own component because the last attempt's message and its expanded state are per-row. */
 function NotificationItem({
   notification,
   unread,
@@ -255,18 +239,16 @@ function NotificationItem({
     hasLocalFile: documentState.hasLocalFile,
   };
 
-  // What this device can actually do, in the order the kind declared. An id this build has never
-  // heard of is skipped rather than rendered unwired: the server ships new kinds, and new actions,
-  // ahead of the clients that understand them.
+  // An id this build has never heard of is skipped rather than rendered unwired: the server ships
+  // new kinds, and new actions, ahead of the clients that understand them.
   const usable = notification.actions.filter((offer) => {
     if (!offer.enabled) return false;
     const spec = registry[offer.id];
     return spec ? spec.available(context) : false;
   });
 
-  // Why the row is thin, when the server withheld something and said so. Only from an action this
-  // build would otherwise have rendered, so a reason about an action it cannot perform anyway is not
-  // presented as the row's explanation.
+  // Only from an action this build would otherwise have rendered: a reason about one it cannot
+  // perform anyway is not this row's explanation.
   const withheldReasonKey =
     notification.actions.find(
       (offer) =>
@@ -308,7 +290,7 @@ function NotificationItem({
       await navigator.clipboard.writeText(notification.detail);
       setCopied(true);
     } catch {
-      // No clipboard permission. The message is on screen and selectable, so this needs no error.
+      // No clipboard permission, and the message is on screen and selectable anyway.
     }
   };
 
@@ -346,8 +328,6 @@ function NotificationItem({
           >
             {notification.detail}
           </span>
-          {/* Chrome for the message itself, never part of the action slots: reading the failure and
-              acting on it should not crowd each other out. */}
           <span className="notification-bell__chrome">
             <button
               type="button"
@@ -378,12 +358,9 @@ function NotificationItem({
         </>
       )}
 
-      {/* Actions were taken away from this row, so say why rather than leaving a bare row. */}
       {note && <span className="notification-bell__note">{note}</span>}
 
-      {/* Every action the row has, in the kind's declared order, the first leading. Three is the most
-          any kind offers once the unusable ones are dropped, so hiding the tail behind a menu would
-          cost more than it saves. */}
+      {/* In the kind's declared order, the first leading. */}
       {usable.length > 0 && (
         <span className="notification-bell__actions">
           {usable.map((offer, index) => (
@@ -409,7 +386,6 @@ function NotificationItem({
 }
 
 interface ActionButtonProps {
-  /** Solid for the row's answer, outlined for its runner-up, ghost for the rest. */
   variant: "primary" | "secondary";
   rowTitle: string;
   label: string;
@@ -431,8 +407,7 @@ function ActionButton({
       fontSize="xs"
       className="notification-bell__cta"
       disabled={busy}
-      // Every row's buttons say the same thing, so the label alone would not tell a screen reader which
-      // failure it acts on.
+      // Every row's buttons read alike, so the label alone would not say which failure this acts on.
       aria-label={`${label}: ${rowTitle}`}
       onClick={onRun}
     >

frontend/editor/src/core/components/notifications/notificationActions.ts

@@ -1,53 +1,40 @@
 import type { AppNotification } from "@app/services/notifications";
 
 /**
- * What this client can do about a notification, keyed by the action id the server offered. Keyed by
- * action rather than by row because the server decides which actions a failure kind offers: adding a
- * kind is no frontend change, and adding a button is one entry here.
+ * Keyed by action rather than by row, because the server decides what a kind offers: adding a kind is
+ * no frontend change, and adding a button is one entry here.
  */
 
-/** Everything an action needs to decide whether it can run, and to run. */
 export interface NotificationActionContext {
   notification: AppNotification;
   /** Whether the document is still in this browser, which is what most actions hinge on. */
   hasLocalFile: boolean;
 }
 
-/**
- * How a client action reports back. `void` means it did what it said, usually a navigation; a failed
- * outcome carries the message the row shows, because an action that quietly does nothing leaves the
- * user guessing.
- */
+/** `void` means it did what it said; a failed outcome carries the message the row shows. */
 export interface ClientActionOutcome {
   ok: boolean;
   message?: string;
 }
 
 export interface ClientActionSpec {
-  /** Whether this device can perform it right now. Asked per row, never during a request. */
+  /** Asked per row, never during a request. */
   available(context: NotificationActionContext): boolean;
-  /** May answer synchronously. */
   run(
     context: NotificationActionContext,
   ): ClientActionOutcome | void | Promise<ClientActionOutcome | void>;
-  /** Whether the panel should get out of the way, because the destination is behind it. */
+  /** Whether the panel should get out of the way, the destination being behind it. */
   closesPanel?: boolean;
 }
 
-/**
- * Indexed by action id. An id with no entry is skipped rather than rendered unwired: the server can
- * ship a new failure kind, with new actions, long before a client knows what they mean.
- */
+/** An id with no entry is skipped rather than rendered unwired. */
 export type ClientActionRegistry = Readonly<
   Record<string, ClientActionSpec | undefined>
 >;
 
 const NONE: ClientActionRegistry = {};
 
-/**
- * Nothing is offered here: every destination a notification could point at ships in a higher layer, so
- * a build without them has nowhere to send anyone, so its rows carry no buttons.
- */
+/** Every destination ships in a higher layer, so this build's rows carry no buttons. */
 export function useNotificationActions(): ClientActionRegistry {
   return NONE;
 }

frontend/editor/src/core/contexts/FileContext.tsx

@@ -611,11 +611,8 @@ function FileContextInner({
         // Remove from memory and cleanup resources
         lifecycleManager.removeFiles(fileIds, stateRef);
 
-        // Only a real delete closes a failure. Most callers pass false and mean "take it out of
-        // the workbench": closing a tab, unchecking it in the file manager, swapping which files
-        // are open. The document is still on the device and its failures still need attention, so
-        // telling the server it was deleted would quietly clear the user's own notifications while
-        // they worked. Fire-and-forget, so a server that cannot be told never blocks the delete.
+        // Only a real delete closes a failure: most callers pass false and mean "take it out of the
+        // workbench", leaving the document, and its failures, very much alive.
         if (deleteFromStorage !== false) {
           void reportFilesRemoved(fileIds);
         }

frontend/editor/src/core/contexts/NavigationContext.tsx

@@ -133,9 +133,8 @@ const NavigationStateContext = createContext<
   NavigationContextStateValue | undefined
 >(undefined);
 /**
- * Exported for the same reason as {@link FileActionsContext}: a component that
- * mounts in both shells has to ask whether these actions are available rather
- * than assume it, and {@link useNavigationActions} throws when they are not.
+ * Exported like {@link FileActionsContext}: a component mounting in both shells must ask whether
+ * these exist, and {@link useNavigationActions} throws when they do not.
  */
 export const NavigationActionsContext = createContext<
   NavigationContextActionsValue | undefined

frontend/editor/src/core/contexts/file/removeFiles.reporting.test.tsx

@@ -7,11 +7,8 @@ import type { FileContextActions } from "@app/types/fileContext";
 import type { FileId } from "@app/types/file";
 
 /**
- * Which removals tell the server a document is gone.
- *
- * `removeFiles` serves two jobs the caller distinguishes only by `deleteFromStorage`: deleting a
- * document, and taking one out of the workbench. Reporting both closes the user's own failure
- * notifications as they open and close files, which is not a decision they made.
+ * `removeFiles` deletes a document or merely takes it out of the workbench, told apart only by
+ * `deleteFromStorage`. Reporting both closed the user's own notifications as they opened files.
  */
 
 const reportFilesRemoved = vi.fn();
@@ -20,8 +17,7 @@ vi.mock("@app/services/failureReporting", () => ({
   reportToolFailure: vi.fn(),
 }));
 
-// A real delete reaches IndexedDB, which jsdom has none of. Only the delete itself is stubbed: the
-// point of these tests is which branch runs, so the branch has to be allowed to finish.
+// IndexedDB, which jsdom has none of. Stubbed so the delete branch can run to the end.
 vi.mock("@app/services/fileStorage", () => ({
   // FileContext subscribes to this to drop files whose bytes are unreadable.
   onRecordUnreadable: () => () => {},
@@ -68,8 +64,7 @@ describe("removeFiles and the failure queue", () => {
   });
 
   it("says nothing when the file is only closed in the workbench", async () => {
-    // Closing a tab, unchecking it in the file manager, swapping which files are open: all of them
-    // pass false and leave the document on the device, so its failures still need attention.
+    // Closing a tab or unchecking it leaves the document on the device, failures and all.
     setup();
 
     await act(async () => {
@@ -80,8 +75,7 @@ describe("removeFiles and the failure queue", () => {
   });
 
   it("treats an unspecified removal as a delete, the way the storage path does", async () => {
-    // Same default as the IndexedDB branch below it: absent means delete, only an explicit false
-    // means keep.
+    // Same default as the IndexedDB branch: only an explicit false means keep.
     setup();
 
     await act(async () => {

frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts

@@ -607,8 +607,7 @@ export const useToolOperation = <TParams>(
 
         // Report it so a leader sees the failure too, then carry on with the user's
         // own error handling. Fire-and-forget: the reporter swallows its own errors.
-        // Chained rather than fired alongside: the re-read has to happen after the row exists, or
-        // it finds the list exactly as it was.
+        // Chained, not fired alongside: the re-read must happen after the row exists.
         void reportToolFailure({
           operation: config.operationType,
           error,

frontend/editor/src/core/hooks/useNotifications.test.ts

@@ -3,10 +3,8 @@ import { act, renderHook, waitFor } from "@testing-library/react";
 import type { AppNotification } from "@app/services/notifications";
 
 /**
- * The bell is mounted several times over (floated over an empty workbench, inside the workbench bar,
- * again in the portal shell) and all of them show the same thing, so what is pinned here is that
- * they share one read of it: one poll, one set of document lookups, one read marker, and no timer
- * left running once the last of them has gone.
+ * The bell is mounted several times over, so what is pinned here is that they share one read: one
+ * poll, one set of lookups, one marker, and no timer left running once the last has gone.
  */
 
 const fetchNotifications = vi.fn();
@@ -15,8 +13,7 @@ vi.mock("@app/services/notifications", () => ({
   fetchNotifications: (...args: unknown[]) => fetchNotifications(...args),
 }));
 
-// The document lookups read IndexedDB, which jsdom has none of. Counted here so that "resolved once
-// per list, not once per row" is observable.
+// Counted here so "resolved once per list, not once per row" is observable.
 const hasLocalFile = vi.fn((_fileId: string) => Promise.resolve(true));
 
 vi.mock("@app/services/localFilePresence", () => ({
@@ -85,9 +82,8 @@ describe("useNotifications", () => {
   });
 
   it("looks up an attended run's document but never an unattended run's", async () => {
-    // Both rows name a document, but only the attended one names a reference this browser could
-    // resolve. Asking storage about a source's hash can only miss, which would then be shown as "not
-    // on this device" about a document that was never on one.
+    // Asking storage about a source's hash can only miss, and would then be shown as "not on this
+    // device" about a document that never was.
     fetchNotifications.mockResolvedValue([
       notification("attended", {
         origin: "POLICY",
@@ -156,8 +152,7 @@ describe("useNotifications", () => {
     await waitFor(() => expect(first.result.current.unreadCount).toBe(2));
     expect(second.result.current.unreadCount).toBe(2);
 
-    // Async, because the store tells its subscribers on a microtask: a bell marks the list read
-    // from inside its own state updater, so it cannot re-render its neighbours from there.
+    // Async because subscribers are told on a microtask: a bell marks the list read while rendering.
     await act(async () => first.result.current.markAllSeen());
 
     expect(first.result.current.unreadCount).toBe(0);
@@ -205,8 +200,7 @@ describe("useNotifications", () => {
     );
     first.unmount();
 
-    // Nothing to report by the time the next bell appears: it must not show the old row while its
-    // own read is in flight.
+    // It must not show the old row while its own read is in flight.
     fetchNotifications.mockResolvedValue([]);
     const second = renderHook(() => useNotifications());
 

frontend/editor/src/core/hooks/useNotifications.ts

@@ -6,23 +6,11 @@ import {
 import { hasLocalFile } from "@app/services/localFilePresence";
 
 /**
- * The caller's notifications, refreshed on a timer because they arrive from background work rather
- * than from anything the user just did.
- *
- * One set of data for however many bells are on screen. The bell is mounted more than once, so the
- * list, the document lookups and the read marker live in the module-level store below and each mount
- * merely subscribes; otherwise two badges can disagree while reading the same localStorage key.
- *
- * A module-level store rather than a React context because there is no single tree to hang a provider
- * in: the portal mounts its bell as a sibling of AppProviders. Each bell still owns its own open state.
- *
- * TODO: read state is tracked here, in the browser, as the id of the newest notification the user
- * has seen. That is enough for one device and one person, but it does not survive a cache clear
- * and does not follow them to another browser. When notifications become a server-side concept
- * with their own table, move this there: the server can then record which user has read which
- * notification, durably and per user, and this hook just renders what it is told.
+ * One polled store for however many bells are mounted. A module store rather than a context because
+ * the portal mounts its bell as a sibling of AppProviders, so there is no single tree to provide in.
  */
 
+// TODO: read state is per-browser. Move it server-side when notifications get their own table.
 const POLL_INTERVAL_MS = 30_000;
 const SEEN_STORAGE_KEY = "stirling.notifications.lastSeenId";
 
@@ -30,8 +18,7 @@ function readLastSeenId(): string | null {
   try {
     return window.localStorage.getItem(SEEN_STORAGE_KEY);
   } catch {
-    // Private mode, or storage disabled. Everything then reads as unseen, which errs towards
-    // showing the user their failures rather than hiding them.
+    // Private mode: everything reads as unseen, which errs towards showing failures.
     return null;
   }
 }
@@ -40,14 +27,10 @@ function writeLastSeenId(id: string): void {
   try {
     window.localStorage.setItem(SEEN_STORAGE_KEY, id);
   } catch {
-    // Nothing to do: the marker simply will not persist across a reload.
+    // The marker just will not survive a reload.
   }
 }
 
-/**
- * What this browser holds for the document a notification is about, which decides what can actually run
- * here. Resolved for the list rather than per row, since the answers come from IndexedDB.
- */
 export interface NotificationDocumentState {
   hasLocalFile: boolean;
 }
@@ -57,23 +40,16 @@ const NO_DOCUMENT: NotificationDocumentState = {
 };
 
 /**
- * Whether this browser could resolve the document a notification names, which is the one place that
- * rule is stated.
- *
- * Two id spaces share the one fileId field. An attended run reports the id its editor minted, so a
- * lookup here can succeed; an unattended run reports its source's one-way hash of a path or key, which
- * was never on any device. The discriminator is the absence of a source rather than the origin, since
- * an attended policy run now sends a client reference too.
+ * Whether this browser could resolve the document a row names. Two id spaces share `fileId`: an
+ * attended run reports the id its editor minted, a source-fed one a hash that was never on a device.
  */
 export function isResolvableHere(notification: AppNotification): boolean {
-  // Coalesced, so a row from a server that does not send the field reads as "no source".
   return (notification.sourceId ?? null) === null;
 }
 
-/** Everything the bells read. Replaced wholesale, never mutated, so it can be a snapshot. */
 interface NotificationsSnapshot {
   notifications: AppNotification[];
-  /** Keyed by fileId, so several rows about the same document cost one pair of lookups. */
+  /** Keyed by fileId, so several rows about one document cost one lookup. */
   documents: Record<string, NotificationDocumentState>;
   lastSeenId: string | null;
 }
@@ -87,9 +63,8 @@ const NOTHING_LOADED: NotificationsSnapshot = {
 let snapshot: NotificationsSnapshot = NOTHING_LOADED;
 const subscribers = new Set<() => void>();
 let pollTimer: number | null = null;
-/** The read in progress, if any, so that everyone who asks while it runs joins it. */
 let inFlight: Promise<void> | null = null;
-/** Bumped whenever polling starts or stops, so a read from a finished cycle cannot write. */
+/** Bumped when polling starts or stops, so a read from a finished cycle cannot write. */
 let cycle = 0;
 let notifyQueued = false;
 
@@ -98,10 +73,8 @@ function getSnapshot(): NotificationsSnapshot {
 }
 
 /**
- * New snapshot now, subscribers told on a microtask. Deferred because a bell marks the list read from
- * inside its own state updater, i.e. while rendering, and re-rendering every other bell from there is
- * the render-phase update React refuses. A microtask is still the same tick, so no bell paints a badge
- * it should have dropped. Coalesced, so a burst of writes costs one round of re-renders.
+ * Subscribers told on a microtask: a bell marks the list read from inside its own state updater, and
+ * re-rendering the others from there is the render-phase update React refuses.
  */
 function publish(next: NotificationsSnapshot): void {
   snapshot = next;
@@ -145,11 +118,7 @@ async function read(forCycle: number): Promise<void> {
   });
 }
 
-/**
- * Read the list once, however many callers want it: a caller arriving mid-read joins the read already
- * running. An action's refresh can then be answered by a read that started just before it landed, which
- * the next poll corrects.
- */
+/** A caller arriving mid-read joins the one already running. */
 function load(): Promise<void> {
   if (inFlight) return inFlight;
   const pending = read(cycle).finally(() => {
@@ -161,8 +130,7 @@ function load(): Promise<void> {
 
 function startPolling(): void {
   cycle += 1;
-  // Re-read from disk rather than trusting what the last bell left in memory: another tab may have
-  // moved the marker on since.
+  // From disk, not memory: another tab may have moved the marker on.
   snapshot = { ...NOTHING_LOADED, lastSeenId: readLastSeenId() };
   pollTimer = window.setInterval(() => void load(), POLL_INTERVAL_MS);
   void load();
@@ -173,14 +141,13 @@ function stopPolling(): void {
     window.clearInterval(pollTimer);
     pollTimer = null;
   }
-  // Anything still in flight belongs to a cycle nobody is watching: drop its result, and do not let
-  // the next bell join it.
+  // Drop anything in flight: its cycle has nobody watching it.
   cycle += 1;
   inFlight = null;
   snapshot = NOTHING_LOADED;
 }
 
-/** Polling lives exactly as long as there is a bell to show it: one timer, no leak. */
+/** Polling lives exactly as long as there is a bell to show it. */
 function subscribe(onStoreChange: () => void): () => void {
   subscribers.add(onStoreChange);
   if (subscribers.size === 1) startPolling();
@@ -190,7 +157,6 @@ function subscribe(onStoreChange: () => void): () => void {
   };
 }
 
-/** Everything currently listed becomes read, for every bell at once. */
 function markAllSeen(): void {
   const newest = snapshot.notifications[0];
   if (!newest || snapshot.lastSeenId === newest.id) return;
@@ -203,13 +169,8 @@ function refresh(): void {
 }
 
 /**
- * Re-read now, for a caller that has just caused a notification to exist. Without it the person who
- * triggered a failure waits up to a whole poll interval to be told about their own action, which
- * reads as the app not having noticed. Everyone else's failures still arrive on the poll, which is
- * what it is for.
- *
- * A no-op when no bell is mounted: there is nobody to tell, and the next mount reads for itself.
- * Safe to call after a report that was refused, since the re-read simply finds nothing new.
+ * Re-read now, for a caller that just caused a notification: without it the person who triggered a
+ * failure waits a whole poll interval to hear about their own action. A no-op with no bell mounted.
  */
 export function refreshNotificationsNow(): void {
   if (subscribers.size === 0) return;
@@ -218,16 +179,11 @@ export function refreshNotificationsNow(): void {
 
 export interface NotificationsState {
   notifications: AppNotification[];
-  /**
-   * How many are newer than the last one the user looked at: the badge, and the boundary the panel
-   * divides new from earlier on. Read before {@link markAllSeen}, which zeroes it.
-   */
+  /** Read before {@link markAllSeen}, which zeroes it. */
   unreadCount: number;
-  /** What this device holds for a row's document. Answers for a document it knows nothing about. */
   documentStateFor: (
     notification: AppNotification,
   ) => NotificationDocumentState;
-  /** Call when the user opens the panel: everything currently listed becomes read. */
   markAllSeen: () => void;
   refresh: () => void;
 }
@@ -239,9 +195,8 @@ export function useNotifications(): NotificationsState {
     getSnapshot,
   );
 
-  // The list is newest first, so everything above the last-seen id is new. An id that is no longer
-  // in the list (dismissed, expired) means we cannot tell how far the user got, so treat the whole
-  // list as unread rather than silently marking it all read.
+  // A marker no longer in the list means we cannot tell how far the user got, so everything reads
+  // as unread rather than being silently marked seen.
   const seenIndex = lastSeenId
     ? notifications.findIndex((n) => n.id === lastSeenId)
     : -1;

frontend/editor/src/core/routes/portalBasename.ts

@@ -7,9 +7,7 @@
 export const PORTAL_BASENAME = "/processor";
 
 /**
- * Fragment identifying the recorded-failures section of the portal's Documents
- * view. Here for the same reason as the basename: whoever links to that section
- * and whoever renders it are in different layers, and neither should have to
- * import the other to agree on the anchor.
+ * The recorded-failures section of the portal's Documents view. Here because whoever links to it and
+ * whoever renders it are in different layers.
  */
 export const PORTAL_FAILURES_ANCHOR = "failures";

frontend/editor/src/core/services/localFilePresence.ts

@@ -1,10 +1,7 @@
 import { fileStorage } from "@app/services/fileStorage";
 import type { FileId } from "@app/types/file";
 
-/**
- * Whether the document a failure was filed against is still in this browser. It decides whether the
- * bell can offer to open it: the id is this workspace's own, so no other device can answer yes.
- */
+/** Whether the document is still in this browser. The id is this workspace's own, so only it can say. */
 export async function hasLocalFile(fileId: string | null): Promise<boolean> {
   if (!isUsableId(fileId)) return false;
 

frontend/editor/src/core/services/notifications.ts

@@ -1,53 +1,33 @@
 import apiClient from "@app/services/apiClient";
 
-/**
- * The caller's notifications. Derived server-side from whatever produces them, so this client
- * needs to know nothing about failures, policies or any later source: it renders what it is given
- * and branches on `source` only when it wants to.
- */
-
+// Derived server-side from whatever produces them, so this client knows nothing about failures.
 const NOTIFICATIONS_PATH = "/api/v1/notifications";
 
-/** Which subsystem produced a notification. Widen as the server gains sources. */
 export type NotificationSource = "FAILURE";
 
 export type NotificationSeverity = "ERROR" | "WARNING" | "INFO";
 
-/** What was running when it failed. */
 export type NotificationOrigin = "TOOL" | "POLICY" | "PIPELINE";
 
-/**
- * Whose document it is, from this reader's point of view. `UNOWNED` is an unattended run: nobody has
- * the file, so nothing that needs the bytes can be offered.
- */
+/** From this reader's point of view. `UNOWNED` is an unattended run: nobody holds the file. */
 export type NotificationOwnership = "MINE" | "THEIRS" | "UNOWNED";
 
-/**
- * One action as offered for one notification, all of them run by this client on its own device.
- * `id` is a plain string rather than a union because the server may know actions this build does
- * not, and the client skips the ones it cannot perform.
- */
+/** `id` is an open string, not a union: the server may know actions this build does not. */
 export interface NotificationActionOffer {
   id: string;
   labelKey: string;
   /** English fallback, for a build with no copy for `labelKey`. */
   defaultLabel: string;
-  /** False means it cannot work for this row. The bell renders no button and states the reason
-   *  instead; the portal's queue still shows it disabled. */
+  /** False renders no button in the bell, and a disabled one in the portal's queue. */
   enabled: boolean;
   disabledReasonKey: string | null;
 }
 
 export interface AppNotification {
-  /**
-   * Unique across sources, so it can be keyed and compared on its own. Prefixed with its source
-   * (`failure:<uuid>`), so it is the id the notification endpoints take and never one a per-source
-   * endpoint would accept.
-   */
+  /** Prefixed with its source (`failure:<uuid>`), so it is never an id a per-source endpoint takes. */
   id: string;
   source: NotificationSource;
-  /** Which failure kind, e.g. `INPUT_PASSWORD_PROTECTED`. An open string: the server adds kinds
-   *  without waiting for a client that knows them. */
+  /** Open string, e.g. `INPUT_PASSWORD_PROTECTED`: the server adds kinds without a client change. */
   kindId: string;
   origin: NotificationOrigin;
   ownership: NotificationOwnership;
@@ -56,12 +36,9 @@ export interface AppNotification {
   titleKey: string;
   defaultTitle: string;
   detail: string | null;
-  /**
-   * Opaque reference to the document, resolvable only by the client that stored it. Two id spaces
-   * share this field, and `sourceId` says which: see `isResolvableHere` in `useNotifications`.
-   */
+  /** Two id spaces share this field, and `sourceId` says which: see `isResolvableHere`. */
   fileId: string | null;
-  /** Which folder, bucket or webhook fed the failing run, and null for an attended run. */
+  /** Which folder, bucket or webhook fed the run, and null for an attended one. */
   sourceId: string | null;
   policyId: string | null;
   occurrences: number;
@@ -74,11 +51,7 @@ interface NotificationsResponse {
   notifications: AppNotification[];
 }
 
-/**
- * Newest first. Empty rather than throwing on a build without the endpoint, or for a caller the
- * server will not answer: a bell that cannot load is a bell with nothing in it, not an error the
- * user needs to see.
- */
+/** Newest first. Empty rather than throwing: a bell that cannot load is an empty bell, not an error. */
 export async function fetchNotifications(
   limit = 20,
 ): Promise<AppNotification[]> {

frontend/editor/src/core/tests/helpers/api-stubs.ts

@@ -274,10 +274,8 @@ export async function mockAppApis(
     route.fulfill({ json: [] }),
   );
 
-  // The notification bell polls GET /api/v1/notifications from every shell it
-  // is mounted in, starting on load. The hook swallows the failure and shows an
-  // empty bell, but the browser still logs the failed request itself, which the
-  // console-hygiene guard counts. Empty list, for the same reason as policies.
+  // The bell polls this on load. The hook swallows the failure, but the browser still logs the
+  // request, which the console-hygiene guard counts.
   await page.route("**/api/v1/notifications*", (route: Route) =>
     route.fulfill({ json: { notifications: [] } }),
   );

frontend/editor/src/core/types/fileContext.ts

@@ -330,9 +330,8 @@ export interface FileContextActions {
       selectFiles?: boolean;
       skipUploadTracking?: boolean;
       /**
-       * Mark every added file as produced in-app rather than uploaded, which is what stops the
-       * policy auto-run enforcing an upload policy on it. Set by anything adding a file that has
-       * already been through a policy or a tool: policy output delivery, and the bell's unlock.
+       * Produced in-app rather than uploaded, which stops the policy auto-run enforcing an upload
+       * policy on it. Set by anything adding a file already through a policy or a tool.
        */
       derivedFromTool?: boolean;
     },

frontend/editor/src/portal/components/failures/FileRunEventList.tsx

@@ -35,10 +35,8 @@ export function FileRunEventList() {
   const section = useRef<HTMLElement>(null);
   const { hash, key } = useLocation();
 
-  // Arriving from a notification, which links to this section by fragment. The browser only
-  // honours a fragment on a real page load, and this is a client-side route change, so scroll
-  // it into view here. Keyed on the navigation as well as the fragment: clicking a second
-  // notification while already parked here changes neither the path nor the hash.
+  // A fragment is only honoured on a real page load, not a client-side route change. Keyed on the
+  // navigation too: a second notification changes neither the path nor the hash.
   useEffect(() => {
     if (hash !== `#${PORTAL_FAILURES_ANCHOR}`) return;
     section.current?.scrollIntoView({ behavior: "smooth", block: "start" });

frontend/editor/src/proprietary/components/notifications/notificationActions.test.tsx

@@ -9,9 +9,8 @@ import type {
 import type { NotificationActionContext } from "@core/components/notifications/notificationActions";
 
 /**
- * What this build can actually do about a failure, and where each action sends the reader. The bell hangs
- * in the editor and in the processor, and only the editor has a file context above it, so the two shells
- * are the interesting cases: selecting the document directly, or handing it over.
+ * Where each action sends the reader. Only the editor has the workbench contexts above it, so the two
+ * shells are the interesting cases: opening the document, or handing it over.
  */
 
 const navigate = vi.fn();
@@ -22,8 +21,7 @@ vi.mock("react-router-dom", async () => ({
   useNavigate: () => navigate,
 }));
 
-// No i18n instance is initialised here, so the real hook would return bare keys. The plugin is
-// stubbed too because the workbench contexts below reach `core/i18n`, which registers it on import.
+// No i18n instance here, and the plugin is stubbed because the contexts below reach `core/i18n`.
 vi.mock("react-i18next", () => ({
   useTranslation: () => ({
     t: (_key: string, fallback: string) => fallback,
@@ -31,8 +29,7 @@ vi.mock("react-i18next", () => ({
   initReactI18next: { type: "3rdParty", init: () => {} },
 }));
 
-// The document lookup reads IndexedDB, which jsdom has none of. Answered here so that whether the
-// bytes are present is a fact of the test rather than of the environment.
+// IndexedDB, which jsdom has none of. Answered here so presence is a fact of the test.
 const h = vi.hoisted(() => ({
   stub: { id: "f-1" } as unknown,
   getStirlingFileStub: vi.fn(),
@@ -83,7 +80,6 @@ function notification(
   };
 }
 
-/** An action the server offered, enabled: what it does with it is the client's decision. */
 function offer(id: string): NotificationActionOffer {
   return {
     id,
@@ -165,8 +161,7 @@ describe("useNotificationActions", () => {
   });
 
   it("leaves View file as the only usable offer when the server offers actions this build cannot run", () => {
-    // The server can ship new kinds with new actions ahead of the clients that understand them, so
-    // an id this build wires nothing for drops out rather than rendering dead.
+    // An id this build wires nothing for drops out rather than rendering dead.
     const actions = registry();
     const usable = [offer("QUARANTINE"), offer("VIEW_FILE")].filter(
       (candidate) => actions[candidate.id]?.available(context()) ?? false,
@@ -178,16 +173,14 @@ describe("useNotificationActions", () => {
   it("opens the document into the viewer when an editor is above", async () => {
     await registry().VIEW_FILE?.run(context());
 
-    // Selecting alone would show nothing: the workbench does not hold the file yet, and it keeps
-    // whatever view it was already on.
+    // Selecting alone shows nothing: the workbench holds neither the file nor the viewer yet.
     expect(addStirlingFileStubs).toHaveBeenCalledWith([h.stub]);
     expect(setActiveFileId).toHaveBeenCalledWith("f-1");
     expect(setWorkbench).toHaveBeenCalledWith("viewer");
   });
 
   it("stays where it is rather than routing through the role-based root", async () => {
-    // "/" decides a landing page from the reader's role, so navigating there reads as the app
-    // reloading and can land them somewhere other than their document.
+    // "/" lands on a page chosen by the reader's role, which reads as the app reloading.
     await registry().VIEW_FILE?.run(context());
 
     expect(window.location.pathname).toBe("/");
@@ -217,11 +210,11 @@ describe("useNotificationActions", () => {
   it("hands the document over to the editor when there is no workbench above it", async () => {
     await registry(inProcessor).VIEW_FILE?.run(context());
 
-    // Nothing to open into, so the intent outlives the navigation that mounts the editor.
+    // The intent outlives the navigation that mounts the editor.
     expect(
       window.sessionStorage.getItem("stirling.notifications.pendingSelection"),
     ).toBe("f-1");
-    // The editor's own URL, not "/", which would hand the reader to the role router instead.
+    // The editor's own URL, not the role router at "/".
     expect(window.location.pathname).toBe("/editor");
   });
 
@@ -242,8 +235,7 @@ describe("useNotificationActions", () => {
   });
 
   it("says it cannot hand the document over rather than navigating to nothing", async () => {
-    // Storage refused, so nothing would be selected on arrival: the row reports it and stays put.
-    // On the prototype: jsdom's storage object is a proxy, so an own-property spy does not take.
+    // Spied on the prototype: jsdom's storage is a proxy, so an own-property spy does not take.
     const setItem = vi
       .spyOn(Storage.prototype, "setItem")
       .mockImplementation(() => {
@@ -257,7 +249,7 @@ describe("useNotificationActions", () => {
       message:
         "This browser will not let the processor pass the document to the editor. Open it from the editor instead.",
     });
-    // Still on the page it started on, so the failure is visible rather than mysterious.
+    // Still on the page it started on, so the failure is visible.
     expect(window.location.pathname).toBe("/");
     setItem.mockRestore();
   });
@@ -269,8 +261,7 @@ describe("useNotificationActions", () => {
   });
 
   it("offers the processor link whenever the server did", () => {
-    // The server only sends it to someone it will let read the queue, so there is nothing left
-    // to gate here.
+    // The server only sends it to someone it will let read the queue.
     expect(
       registry(inProcessor).VIEW_IN_PROCESSOR?.available(
         context({ hasLocalFile: false }),

frontend/editor/src/proprietary/components/notifications/notificationActions.ts

@@ -30,36 +30,20 @@ export {
 };
 
 /**
- * What this build can do about a failure notification: open the document it is about, or go to the
- * recorded failures in the processor.
- *
- * THE SHELL PROBLEM. The portal mounts as a sibling of the route that renders `AppProviders` (see
- * `proprietary/App.tsx`), so in the processor shell there is no FileContext, ToolWorkflowContext or
- * NavigationContext above this hook. Three things below are shaped by that and marked where they
- * appear: file state is reached through the raw context so its absence is a value rather than a thrown
- * error; a document the processor cannot select is handed over through session storage; and editor
- * destinations are reached by pushing the URL and announcing it. All three go away when the portal
- * route moves inside `AppProviders`.
+ * The portal mounts as a sibling of `AppProviders`, so in the processor shell none of the workbench
+ * contexts exist above this hook. That is why contexts are read raw and a document is handed over.
  */
 
-/**
- * The document a notification's actions are about, waiting for an editor to pick it up. Written only
- * when there is no file context to select it directly (shell problem, point 2). Session-scoped and
- * one-shot: it is a click the user just made, not state worth keeping.
- */
 const HANDOFF_KEY = "stirling.notifications.pendingSelection";
 
-/** The recorded-failures section of the processor, which is as precise as this link gets. */
 const FAILURES_DESTINATION = `${PORTAL_BASENAME}/documents#${PORTAL_FAILURES_ANCHOR}`;
 
-/** False when this browser will not store it, which the caller must not paper over. */
+/** False when storage refused it: navigating anyway lands the user in an editor with nothing open. */
 function stashSelection(fileId: string): boolean {
   try {
     window.sessionStorage.setItem(HANDOFF_KEY, fileId);
     return true;
   } catch {
-    // Private mode, or storage disabled. Navigating anyway would land the user in the editor with
-    // nothing selected and no idea why, so the caller reports it instead.
     return false;
   }
 }
@@ -75,10 +59,8 @@ function takeSelection(): string | null {
 }
 
 /**
- * Go to an editor path from either shell. Not the router's `navigate` (shell problem, point 3): the
- * editor reads its tool out of the URL on mount and on a history pop, and a router push is neither, so
- * the address would change and the workbench would not. Pushing plus announcing covers both readers,
- * the same way `settingsNavigation` opens a settings section.
+ * Not the router's `navigate`: the editor reads its tool from the URL on mount and on a history pop,
+ * and a router push is neither, so the address would change and the workbench would not.
  */
 function goToEditor(path: string): void {
   window.history.pushState({}, "", withBasePath(path));
@@ -88,9 +70,8 @@ function goToEditor(path: string): void {
 export function useNotificationActions(): ClientActionRegistry {
   const { t } = useTranslation();
   const navigate = useNavigate();
-  // Raw contexts, not the hooks that wrap them (shell problem, point 1): in the processor shell
-  // there is no provider above the bell, and a hook that insists on one would take the whole panel
-  // down with it. All four are present together or not at all, which is what `canOpenHere` means.
+  // Raw, because the hooks that wrap these throw when there is no provider, and in the processor
+  // shell there is none. All four are present together or not at all.
   const fileContext = useContext(FileActionsContext);
   const fileStore = useContext(FileStoreContext);
   const navigation = useContext(NavigationActionsContext);
@@ -98,13 +79,8 @@ export function useNotificationActions(): ClientActionRegistry {
   const canOpenHere = Boolean(fileContext && fileStore && navigation && viewer);
 
   /**
-   * Open a document the way the file sidebar does, rather than merely selecting it: a selected id
-   * that is not in the workbench shows nothing, and the workbench keeps whatever view it was on.
-   * So the stub is added if the workbench does not already hold it, made the active file, and the
-   * viewer brought to the front.
-   *
-   * False when this shell cannot do it, or when the document is no longer in storage: the caller
-   * turns that into the row's message instead of a navigation that appears to do nothing.
+   * Opens the way the file sidebar does. Selecting alone shows nothing: an id the workbench does not
+   * hold has nothing to render, and the workbench keeps whatever view it was on.
    */
   const openInWorkbench = useCallback(
     async (fileId: string): Promise<boolean> => {
@@ -126,8 +102,7 @@ export function useNotificationActions(): ClientActionRegistry {
     [fileContext, fileStore, navigation, viewer],
   );
 
-  // Pick up a document handed over by the other shell (shell problem, point 2). One-shot: the
-  // handoff is read and cleared, so a later render cannot reopen a file the user has moved on from.
+  // One-shot: read and cleared, so a later render cannot reopen a file the user has moved on from.
   useEffect(() => {
     if (!canOpenHere) return;
     const fileId = takeSelection();
@@ -140,21 +115,13 @@ export function useNotificationActions(): ClientActionRegistry {
     ): Promise<ClientActionOutcome | void> => {
       if (!fileId) return;
 
-      // Already in the shell that owns the workbench, so open it in place. Navigating as well
-      // would send the user through the role-based router at "/", which reads as the app
-      // reloading itself and lands them wherever their role says, not on their document.
+      // In place, with no navigation: "/" is the role-based router, so going there reads as the app
+      // reloading and lands the user wherever their role says rather than on their document.
       if (canOpenHere) {
-        // No message of its own: the row falls back to "that did not work", which is the whole
-        // truth here. The document was in storage a moment ago or the button would not be on
-        // screen, so a failure now is a race rather than something the user can act on.
         return (await openInWorkbench(fileId)) ? undefined : { ok: false };
       }
 
-      // The processor shell has no workbench to open into, so hand the document over and go to
-      // the editor's own URL. EDITOR_BASENAME rather than "/" for the same reason as above.
       if (!stashSelection(fileId)) {
-        // Nothing would be opened on arrival, so say so here rather than navigate to a page
-        // that looks like it worked.
         return {
           ok: false,
           message: t(
@@ -173,10 +140,8 @@ export function useNotificationActions(): ClientActionRegistry {
     };
 
     const viewInProcessor: ClientActionSpec = {
-      // The server only offers this to someone it will let read the queue, so audience is already
-      // settled. What is left is whether the destination exists: the failures section it lands on is
-      // mounted in dev only until failures get their own review screen, so in a build this would
-      // navigate nowhere. Both gates lift together, and the other one is in portal/views/Documents.
+      // Its destination is dev-only until failures get a review screen; the other half of this gate
+      // is in portal/views/Documents, and both lift together.
       available: () => import.meta.env.DEV,
       closesPanel: true,
       run: () => navigate(FAILURES_DESTINATION),

frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.chain.test.tsx

@@ -91,8 +91,7 @@ describe("auto-run ordered chaining", () => {
 
     // The first policy (order 0) runs on the upload; the second waits for the chain.
     expect(runStored).toHaveBeenCalledTimes(1);
-    // The workspace id goes with the run: a failure of it is then recorded against a document this
-    // browser can resolve, which is what makes the notification about it actionable.
+    // Recorded against a document this browser can resolve, which is what makes its failure actionable.
     expect(runStored).toHaveBeenCalledWith(
       "backend-sec",
       [{ size: 100 }],
@@ -126,8 +125,7 @@ describe("auto-run ordered chaining", () => {
       await vi.advanceTimersByTimeAsync(1);
     });
 
-    // The next policy (order 1) fires on the first policy's output, not the original - and reports
-    // that output's own workspace id, since that is the document now in front of the user.
+    // Fires on the first policy's output and reports that output's own id, not the original's.
     expect(runStored).toHaveBeenCalledWith(
       "backend-cls",
       [{ size: 100 }],
@@ -160,13 +158,8 @@ describe("auto-run ordered chaining", () => {
   });
 
   it("never dispatches on a file marked derivedFromTool", async () => {
-    // THE GATE OTHER CODE RELIES ON. A file added programmatically after it has already been through a
-    // policy or a tool carries this flag, and this effect must leave it alone. A policy run is a BILLED
-    // automation run, so both callers charge the customer for work nobody asked for if this stops
-    // holding: policy output delivery (`importOutputs`) would re-enforce a policy on its own output
-    // forever, and the bell's "Decrypt and retry" (`notificationActions.adopt`) would have the adoption
-    // of an unlocked document fire the whole upload chain on it. If this test fails, fix the gate rather
-    // than the test.
+    // A policy run is billed, so this gate is what stops `importOutputs` re-enforcing a policy on
+    // its own output forever. If this fails, fix the gate rather than the test.
     setFileStubs([
       { id: "file-1", name: "unlocked.pdf", derivedFromTool: true },
     ]);

frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.ts

@@ -270,8 +270,7 @@ export function usePolicyAutoRun(): void {
           dispatchKey(finished.categoryId, finished.fileId),
         );
       }
-      // A run that failed has just recorded an incident against the person watching it, so read the
-      // list rather than leaving them to wait out a poll interval for news of their own upload.
+      // Read now rather than leaving them a poll interval to hear about their own upload.
       if (view.status === "FAILED") refreshNotificationsNow();
       const code = view.errorCode;
       if (code !== "PAYG_LIMIT_REACHED" && code !== "FEATURE_DEGRADED") return;
@@ -961,9 +960,8 @@ async function runPolicyOnFile(
   await acquireDispatchSlot(priority);
   try {
     const target = resolvePolicyRunTarget();
-    // Hand the workspace id over with the run, so a failure of it is recorded against a document this
-    // browser can resolve and the notification can offer to open or unlock it. One file per run here,
-    // which is the only shape the server records a reference for.
+    // Recorded against a document this browser can resolve. One file per run, which is the only
+    // shape the server keeps a reference for.
     const runId = await runStoredPolicy(backendId, [file], fileId);
     // recordRunStart marks this (policy, file) dispatched as it records the run.
     recordRunStart({

frontend/editor/src/proprietary/services/policyApi.test.ts

@@ -1,9 +1,8 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
 /**
- * What the run request puts on the wire. Only the document reference is pinned, because it is the one
- * field with a rule attached: the server records it against any failure of the run, so a filename here
- * would put a document name into a table that deliberately has nowhere to keep one.
+ * Only the document reference is pinned, being the one field with a rule attached: a filename here
+ * would reach a table that deliberately has nowhere to keep one.
  */
 
 const post = vi.fn().mockResolvedValue({ data: { jobId: "run-1" } });

frontend/editor/src/proprietary/services/policyApi.ts

@@ -61,11 +61,8 @@ export async function reorderPolicies(orderedIds: string[]): Promise<void> {
 }
 
 /**
- * Run a stored policy by id on the supplied files; returns the run id.
- *
- * `fileId` is this workspace's own opaque id for the document being run. The server records it against
- * any failure of the run, which is the only way an attended failure can name a document this browser can
- * resolve. Only honoured for a single-document run, and never a filename.
+ * Run a stored policy by id; returns the run id. `fileId` is this workspace's own opaque id, recorded
+ * against any failure of the run. Only honoured for a single-document run, and never a filename.
  */
 export async function runStoredPolicy(
   id: string,