Merge pull request #1051 from PR0M3TH3AN/codex/add-logging-for-improved-fetching-branch

Add dev logging for improved comment cache fallback

js/services/commentThreadService.js

@@ -59,6 +59,12 @@ function fromParentKey(parentKey) {
   return parentKey === ROOT_PARENT_KEY ? null : parentKey;
 }
 
+function logDev(loggerCandidate, ...args) {
+  if (loggerCandidate?.info) {
+    loggerCandidate.info(...args);
+  }
+}
+
 function safeCall(handler, payload) {
   if (typeof handler !== "function") {
     return;
@@ -318,6 +324,10 @@ export default class CommentThreadService {
     };
 
     if (!FEATURE_IMPROVED_COMMENT_FETCHING) {
+      logDev(
+        this.logger?.dev,
+        "[commentThread] Improved fetching fallback: feature disabled.",
+      );
       return fallbackFetch(options);
     }
 
@@ -328,6 +338,10 @@ export default class CommentThreadService {
     );
 
     if (!videoEventId) {
+      logDev(
+        this.logger?.dev,
+        "[commentThread] Improved fetching fallback: missing video event id.",
+      );
       return fallbackFetch(options);
     }
 
@@ -336,6 +350,11 @@ export default class CommentThreadService {
       return cached;
     }
 
+    logDev(
+      this.logger?.dev,
+      `[commentThread] Comment cache miss for ${videoEventId}, fetching fresh thread.`,
+    );
+
     const fetchOptions = {
       ...options,
       since: 0,
@@ -396,6 +415,10 @@ export default class CommentThreadService {
 
     const cacheKey = this.getCommentCacheKey(videoEventId);
     if (!cacheKey) {
+      logDev(
+        this.logger?.dev,
+        "[commentThread] Comment cache skipped: invalid video id.",
+      );
       return null;
     }
 
@@ -408,12 +431,20 @@ export default class CommentThreadService {
     }
 
     if (raw === null) {
+      logDev(
+        this.logger?.dev,
+        `[commentThread] Comment cache miss for ${videoEventId}: no entry present.`,
+      );
       return null;
     }
 
     try {
       const parsed = JSON.parse(raw);
       if (!parsed || typeof parsed !== "object") {
+        logDev(
+          this.logger?.dev,
+          `[commentThread] Comment cache rejected for ${videoEventId}: malformed payload.`,
+        );
         this.removeCommentCache(cacheKey);
         return null;
       }
@@ -423,6 +454,10 @@ export default class CommentThreadService {
         : null;
 
       if (cacheVersion !== COMMENT_CACHE_VERSION) {
+        logDev(
+          this.logger?.dev,
+          `[commentThread] Comment cache rejected for ${videoEventId}: version ${cacheVersion} != ${COMMENT_CACHE_VERSION}.`,
+        );
         this.removeCommentCache(cacheKey);
         return null;
       }
@@ -437,14 +472,17 @@ export default class CommentThreadService {
         Number.isFinite(timestamp) &&
         Date.now() - timestamp <= COMMENT_CACHE_TTL_MS
       ) {
-        if (devLogger?.info) {
-          devLogger.info(
-            `[commentThread] Loaded ${comments.length} cached comments for ${videoEventId}.`,
-          );
-        }
+        logDev(
+          this.logger?.dev,
+          `[commentThread] Loaded ${comments.length} cached comments for ${videoEventId}.`,
+        );
         return comments;
       }
 
+      logDev(
+        this.logger?.dev,
+        `[commentThread] Comment cache rejected for ${videoEventId}: entry expired.`,
+      );
       this.removeCommentCache(cacheKey);
     } catch (error) {
       if (this.logger?.warn) {
@@ -453,6 +491,10 @@ export default class CommentThreadService {
           error,
         );
       }
+      logDev(
+        this.logger?.dev,
+        `[commentThread] Comment cache rejected for ${videoEventId}: parse error.`,
+      );
       this.removeCommentCache(cacheKey);
     }
 
@@ -482,11 +524,10 @@ export default class CommentThreadService {
           timestamp: Date.now(),
         }),
       );
-      if (devLogger?.info) {
-        devLogger.info(
-          `[commentThread] Cached ${comments.length} comments for ${videoEventId}.`,
-        );
-      }
+      logDev(
+        this.logger?.dev,
+        `[commentThread] Cached ${comments.length} comments for ${videoEventId}.`,
+      );
     } catch (error) {
       this.handleCommentCacheError("write", videoEventId, error);
     }

tests/comment-thread-service.test.mjs

@@ -2,7 +2,14 @@ import test from "node:test";
 import assert from "node:assert/strict";
 
 import CommentThreadService from "../js/services/commentThreadService.js";
-import { COMMENT_EVENT_KIND, listVideoComments } from "../js/nostr/commentEvents.js";
+import {
+  COMMENT_EVENT_KIND,
+  listVideoComments,
+} from "../js/nostr/commentEvents.js";
+import {
+  FEATURE_IMPROVED_COMMENT_FETCHING,
+  setImprovedCommentFetchingEnabled,
+} from "../js/constants.js";
 import { buildCommentEvent } from "../js/nostrEventSchemas.js";
 
 function createBaseVideo() {
@@ -180,6 +187,106 @@ test("CommentThreadService surfaces cache write failures", () => {
   );
 });
 
+test("CommentThreadService logs cache usage and fallback decisions", async () => {
+  const initialFlag = FEATURE_IMPROVED_COMMENT_FETCHING;
+  const devLogs = [];
+  const fetchCalls = [];
+  const logger = {
+    warn: () => {},
+    dev: {
+      log: () => {},
+      info: (...args) => devLogs.push(args.join(" ")),
+      debug: () => {},
+      warn: (...args) => devLogs.push(args.join(" ")),
+      error: () => {},
+    },
+    user: { warn: () => {} },
+  };
+
+  const fetchVideoComments = async (...args) => {
+    fetchCalls.push(args);
+    return [
+      {
+        id: "remote-1",
+        pubkey: "remote",
+        content: "Fresh comment",
+        created_at: 2,
+        tags: [],
+      },
+    ];
+  };
+
+  const service = new CommentThreadService({
+    fetchVideoComments,
+    logger,
+  });
+
+  try {
+    setImprovedCommentFetchingEnabled(true);
+    const cachedComments = [
+      {
+        id: "cached-1",
+        pubkey: "cached",
+        content: "Cached comment",
+        created_at: 1,
+        tags: [],
+      },
+    ];
+
+    service.cacheComments("video123", cachedComments);
+    devLogs.length = 0;
+    fetchCalls.length = 0;
+
+    const cachedResult = await service.fetchThread({ videoEventId: "video123" });
+
+    assert.deepEqual(
+      cachedResult,
+      cachedComments,
+      "feature flag on should return cached comments",
+    );
+    assert.equal(
+      fetchCalls.length,
+      0,
+      "cache hits should avoid fallback fetch",
+    );
+    assert.equal(
+      devLogs.some((entry) =>
+        entry.includes("Loaded 1 cached comments for video123"),
+      ),
+      true,
+      "dev logs should note cache usage when enabled",
+    );
+
+    setImprovedCommentFetchingEnabled(false);
+    devLogs.length = 0;
+    fetchCalls.length = 0;
+
+    const fallbackResult = await service.fetchThread({
+      videoEventId: "video123",
+    });
+
+    assert.equal(
+      fetchCalls.length,
+      1,
+      "feature flag off should invoke fallback fetch",
+    );
+    assert.equal(
+      devLogs.some((entry) =>
+        entry.includes("Improved fetching fallback: feature disabled"),
+      ),
+      true,
+      "dev logs should note fallback when feature is disabled",
+    );
+    assert.deepEqual(
+      fallbackResult.map((comment) => comment.id),
+      ["remote-1"],
+      "fallback path should return freshly fetched comments",
+    );
+  } finally {
+    setImprovedCommentFetchingEnabled(initialFlag);
+  }
+});
+
 test("CommentThreadService normalizes mixed-case event ids in thread state", async () => {
   const video = { ...createBaseVideo(), id: "VideoMixed123" };
   const topLevelId = "CoMmEnT-Root";