Skip to content

Remove non-null assertions, fix cache validation message logic, and merge main branch#11

Open
kertal with Copilot wants to merge 5 commits into
claude/local-first-indexdb-0zENVfrom
copilot/sub-pr-10
Open

Remove non-null assertions, fix cache validation message logic, and merge main branch#11
kertal with Copilot wants to merge 5 commits into
claude/local-first-indexdb-0zENVfrom
copilot/sub-pr-10

Conversation

Copilot AI commented Jan 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Addresses two TypeScript safety and UX issues in the local-first cache validation logic, and resolves merge conflicts with the main branch:

Remove non-null assertion operators

  • Replace metadata! with ternary pattern: metadata ? cacheUtils.isValidCache(metadata, ...) : false
  • Eliminate !! operators in boolean contexts where unnecessary
  • Maintain type safety without bypassing null checks

Fix cache status message logic

  • Check if cached data matches current query parameters, not just existence
  • Show "Cache outdated" only when cache exists AND matches query (stale but relevant)
  • Show "Starting search..." when cache is missing or for different parameters
// Before: Shows "Cache outdated" even for unrelated cached data
setLoadingProgress(hasCachedEvents || hasCachedSearchItems ?
  'Cache outdated, fetching fresh data...' :
  'Starting search...'
);

// After: Validates cache matches query before showing status
const cacheMatchesQuery = 
  (hasCachedEvents && eventsMetadata
    ? cacheUtils.matchesQuery(eventsMetadata, usernames, startDate, endDate)
    : false) ||
  (hasCachedSearchItems && searchItemsMetadata
    ? cacheUtils.matchesQuery(searchItemsMetadata, usernames, startDate, endDate)
    : false);

setLoadingProgress(cacheMatchesQuery ?
  'Cache outdated, fetching fresh data...' :
  'Starting search...'
);

Merge main branch and resolve conflicts

  • Merged latest changes from main branch
  • Resolved conflict in src/utils/indexedDB.ts by keeping both exports:
    • cacheUtils: Cache validation utilities for local-first architecture (from this branch)
    • prCacheStorage: PR cache storage utilities (from main branch)
  • Both utilities coexist as separate exports serving different purposes
  • All tests pass after merge

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

claude and others added 2 commits January 1, 2026 18:16
@claude
feat: Implement true local-first architecture with cache freshness ch…
04c2dfc 
…ecking

This commit refactors the data fetching and caching logic to follow true local-first principles:

**Changes:**

1. **Cache Freshness Utilities** (src/utils/indexedDB.ts):
   - Added CACHE_TTL_MS constant (30 minutes default)
   - Implemented cacheUtils.isFresh() to check cache age
   - Implemented cacheUtils.matchesQuery() to validate cache against current query
   - Implemented cacheUtils.isValidCache() to combine both checks

2. **Local-First Data Fetching** (src/hooks/useGitHubDataFetching.ts):
   - Added eventsMetadata and searchItemsMetadata parameters
   - Check cache freshness BEFORE deciding to block UI
   - If cache is fresh and matches query: non-blocking background refresh
   - If cache is stale or missing: blocking initial load
   - Users see cached data immediately when available
   - API fetches happen in background without locking the UI

3. **Updated App Integration** (src/App.tsx):
   - Destructure metadata from useIndexedDBStorage hooks
   - Pass metadata to useGitHubDataFetching for cache validation

**Before:** API-first with local caching (always blocks UI to fetch)
**After:** Local-first with background sync (shows cache, refreshes behind scenes)

**Benefits:**
- Instant data display when cache is fresh (< 30 min old)
- No UI blocking for background refreshes
- Better offline experience
- Clearer data flow that's easier to reason about
- True local-first architecture as documented

All tests pass. No breaking changes to existing functionality.
Initial plan
db9cf11
Copilot AI mentioned this pull request Jan 1, 2026
Draft
@kertal
Fix TypeScript null safety and cache validation logic
aee6cbd 
- Remove non-null assertion operators by using proper conditional checks
- Check cache query match before showing "Cache outdated" message
- Validate cache matches query parameters before displaying status

Co-authored-by: kertal <463851+kertal@users.noreply.github.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jan 1, 2026 •
edited
Loading

Copy link
Copy Markdown

Deploying git-vegas with  Cloudflare Pages  Cloudflare Pages

Latest commit: 861b858
Status: ✅  Deploy successful!
Preview URL: https://fe206960.git-vegas.pages.dev
Branch Preview URL: https://copilot-sub-pr-10.git-vegas.pages.dev

View logs

@kertal
Refactor to remove all non-null assertions
44ae170 
- Remove !! operator as it's unnecessary with logical AND
- Use consistent ternary pattern for cache query matching
- Eliminate redundant null checks while maintaining type safety

Co-authored-by: kertal <463851+kertal@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement local-first architecture with IndexDB Remove non-null assertions and fix cache validation message logic Jan 1, 2026
Copilot AI requested a review from kertal January 1, 2026 18:44
@kertal
kertal marked this pull request as ready for review January 8, 2026 05:48
@kertal
kertal force-pushed the claude/local-first-indexdb-0zENV branch from 04c2dfc to 806ef15 Compare January 9, 2026 22:43
@kertal

kertal commented Jan 11, 2026

Copy link
Copy Markdown
Owner

@copilot resolve conflicts

@kertal
Merge main branch and resolve conflicts in indexedDB.ts
861b858 
- Keep both cacheUtils (from this branch) and prCacheStorage (from main)
- Both utilities can coexist as separate exports
- All tests pass after merge

Co-authored-by: kertal <463851+kertal@users.noreply.github.com>

Copilot AI commented Jan 11, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve conflicts

Conflicts resolved in commit 861b858. The conflict in src/utils/indexedDB.ts was between cacheUtils (from this branch) and prCacheStorage (from main). Both exports are now included and all tests pass.

Copilot AI changed the title Remove non-null assertions and fix cache validation message logic Remove non-null assertions, fix cache validation message logic, and merge main branch Jan 11, 2026
Copilot AI requested a review from kertal January 11, 2026 19:48
@kertal

kertal commented Jan 11, 2026

Copy link
Copy Markdown
Owner

@claude fix conflicts

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

Reviewers

@kertal kertal Awaiting requested review from kertal

Assignees

@kertal kertal

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kertal @claude