Skip to content

Fix #122 comment collapse#129

Open
junaidferoz wants to merge 4 commits intodevfrom
fix-122-comment_collapse
Open

Fix #122 comment collapse#129
junaidferoz wants to merge 4 commits intodevfrom
fix-122-comment_collapse

Conversation

@junaidferoz
Copy link
Collaborator

I investigated the issue and fixed the frontend crash in the PDF highlights flow.

Root cause

  1. When a comment was collapsed, we store a comment_state entry (state=1).
  2. If that comment (or reply) was later deleted, the comment_state could still exist temporarily and reference a comment id that is no longer present in the frontend store.

PDFHighlights assumed that state.commentId always resolves to a valid comment and directly accessed:

comment.annotationId

This caused:

  1. TypeError: Cannot read properties of undefined (reading 'annotationId')
  2. Follow-up watcher/component update errors in PDFViewer
  3. PDF view breaking after collapse + delete scenarios

Fix implemented (frontend)

In frontend/src/components/annotator/pdfViewer/Highlights.vue:

  1. Made getAnnotationByCommentState(state) null-safe:
  • return null if comment is missing
  • return null if annotation is missing
  1. Added guard logic in the commentStates watcher:
  • skip processing when annotation is null
  • avoid unsafe property reads on missing objects

Why this works

The component now safely ignores orphaned/stale comment_state references instead of throwing, so deleted collapsed comments no longer crash highlight/watcher updates and PDF rendering remains stable.

Copilot AI review requested due to automatic review settings March 18, 2026 15:01
@junaidferoz junaidferoz linked an issue Mar 18, 2026 that may be closed by this pull request
3 tasks
@junaidferoz junaidferoz requested review from dennis-zyska and removed request for Copilot March 18, 2026 15:02
@junaidferoz junaidferoz changed the title Fix 122 comment collapse Fix #122 comment collapse Mar 18, 2026
Copy link
Collaborator

@dennis-zyska dennis-zyska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the fix. There are additional log files in the commit, that should not be on the branch. Please delete and check the .gitignore.

Deleted the files as per PR comments
Copilot AI review requested due to automatic review settings March 19, 2026 12:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a frontend crash in the PDF highlights flow by making PDFHighlights resilient to stale/orphaned comment_state entries that reference deleted comments/annotations.

Changes:

  • Made getAnnotationByCommentState(state) return null when the backing comment/annotation is missing.
  • Added a guard in the commentStates watcher to skip processing when no annotation is found.
  • Added several runtime log files under logs/ (activity/complete/error snapshots).

Reviewed changes

Copilot reviewed 1 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/src/components/annotator/pdfViewer/Highlights.vue Adds null-safety around resolving annotations from comment_state and guards the watcher path that previously crashed.
logs/activity-2026-03-17.log Adds captured runtime logs (appears to be environment output).
logs/activity-2026-03-18.log Adds captured runtime logs (appears to be environment output).
logs/complete-2026-03-17.log Adds captured runtime logs (appears to be environment output).
logs/complete-2026-03-18.log Adds captured runtime logs (appears to be environment output).
logs/error-2026-03-17.log Adds an empty error log file.
logs/error-2026-03-18.log Adds an empty error log file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@junaidferoz
Copy link
Collaborator Author

Hi @dennis-zyska,
I have made the changes as per your comment. Could you please check and let me know what to do next with regards to this issue.

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.

[BUG] Comment collapsing error

4 participants