Maint/improve - #26
Merged
Merged
Conversation
Add explicit verification barriers so tab/data mutations only happen after successful persistence or restore confirmation. Condense changes: - Add verifyCondenseWrite() in background condense flow. - After appendSavedGroup(), read back the saved group and verify id/url/title parity. - Skip chrome.tabs.remove() when verification fails, leaving source tabs open. - Log verification failures with runtime_context for diagnosis. Restore changes: - Add chunk-level restore verification in restore.ts. - Verify each restore chunk against expected URL multiplicity (supports duplicates) using created tab metadata first, then window query fallback. - Accept pendingUrl during verification to handle immediate post-create tab state. - Fail restoreTabs() when verification cannot confirm expected URLs. - Keep discard scheduling logic but ensure settings-toggle abort path flushes pending discards before aborting workers. - Remove unreachable branch guards in chunk sizing/empty-chunk logic for clarity and maintainability. Single restore (list page): - In restoreSingle(), verify the restored tab exists (url or pendingUrl) before removing it from saved storage. - If verification fails, keep the saved entry and show a user-facing status. Docs: - Update README and architecture docs to describe the new verification guardrails for condense and restore flows.
Expand integration coverage for the new atomic behavior and verification semantics. Background condense tests: - Add case proving tabs remain open when persisted group verification fails. - Add case proving duplicate-reject condense that saves zero tabs leaves source tabs open and does not duplicate saved entries. List page action tests: - Add restore-single case where verification fails and the tab remains in storage/list. - Add restore-all failure case where group storage is preserved. Restore logic tests: - Keep fallback verification path covered by stripping tabs from create() response while still creating real windows. - Update fallback query-failure expectation to false (cannot verify restore => fail-safe). - Add duplicate-URL restore test to cover multiset verification logic. These tests ensure database/list entries are only removed after restore success is actually verified, matching the atomicity requirement.
Implement URL-scheme filtering in the shared condense eligibility path so browser-internal tabs are never saved or closed during condense.\n\nWhat changed:\n- Added a centralized internal-URL prefix blocklist in shared condense helpers.\n- Updated eligibility filtering to reject internal browser surfaces while preserving existing pinned/list-tab behavior.\n- Normalized URL comparisons for list-tab matching and filtering decisions without mutating the original stored URL string.\n- Kept save-path logic aligned with eligibility filtering so persistence and filtering stay consistent if helpers are reused.\n\nWhy:\n- Users should not lose internal Chrome surfaces (for example settings/devtools/about pages) due to condense actions.\n- This keeps condense focused on user-content tabs and prevents surprising browser-UI disruption.\n\nTest coverage added:\n- Unit: explicit filtering assertions for chrome://, devtools://, and about: URLs.\n- Integration: end-to-end condense flow assertion that internal tabs remain open while eligible web tabs are condensed.
Update project documentation to reflect the new condense eligibility rule that excludes browser-internal tabs.\n\nWhat changed:\n- README: documented internal URL schemes skipped during condense.\n- spec.md: updated eligible-tab definition to explicitly exclude internal browser URLs.\n- spec.md: removed outdated non-goal that said there was no internal-tab special casing.\n- TEST_PLAN.md: updated core condense scenario wording to include internal-tab filtering.\n\nWhy:\n- Keeps behavioral docs aligned with implementation and tests.\n- Reduces ambiguity for maintainers and reviewers when validating condense behavior.
Root cause:\n- Collapse/expand state was inferred from transient DOM classes on group cards.\n- During imports, cards can be reconciled/recreated in different orders while storage-change refreshes run, so new and existing groups could end up with inconsistent collapse behavior.\n\nImplementation:\n- Added as the explicit source of truth for per-group collapse state.\n- Added to persist state changes and to sync card visuals from state without triggering eager loads.\n- Updated to write state first, then update DOM, then lazy-load only when expanded.\n- Updated index reconciliation (/) to prune stale collapse entries.\n- Updated to derive the toggle-all state from instead of DOM class inspection.\n- Updated first-render default collapse logic to initialize state () directly.\n- During post-initial renders, new groups now get deterministic defaults: if collapse-all is active, they start collapsed; otherwise they start expanded. Existing groups preserve their saved per-key state.\n- Updated delegated group toggle handler to use so all collapse paths share one stateful implementation.\n\nTests:\n- Added integration coverage for append-import behavior:\n - Existing groups keep their pre-import collapse/expand state.\n - Newly imported groups expand when collapse-all is not active.\n - Newly imported groups collapse when collapse-all is active.\n\nComplexity:\n- State updates remain O(1) for individual toggle operations and O(n) for full reconciliation/render passes (same asymptotic behavior as before).\n- No additional nested loops or quadratic paths were introduced.
Adds explicit import behavior documentation in the Export / Import JSON section. Documented rules: - Existing groups keep their current collapse/expand state after import. - Newly imported groups inherit the current global list mode: - collapsed when Collapse all is active - expanded otherwise. This matches the deterministic behavior implemented in list-page state management and covered by integration tests.
Contributor
Summary of ChangesHello @viseshrp, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and user experience of the extension by introducing critical safety guardrails for tab management. It ensures that tab condense and restore operations are verified for data integrity before modifying saved states, preventing data loss. Additionally, it refines the handling of browser-internal tabs during condense and improves the persistence of group collapse states on the list page, leading to more predictable and reliable behavior. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #26 +/- ##
=======================================
+ Coverage 94.5% 94.9% +0.3%
=======================================
Files 17 17
Lines 2064 2120 +56
Branches 529 555 +26
=======================================
+ Hits 1952 2012 +60
+ Misses 99 96 -3
+ Partials 13 12 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements, primarily focused on adding safety guardrails to prevent data loss during condense and restore operations. The new verification steps are a great addition. The refactoring of the group collapse/expand logic and the filtering of internal browser URLs are also valuable enhancements. I've identified one regression where the 'save memory on restore' feature is no longer working for single-tab restores, and a minor documentation issue. Overall, this is a solid set of changes that improves the extension's reliability.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.