Skip to content

Removed unused depth map from comment thread graph - #31677

Open
irontaek wants to merge 1 commit into
TryGhost:mainfrom
irontaek:chore/comments-ui-unused-depth-map
Open

irontaek wants to merge 1 commit into
TryGhost:mainfrom
irontaek:chore/comments-ui-unused-depth-map

Conversation

@irontaek

@irontaek irontaek commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Why

buildThreadGraph in apps/comments-ui/src/utils/thread-graph.ts fills a depthById map for every reply and never reads it:

const depthById = new Map<string, number>();
// …
depthById.set(reply.id, depth);

Those are the only two occurrences of depthById in the repository. The same value is written onto the reply itself one line earlier (reply.depth = depth), and every later depth lookup in the file (getAncestorAtDepth, getWindowForComment) reads reply.depth / current.depth.

What

Removes the map and the set call. Nothing else changes.

Tests

No test change. The removed map had no consumer, so behaviour is identical.

https://claude.ai/code/session_01W9Mk18mBNfkPfgg3hYweKq

buildThreadGraph filled depthById for every reply but never read it;
the depth is already stored on each reply as reply.depth.

Claude-Session: https://claude.ai/code/session_01W9Mk18mBNfkPfgg3hYweKq
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: TryGhost/Ghost/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Advanced
  • Run ID: 69090f40-69b7-410a-bc0c-8d0d53ceaaac

📥 Commits

Reviewing files that changed from the base of the PR and between 157ad0f and f5d625f.


📒 Files selected for processing (1)
  • apps/comments-ui/src/utils/thread-graph.ts

💤 Files with no reviewable changes (1)
  • apps/comments-ui/src/utils/thread-graph.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



Walkthrough

buildThreadGraph no longer creates a local depthById map or writes reply depths to it. It continues to assign depth values directly to each reply.

Priority: ⬇️ Low

Change: Refactor

Merge Risk: ⚪ Minimal · up to f5d62

No behavior change or merge-blocking concern is identified; this cleanup is mergeable after normal checks.

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: removing the unused depth map from the comment thread graph.
Description check Passed The description directly explains why the unused depthById map and its set call were removed, and it matches the reported changes.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Type-Safe Boundaries Passed The PR only removes the local depthById declaration and its set call from buildThreadGraph. The diff adds no boundary-data consumption, any, unchecked as, @ts-nocheck, or @ts-ignore. Dep…
New Files Are Typescript Passed The pull request modifies one pre-existing TypeScript file. It adds no files and adds no .js, .jsx, .cjs, or .mjs source file.


✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant