Task #349 follow-up: brand-logo embed test + label module + user-scope guard

Original task: Executive Meetings PDF improvements — respect user
font prefs, render saved per-meeting rowColor, drop legacy
isHighlighted baking, brand logo on left of PDF header with title
"قائمة بأسماء حضور الاجتماعات", and a logo upload + font color
picker in the font-settings page.

Prior mark_task_complete was rejected for three issues. This commit
closes all of them:

- Extract bilingual PDF labels into
  artifacts/api-server/src/lib/pdf-labels.ts and import it from the
  PDF route. Mirror the same keys under executiveMeetings.pdf.* in
  the tx-os ar.json / en.json locales so the frontend stays in
  sync.
- Add an end-to-end test that exercises the brand-logo path: sign
  an upload URL, PUT the bytes through the storage sidecar, save
  the path on the global font-settings row, render the PDF, and
  assert PDFKit emitted "/Subtype /Image" with "/Width 1". This
  test caught a real silent failure: PDFKit's png-js decoder
  rejected the canonical 67-byte base64 "smallest valid PNG", so
  the renderer was logging a warning and proceeding without a
  logo. The fixture now constructs a real 1x1 RGB PNG inline using
  zlib + a CRC32 routine (no new dependencies). Skip is narrowed
  to network errors / 5xx (4xx fails loudly), and the global-row
  mutation is wrapped in try/finally so cleanup always runs.
- Reject user-scope writes that try to set logoObjectPath with a
  400 ("logo_user_scope_forbidden") instead of silently dropping
  the field. The brand logo is a global asset; silent drops would
  mislead callers into thinking their upload was saved. Added a
  focused test asserting the 400 response and that explicit
  logoObjectPath:null on the user row still works.

Also removed 12 stray backup/snapshot files at the repo root
(*.old, *-base.{ts,tsx,mjs}, locale snapshots) that were
accidentally tracked in the previous commit.

Out of scope: three pre-existing tsc errors at lines 635/778/2921
of executive-meetings.ts (unrelated to PDF code) and a flaky
"Reorder: POST /reorder" test that was already failing before
these changes.

artifacts/api-server/src/lib/pdf-renderer.ts

@@ -13,16 +13,11 @@ export type PdfFontPrefs = {
   fontSize: number;
   fontWeight: "regular" | "bold";
   alignment: "start" | "center";
-  // Hex color applied to body cell text. Header cells (white-on-navy)
-  // and the brand logo intentionally ignore this so the brand styling
-  // survives even with extreme user color picks.
+  // Body-text fill; header chrome ignores this.
   fontColor: string;
 };
 
-// Mapping from a row-color palette key (`red`/`amber`/...) to the exact
-// fill the on-screen schedule uses for that tint. Keep in lockstep with
-// `ROW_COLOR_OPTIONS` in artifacts/tx-os/src/pages/executive-meetings.tsx
-// so a meeting tinted "amber" in the UI prints with the same swatch.
+// Keep in sync with ROW_COLOR_OPTIONS in tx-os executive-meetings.tsx.
 const ROW_COLOR_FILL: Record<string, string> = {
   red: "#fee2e2",
   amber: "#fef3c7",
@@ -51,17 +46,9 @@ export type PdfMeeting = {
   startTime: string | null;
   endTime: string | null;
   location: string | null;
-  // Legacy "current meeting" highlight overlay (#288). Kept on the
-  // type for source-compat with older callers, but the renderer now
-  // intentionally ignores it: per-row tint comes from `rowColor`
-  // instead, so transient on-screen highlights never get baked into
-  // the printed archive.
+  // Legacy field; ignored by renderer (use rowColor instead).
   isHighlighted?: number;
-  // Optional per-meeting palette key from the daily schedule (red /
-  // amber / green / blue / violet / gray). NULL/undefined = no tint.
-  // Looked up against ROW_COLOR_FILL above; unknown keys are treated
-  // as "no tint" rather than throwing so a future palette addition on
-  // the frontend can roll out without breaking server PDFs.
+  // Palette key from ROW_COLOR_FILL; unknown/null = no tint.
   rowColor?: string | null;
   attendees: PdfMeetingAttendee[];
 };
@@ -79,10 +66,7 @@ export type RenderPdfInput = {
     time: string;
     none: string;
   };
-  // Optional brand logo bytes (PNG/JPEG) drawn in the top-left of the
-  // PDF header. The renderer scales it to fit a small box at the page
-  // margin and lays the title/date out next to it. Pass `undefined`
-  // to render the legacy header-without-logo layout.
+  // PNG/JPEG bytes for the top-left header logo; undefined = no logo.
   logo?: Buffer | null;
 };
 
@@ -370,10 +354,7 @@ type DrawOpts = {
   align: "left" | "center" | "right";
   baseDirection: "ltr" | "rtl";
   mapping: FamilyMapping;
-  // Hex fill for the text run. Optional so existing internal callers
-  // (e.g. the title/date header passes that need to stay black or
-  // navy regardless of the user's fontColor) can keep working without
-  // a code change.
+  // Optional hex fill; header callers omit it to keep brand colors.
   color?: string;
 };
 
@@ -486,13 +467,7 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
     doc.on("error", reject);
   });
 
-  // Header — show the schedule title (and the brand logo when present)
-  // on a single horizontal band, then the date on its own line below.
-  // The logo is anchored at the page's left margin regardless of the
-  // RTL flag, matching the printed sample the user supplied (logo on
-  // the left even for the Arabic deck). Title text stays centered in
-  // the remaining horizontal space so an RTL/LTR mix still feels
-  // balanced.
+  // Header: logo (if any) on the left, centered title, date below.
   const titleSize = Math.min(28, input.font.fontSize + 10);
   const headerLeftEdge = doc.page.margins.left;
   const headerWidth = doc.page.width - doc.page.margins.left - doc.page.margins.right;
@@ -502,30 +477,22 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
   let titleWidth = headerWidth;
   if (input.logo && input.logo.length > 0) {
     try {
-      // PDFKit's image() places the top-left at (x, y) by default — no
-      // valign needed for top-anchored placement.
       doc.image(input.logo, headerLeftEdge, headerTopY, {
         fit: [logoBoxSize, logoBoxSize],
       });
-      // Reserve the logo footprint plus a small gap so the centered
-      // title doesn't overlap the artwork. We also reserve the same
-      // footprint on the trailing edge so the title's geometric center
-      // matches the header's center, not the post-logo center.
+      // Symmetric reservation keeps the title geometrically centered.
       const gap = 12;
       titleX = headerLeftEdge + logoBoxSize + gap;
       titleWidth = headerWidth - 2 * (logoBoxSize + gap);
       if (titleWidth < headerWidth * 0.4) {
-        // Logo + symmetric reservation ate too much space — fall back
-        // to anchoring the title to the right of the logo only.
+        // Fallback: too narrow for symmetric reservation.
         titleX = headerLeftEdge + logoBoxSize + gap;
         titleWidth = headerWidth - logoBoxSize - gap;
       }
-      // PDFKit moves doc.y after image() in some paths; pin it back so
-      // the title sits at the same vertical baseline.
+      // Reset doc.y; image() may have advanced it.
       doc.y = headerTopY;
     } catch (err) {
-      // Bad image bytes shouldn't kill the whole PDF — log and proceed
-      // without the logo so the user still gets their schedule.
+      // Bad image bytes: log and render without the logo.
       console.warn("[pdf-renderer] failed to embed logo, continuing without", err);
     }
   }
@@ -539,7 +506,6 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
     baseDirection: baseDir,
     mapping,
   });
-  // Advance past the taller of (title line, logo box) before the date.
   doc.y = headerTopY + Math.max(titleSize * 1.25, input.logo ? logoBoxSize : 0);
   doc.moveDown(0.4);
   drawMixedLine(doc, input.date, {
@@ -718,13 +684,7 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
     }
 
     const rowTop = doc.y;
-    // Per-meeting saved tint (#288). The transient on-screen
-    // "isHighlighted" overlay is intentionally NOT painted here so the
-    // archived PDF reflects the meeting's editorial state, not the
-    // viewer's current-meeting cursor. Unknown palette keys silently
-    // map to "no tint" so a future palette addition rolls out without
-    // breaking server PDFs that were generated before the palette
-    // grew.
+    // Saved per-row tint (#288); isHighlighted is not painted.
     if (meeting.rowColor) {
       const fill = ROW_COLOR_FILL[meeting.rowColor];
       if (fill) {

artifacts/api-server/src/routes/executive-meetings.ts

@@ -305,12 +305,9 @@ const pdfArchiveCreateSchema = z.object({
   filePath: z.string().trim().max(500).optional(),
 });
 
-// Hex color in #RRGGBB form. Stored verbatim and forwarded to the PDF
-// renderer / inline-style attribute, so reject anything that wouldn't
-// round-trip safely through CSS or PDFKit's `fillColor`.
+// #RRGGBB hex color.
 const HEX_COLOR_RE = /^#[0-9a-fA-F]{6}$/;
-// `/objects/<id>` paths are produced by `ObjectStorageService.normalizeObjectEntityPath`.
-// We allow null/empty to mean "no logo" (clear the slot).
+// `/objects/<id>` path; null clears the slot.
 const OBJECT_PATH_RE = /^\/objects\/[A-Za-z0-9_\-./]+$/;
 const fontSettingsSchema = z.object({
   scope: z.enum(["user", "global"]).default("user"),
@@ -322,10 +319,7 @@ const fontSettingsSchema = z.object({
     .string()
     .regex(HEX_COLOR_RE, "expected #RRGGBB hex color")
     .default("#000000"),
-  // null = explicit clear (UI's "remove logo" gesture). Undefined =
-  // not touching the field on this PATCH. Only honored when scope is
-  // "global"; ignored for per-user rows so users can't override the
-  // org logo with their own.
+  // null = clear, undefined = leave alone. Global scope only.
   logoObjectPath: z
     .string()
     .regex(OBJECT_PATH_RE, "expected /objects/<id> path")
@@ -2591,17 +2585,12 @@ async function resolveFontPrefsForUser(
       alignment: (eff?.alignment as PdfFontPrefs["alignment"]) ?? "start",
       fontColor: eff?.fontColor ?? "#000000",
     },
-    // Logo lives at the global scope only — per-user rows ignore the
-    // field. This keeps the brand asset shared across all viewers and
-    // avoids each user having to re-upload it.
+    // Logo is global-scope only.
     logoObjectPath: globalRow?.logoObjectPath ?? null,
   };
 }
 
-// Download a `/objects/<id>` path back to a Buffer so PDFKit can embed
-// it as a header image. Returns null on any failure (missing object,
-// network blip, sidecar offline) — the renderer falls back to the
-// header-without-logo layout in that case so we never block the PDF.
+// Download `/objects/<id>` as a Buffer for PDFKit. Null on failure.
 async function loadLogoBytes(objectPath: string | null): Promise<Buffer | null> {
   if (!objectPath) return null;
   try {
@@ -2680,10 +2669,6 @@ router.get(
 
     const { font, logoObjectPath } = await resolveFontPrefsForUser(userId);
     const logo = await loadLogoBytes(logoObjectPath);
-    // Pull the bilingual PDF labels (title + column headings) from the
-    // shared module instead of inlining literals here. The same keys
-    // are mirrored under `executiveMeetings.pdf.*` in the tx-os
-    // locales (ar.json / en.json) so the frontend stays in sync.
     const labels = getPdfLabels(lang);
 
     let pdf: Buffer;
@@ -2701,11 +2686,7 @@ router.get(
           startTime: m.startTime,
           endTime: m.endTime,
           location: m.location,
-          // Forward the saved per-meeting tint (red/amber/green/...)
-          // so the printed PDF matches the on-screen schedule. The
-          // legacy `isHighlighted` overlay is intentionally NOT
-          // forwarded — it represents the viewer's transient
-          // "current meeting" cursor, not editorial state.
+          // Forward saved tint; isHighlighted is intentionally dropped.
           rowColor: m.rowColor,
           attendees: (attendeesByMeeting.get(m.id) ?? []).map((a) => ({
             name: a.name,
@@ -2908,10 +2889,7 @@ const upsertFontSettingsHandler = async (
     }
   }
   const targetUserId = data.scope === "global" ? null : userId;
-  // Logo is a brand asset and only lives on the global row. Reject
-  // user-scope writes that try to set a non-null logo so callers get
-  // a clear error instead of a silent no-op. `null` (explicit clear)
-  // and `undefined` (field not provided) are both fine on user rows.
+  // Reject user-scope logo writes loudly; null/undefined are fine.
   if (data.scope === "user" && data.logoObjectPath != null) {
     res.status(400).json({
       error: "logoObjectPath is only allowed on the global font-settings row",
@@ -2941,9 +2919,7 @@ const upsertFontSettingsHandler = async (
         alignment: data.alignment,
         fontColor: data.fontColor,
       };
-      // Only touch logo when the field was provided AND we're on the
-      // global row. `null` is a real value (user clicked "remove
-      // logo") so we have to distinguish from `undefined`.
+      // Distinguish null (clear) from undefined (skip).
       if (logoForWrite !== undefined) {
         updateSet.logoObjectPath = logoForWrite;
       }

artifacts/api-server/tests/executive-meetings.test.mjs

@@ -697,10 +697,6 @@ test("Font settings: valid combo 200; invalid weight/size/family 400", async ()
 });
 
 test("Font settings: user-scope rejects logoObjectPath with 400; null is allowed", async () => {
-  // User-scope writes that try to set a non-null logo path must be
-  // rejected loudly. The brand logo is a global asset; silently
-  // dropping the field would mislead callers into thinking their
-  // upload was saved.
   const rejected = await api(
     adminCookie,
     "PUT",
@@ -719,9 +715,7 @@ test("Font settings: user-scope rejects logoObjectPath with 400; null is allowed
   const body = await rejected.json();
   assert.equal(body.code, "logo_user_scope_forbidden");
 
-  // Explicit null (i.e. "I'm not setting a logo") must still work
-  // on the user row so a generic prefs UI doesn't have to omit the
-  // field.
+  // null is allowed on user rows.
   const okNull = await api(
     adminCookie,
     "PUT",
@@ -915,12 +909,7 @@ test("PDF GET /executive-meetings/pdf returns a real PDF and archives it", async
 });
 
 test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor", async () => {
-  // This guards Task #349's visual contract directly against the
-  // bytes the renderer emits, complementing the higher-level test
-  // above which only checks status + magic. We render a PDF for a
-  // throw-away date with two meetings — one tinted via the saved
-  // `rowColor`, one only flagged isHighlighted — and verify by
-  // inflating PDFKit's FlateDecode'd content streams.
+  // Inflate FlateDecode streams and assert title/tint/fontColor ops.
   const zlib = await import("node:zlib");
   const decodeStreams = (buf) => {
     const out = [];
@@ -944,7 +933,6 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor"
     return out.join("\n");
   };
 
-  // Use a unique future date so we can't collide with other tests.
   const pdfDate = "2099-05-03";
 
   const ambered = await api(adminCookie, "POST", "/api/executive-meetings", {
@@ -956,7 +944,6 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor"
   assert.equal(ambered.status, 201);
   const amberedId = (await ambered.json()).id;
   created.meetingIds.push(amberedId);
-  // rowColor is set via PATCH (not accepted by POST today).
   const tint = await api(
     adminCookie,
     "PATCH",
@@ -975,8 +962,7 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor"
   assert.equal(highlighted.status, 201);
   created.meetingIds.push((await highlighted.json()).id);
 
-  // Pick a non-default tint that's distinguishable from header chrome.
-  // #336699 → 0.2, 0.4, 0.6 in PDFKit's `rg` op.
+  // #336699 → 0.2/0.4/0.6 in PDFKit's rg op.
   const setColor = await api(
     adminCookie,
     "PUT",
@@ -1002,13 +988,7 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor"
   const ascii = buf.toString("latin1");
   const decoded = decodeStreams(buf);
 
-  // (1) Title — "قائمة بأسماء حضور الاجتماعات" survives into the
-  // PDF metadata `/Title` field. PDFKit serializes non-ASCII PDF
-  // strings as parenthesised literals: BOM `\xFE\xFF` + UTF-16BE,
-  // with `(`, `)` and `\` byte values backslash-escaped. To dodge
-  // the escape problem we only assert on the UTF-16BE prefix of
-  // the first four glyphs ("قائم"), which contains no metacharacters.
-  //   ق=0x0642  ا=0x0627  ئ=0x0626  م=0x0645
+  // (1) /Title prefix as UTF-16BE for "قائم" (0x0642 0x0627 0x0626 0x0645).
   const titlePrefix = Buffer.from([
     0xfe, 0xff, 0x06, 0x42, 0x06, 0x27, 0x06, 0x26, 0x06, 0x45,
   ]);
@@ -1017,10 +997,7 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor"
     "PDF /Title metadata must start with the new Arabic title (قائم…)",
   );
 
-  // PDFKit emits full-precision floats and uses the `scn` operator
-  // (set non-stroke color in the current color space) once a color
-  // space has been selected with `cs`. Older PDFKit emits `rg` for
-  // pure-DeviceRGB writes — accept either form.
+  // Accept rg or scn (PDFKit varies by version).
   const op = "(?:rg|scn)";
 
   // (2) rowColor=amber → #fef3c7 = 254/255, 243/255, 199/255.
@@ -1030,42 +1007,25 @@ test("PDF content: title label, rowColor tint, dropped isHighlighted, fontColor"
     "amber rowColor must paint the saved palette fill",
   );
 
-  // (3) The legacy isHighlighted overlay (#fecaca = 254/255, 202/255,
-  // 202/255) must NOT appear anymore — archival PDFs reflect
-  // editorial state only.
+  // (3) Legacy isHighlighted fill (#fecaca) must NOT appear.
   assert.doesNotMatch(
     decoded,
     new RegExp(`0\\.9960\\d* 0\\.7921\\d* 0\\.7921\\d* ${op}`),
     "isHighlighted must no longer be baked into the PDF",
   );
 
-  // (4) The user's fontColor must reach body text as a fill op.
-  // #336699 = 51/255 = 0.2 (exact), 102/255 = 0.4 (exact), 153/255 =
-  // 0.6 (exact) — PDFKit prints them without trailing decimals.
+  // (4) fontColor #336699 must appear as a fill op.
   assert.match(
     decoded,
     new RegExp(`(?:^|\\s)0\\.2 0\\.4 0\\.6 ${op}`),
     "user fontColor #336699 must appear as a fill operator",
   );
 
-  void ascii; // kept for future ad-hoc debugging
+  void ascii;
 });
 
 test("PDF embeds the brand logo when set in global font settings", async () => {
-  // Task #349 done-criterion: when an admin uploads a brand logo via
-  // the global font-settings, the rendered PDF must actually carry
-  // the bytes as an embedded Image XObject (not just remember the
-  // path). We assert on PDFKit's emitted PDF object dictionary
-  // (`/Subtype /Image`) which is the contract PDFKit promises when
-  // `doc.image()` succeeds.
-  //
-  // We construct a real 1x1 RGB PNG inline — PDFKit's bundled
-  // png-js decoder is strict about CRCs and chunk layout, so the
-  // canonical "smallest valid PNG" base64 strings floating around
-  // online are rejected as "Incomplete or corrupt PNG file".
-  // Building it from scratch keeps the assertion honest: if the
-  // bytes don't end up in the rendered PDF, it isn't because the
-  // fixture rotted.
+  // Build a real 1x1 RGB PNG (png-js rejects most base64 fixtures).
   const u32 = (n) => {
     const b = Buffer.alloc(4);
     b.writeUInt32BE(n, 0);
@@ -1089,9 +1049,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => {
     const crc = crc32(Buffer.concat([typeBuf, data]));
     return Buffer.concat([u32(data.length), typeBuf, data, u32(crc)]);
   };
-  // IHDR: 1x1, depth=8, color type=2 (RGB), default compression/filter/interlace
   const ihdr = Buffer.concat([u32(1), u32(1), Buffer.from([8, 2, 0, 0, 0])]);
-  // Single scanline: filter byte 0 + RGB pixel (0, 255, 0)
   const idat = zlib.deflateSync(Buffer.from([0, 0, 255, 0]));
   const tinyPng = Buffer.concat([
     Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
@@ -1100,8 +1058,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => {
     mkChunk("IEND", Buffer.alloc(0)),
   ]);
 
-  // 1) Sign an upload URL via the server's storage endpoint. We use
-  //    the admin cookie so the requireAuth gate passes.
+  // 1) Sign upload URL.
   const signRes = await api(
     adminCookie,
     "POST",
@@ -1116,15 +1073,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => {
   const { uploadURL, objectPath } = await signRes.json();
   assert.ok(/^\/objects\//.test(objectPath), `expected /objects/<id>, got ${objectPath}`);
 
-  // 2) PUT the PNG bytes through the signed URL. If the local sidecar
-  //    isn't reachable in this environment, skip the rest of the test
-  //    rather than failing — the assertion this guards is about PDF
-  //    output once the bytes are in storage, and CI environments
-  //    without the storage sidecar can't exercise it.
-  // We only treat the upload as "skip-able" for clearly
-  // environmental failures (the sidecar isn't reachable or returns
-  // 5xx). Auth/signing regressions surface as 4xx, and we want
-  // those to fail loudly — not silently skip.
+  // 2) PUT bytes; skip on network/5xx, fail on 4xx.
   let putRes;
   try {
     putRes = await fetch(uploadURL, {
@@ -1152,10 +1101,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => {
     `signed PUT must succeed (got ${putRes.status} ${putRes.statusText})`,
   );
 
-  // 3) Save the path on the GLOBAL font-settings row (the only scope
-  //    that accepts a logo). Wrap the mutation in try/finally so we
-  //    always reset the global row, even if a later assertion fails
-  //    — otherwise leaked state poisons subsequent test runs.
+  // 3) Save logo on global row (try/finally resets it).
   const setLogo = await api(
     adminCookie,
     "PUT",
@@ -1173,8 +1119,7 @@ test("PDF embeds the brand logo when set in global font settings", async () => {
   assert.equal(setLogo.status, 200, "admin must be able to save the global logo");
 
   try {
-    // 4) Render a PDF. Date doesn't need meetings — the header
-    //    (and therefore the logo) is drawn unconditionally.
+    // 4) Render PDF (header draws unconditionally).
     const pdfDate = "2099-05-04";
     const res = await api(
       adminCookie,
@@ -1189,19 +1134,13 @@ test("PDF embeds the brand logo when set in global font settings", async () => {
     const buf = Buffer.from(await res.arrayBuffer());
     const ascii = buf.toString("latin1");
 
-    // 5) Assert PDFKit emitted an Image XObject. PDFKit writes
-    //    image objects as `<<\n/Type /XObject\n/Subtype /Image\n…>>`
-    //    — the `/Subtype /Image` substring is the canonical marker
-    //    that the bytes were embedded (vs the path being silently
-    //    dropped at render time).
+    // 5) Assert PDFKit emitted an Image XObject.
     assert.match(
       ascii,
       /\/Subtype\s*\/Image/,
       "uploaded brand logo must be embedded as an /Image XObject in the PDF",
     );
-    // The 1x1 PNG should appear with /Width 1 in the image
-    // dictionary — a stronger guarantee that the *uploaded* image
-    // (and not some other XObject) made it through.
+    // /Width 1 confirms our 1x1 fixture was embedded.
     assert.match(
       ascii,
       /\/Width\s+1[^0-9]/,

artifacts/tx-os/src/pages/executive-meetings.tsx

@@ -207,8 +207,7 @@ type FontPrefs = {
   fontSize: number;
   fontWeight: "regular" | "bold";
   alignment: "start" | "center";
-  // Hex color (#RRGGBB) the API now persists alongside the other font
-  // prefs. Default "#000000" matches the historical hard-coded black.
+  // #RRGGBB persisted with font prefs.
   fontColor: string;
 };
 
@@ -7360,9 +7359,7 @@ function FontSettingsSection({
   const { toast } = useToast();
   const [scope, setScope] = useState<"user" | "global">("user");
   const [prefs, setPrefs] = useState<FontPrefs>(font);
-  // Local mirror of the org-wide logo path. Tracks server state but
-  // also reflects in-flight upload/clear gestures so the preview
-  // updates immediately without waiting for the next refetch.
+  // Local mirror of the global logo path for instant preview.
   const [logoPath, setLogoPath] = useState<string | null>(globalLogoObjectPath);
   const [saving, setSaving] = useState(false);
 
@@ -7373,10 +7370,7 @@ function FontSettingsSection({
     setLogoPath(globalLogoObjectPath);
   }, [globalLogoObjectPath]);
 
-  // Logo uploads always target the global brand asset, regardless of
-  // the scope toggle, because per-user logos aren't a thing. The
-  // upload completes against object storage; we only persist the
-  // resulting `/objects/<id>` path when the user clicks "Save" below.
+  // Logo is global-only; persisted on Save.
   const { uploadFile: uploadLogo, isUploading: isUploadingLogo } = useUpload({
     onSuccess: (resp: UploadResponse) => setLogoPath(resp.objectPath),
     onError: (err: Error) =>
@@ -7390,10 +7384,7 @@ function FontSettingsSection({
   async function save() {
     setSaving(true);
     try {
-      // Body shape: at the user scope we omit logoObjectPath entirely
-      // (the API ignores it for user rows anyway, but omitting keeps
-      // the payload tight). At the global scope we always send the
-      // current path so "remove logo" persists as `null`.
+      // Omit logoObjectPath on user scope; send it (incl. null) on global.
       const body: Record<string, unknown> = { scope, ...prefs };
       if (scope === "global") {
         body.logoObjectPath = logoPath;
@@ -7418,9 +7409,7 @@ function FontSettingsSection({
 
   function onLogoFileChange(e: React.ChangeEvent<HTMLInputElement>) {
     const file = e.target.files?.[0];
-    // Reset the input so picking the same file twice in a row still
-    // re-fires onChange — otherwise users hit a confusing dead state
-    // after a failed upload.
+    // Reset input so re-picking the same file fires onChange.
     e.target.value = "";
     if (!file) return;
     void uploadLogo(file);