Skip to content

Improve query history localStorage recovery handling#930

Open
shivambind269-ai wants to merge 3 commits into
imDarshanGK:mainfrom
shivambind269-ai:fix-history-persistence
Open

Improve query history localStorage recovery handling#930
shivambind269-ai wants to merge 3 commits into
imDarshanGK:mainfrom
shivambind269-ai:fix-history-persistence

Conversation

@shivambind269-ai

Copy link
Copy Markdown

Summary

This PR improves the reliability of Query History persistence by adding safe recovery logic when loading history from localStorage.

Changes Made

  • Added error handling around JSON.parse() when reading qyx_history.
  • Prevents application issues when localStorage contains malformed or corrupted data.
  • Falls back to an empty history array when parsing fails.
  • Preserves existing history persistence behavior and 50-entry limit.

Why This Change?

Although Query History is already stored in localStorage, corrupted or invalid JSON data can cause parsing failures during initialization. This change makes the history loading process more robust and improves user experience.

Testing

  • Verified application loads normally with valid history data.
  • Verified application does not crash when invalid JSON is present in qyx_history.
  • Verified history falls back safely to an empty state when parsing fails.

Fixes #718

@imDarshanGK

Copy link
Copy Markdown
Owner

@shivambind269-ai add demo

@shivambind269-ai

Copy link
Copy Markdown
Author

Hi @imDarshanGK,

Added demo/testing evidence for the localStorage recovery fix.

Test steps:

  1. Manually corrupted qyx_history in localStorage with invalid JSON.
  2. Refreshed the application.
  3. Verified JSON.parse() failure is caught by the added error handling.
  4. Verified the application continues to load normally without crashing.
  5. Verified Query History safely falls back to an empty state instead of breaking initialization.

Attached screenshot demonstrating the behavior.

Thank you for the review.

Screenshot 2026-06-14 201609

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This PR only adds JSON parse error handling, but the issue is about history not persisting after refresh. Please verify that the actual localStorage persistence and rehydration logic correctly fixes the original problem.

@imDarshanGK imDarshanGK added the invalid Contribution not following assignment rules label Jun 16, 2026
@shivambind269-ai

Copy link
Copy Markdown
Author

Thanks for the review.

I re-checked the history persistence flow. The current implementation already saves history to localStorage, reloads it on initialization, and calls renderHistory() during startup. My change only adds recovery handling for malformed qyx_history data by wrapping JSON.parse() in a try/catch block.

Given the linked issue is specifically about history not persisting after refresh, I agree this PR does not directly address that behavior. I'll investigate whether the refresh issue can still be reproduced on the current codebase and either update the PR with the actual fix or close/retarget it if the issue has already been resolved separately.

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This PR does not address the actual issue of Query History not persisting after refresh. It only adds error handling for JSON.parse, but persistence/re-hydration logic should be fixed to ensure history loads correctly from localStorage.

@shivambind269-ai

Copy link
Copy Markdown
Author

Thank you for the feedback.

I investigated the current implementation and reproduced the query history flow locally.

I verified that:

  1. A new history entry is added after analysis.
  2. The entry is saved to localStorage.
  3. After a browser refresh, the history entry remains visible and is reloaded correctly.
    Based on my testing, I was not able to reproduce the persistence issue described in [Bug] [Frontend] Query History does not persist after browser refresh #718 on the current codebase.
    My PR specifically addresses a separate issue where malformed or corrupted qyx_history data in localStorage can cause initialization failures. The added error handling allows the application to recover safely instead of breaking when invalid JSON is encountered.
    Please let me know if there is a specific scenario where the persistence issue can still be reproduced, and I'd be happy to investigate further.

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please ensure this PR fixes the root persistence issue (rehydration after refresh), not only JSON error handling.

@shivambind269-ai

Copy link
Copy Markdown
Author

Thank you for the clarification.
I re-tested the complete flow:

  • History is saved to localStorage.
  • History is reloaded on startup.
  • History remains after refresh.
  • Persisted entries can be restored successfully.
    I was not able to reproduce the persistence issue described in [Bug] [Frontend] Query History does not persist after browser refresh #718 on the current codebase.
    Could you please provide the current reproduction steps or environment where the rehydration issue still occurs? That will help me identify and implement the correct fix.

@shivambind269-ai shivambind269-ai deleted the fix-history-persistence branch June 19, 2026 10:46
@shivambind269-ai shivambind269-ai restored the fix-history-persistence branch June 19, 2026 10:46
@shivambind269-ai

Copy link
Copy Markdown
Author

Thank you for the review.

I have updated the branch and resolved the merge conflicts with the latest main.

I re-tested the current implementation and verified that query history is persisted, reloaded after refresh, and restored correctly in my local environment. My current change adds recovery handling for malformed localStorage data.

Could you please provide the current reproduction steps or scenario where the persistence/rehydration issue still occurs? I would be happy to investigate further and update the PR accordingly.

@imDarshanGK imDarshanGK left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@shivambind269-ai
The issue reports that Query History is not persisting after browser refresh. However, this PR only adds error handling when reading data from localStorage.

Please explain how this change fixes the reported bug and provide a demo showing:

  1. Add an analysis entry.
  2. Refresh the page.
  3. Verify the history is still present.
  4. Reopen the browser and verify the history remains available.

Also indicate whether any changes were made to the history save logic.

@shivambind269-ai

Copy link
Copy Markdown
Author

Thank you for the review.

After re-checking the implementation and my changes, I confirm that this PR does not modify the history save logic.

The existing save logic remains unchanged:

localStorage.setItem('qyx_history', JSON.stringify(history));

The change in this PR is limited to handling cases where qyx_history contains malformed or corrupted JSON. Previously, JSON.parse() could throw an exception during initialization and prevent history from loading correctly. This PR adds a safe fallback to an empty history state when parsing fails.
Regarding the requested demo:

  • I verified that history entries are added successfully.
  • I verified that history remains available after page refresh.
  • I verified that history remains available after reopening the browser in my local testing.
    Because the persistence behavior already appears to be working in the current codebase, I was unable to reproduce the issue described in [Bug] [Frontend] Query History does not persist after browser refresh #718. As a result, this PR does not address the root persistence bug and instead improves robustness of localStorage recovery.
    If the expectation is to fix [Bug] [Frontend] Query History does not persist after browser refresh #718 specifically, I may need updated reproduction steps for the persistence issue on the current codebase.

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

Labels

invalid Contribution not following assignment rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [Frontend] Query History does not persist after browser refresh

2 participants