Semantic search, NFT/profile/analytics pages, valora→perpetua rename

[zeeghazi]

Summary

• Rename valora → perpetua (routes, pages, types)
• Add NFT details page (/nft/:tokenId)
• Add user profile page (/user/:principal) with NFT history
• Add dashboard analytics page (/dashboard/analytics)
• Add semantic search on Alexandrian page
• Add backend similarity module (store/queries/updates)
• Add basic SEO meta tags across pages
• New engagementService in core/services
• Drop stale planning docs under plan/

[github-actions]

Code Review — PR 141

Overview

This PR delivers a significant feature batch: semantic (CLIP) image search, NFT details page, user profile page, analytics dashboard, engagement tracking, and a rename of the `valora` feature to `perpetua`. The rename itself is clean and thorough. Several new features contain correctness bugs worth fixing before merge.

---

Bugs

1. Impression dedup is broken in record_engagement_batch

File: `src/alex_backend/src/dialectica/api/updates.rs`

The docstring says "Impressions: deduped per authenticated user (one per arweave_id per user)", but the implementation just increments the counter unconditionally — it never checks whether the caller already registered an impression:

```rust
// Process impressions — dedup per user
for arweave_id in &impressions {
let current = imp_store.get(&StorableString(arweave_id.clone())).unwrap_or(0);
imp_store.insert(StorableString(arweave_id.clone()), current + 1); // no dedup
}
```

Either update the docstring to say impressions are not deduped, or mirror the views logic and store a set of principals.

---

2. get_trending inflates anonymous view counts

File: `src/alex_backend/src/dialectica/api/queries.rs`

`get_trending` counts `viewers.0.0.len()` to rank content. Anonymous viewers push `None` without any dedup (`record_view` always appends `None` for anonymous callers). A single anonymous user refreshing a page repeatedly inflates that token's trending rank unboundedly. Consider capping anonymous contributions or counting them separately.

---

3. Timestamp regression in Comment.tsx

File: `src/alex_frontend/core/features/nft/components/Comment.tsx`

The diff removes the nanoseconds-to-milliseconds conversion:
```

• {convertTimestamp(Number(comment.created_at) / 1000000, 'relative')}

• {convertTimestamp(comment.created_at, 'relative')}
  ```

`comment.created_at` is `nat64` (nanoseconds), a BigInt. The old code converted it to milliseconds. Unless `convertTimestamp` was simultaneously updated to accept nanoseconds or BigInt, every comment's relative timestamp will now display incorrectly. Verify `convertTimestamp`'s expected input unit.

---

4. reset_embeddings hardcodes a magic number instead of the constant

File: `src/alex_backend/src/similarity/updates.rs` line 758

```rust
*emb.borrow_mut() = ic_stable_structures::StableBTreeMap::new(
MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(30))) // should use EMBEDDINGS_MEM_ID
);
```

The constant `EMBEDDINGS_MEM_ID` is defined in `store.rs` specifically to avoid this. If the ID ever changes, `reset_embeddings` will silently operate on the wrong memory segment.

---

Performance Concerns

5. Linear scan in similarity search — O(n) per query

File: `src/alex_backend/src/similarity/queries.rs`

Both `search_similar` and `search_by_vector` iterate the entire `EMBEDDINGS` BTreeMap and compute cosine similarity against every entry. On an ICP canister with a 2-second query instruction limit, this will hit the instruction ceiling well before 10,000 embeddings. Consider documenting the expected scale or adding a hard cap on indexed entries.

6. get_indexed_ids has no pagination

File: `src/alex_backend/src/similarity/queries.rs`

```rust
fn get_indexed_ids() -> Vec {
EMBEDDINGS.with(|emb| emb.borrow().iter().map(|(key, _)| key.0.clone()).collect())
}
```

This returns every arweave ID in one call. If embeddings reach the tens of thousands, this query will exceed the response size limit. Add an optional offset/limit or remove if only used for debugging.

7. Trending sort is not cached

File: `src/alex_frontend/core/features/alexandrian/api/createTokenFetcher.ts`

`await alex_backend.get_trending(BigInt(100))` is called on every sort or page change in trending mode. This result should be cached with React Query (e.g., `staleTime: 5 * 60_000`) rather than re-fetched on every render cycle.

---

Code Quality

8. Significant logic duplication between NftPage and Arweave.tsx

File: `src/alex_frontend/lbry/src/pages/NftPage.tsx`

The price-history rendering (~80 lines) and share-button logic (~60 lines) in `NftPage.tsx` are near-identical copies of code in `Arweave.tsx`. These should be extracted into shared components (e.g., ``, ``) rather than duplicated.

9. UserProfilePage does not guard against invalid principal format

File: `src/alex_frontend/lbry/src/pages/UserProfilePage.tsx`

`Principal.fromText(principal)` throws if the principal string is malformed. Since the route `/user/:principal` accepts arbitrary URL input, wrap this in try/catch or validate the format to avoid unhandled errors crashing the page.

10. engagementService drops data on flush failure

File: `src/alex_frontend/core/services/engagementService.ts`

```ts
impressions.clear(); // cleared before the await
views.clear();
try {
await actor.record_engagement_batch(impressionIds, viewIds);
} catch (e) {
console.error("Failed to flush engagement:", e);
// data is gone
}
```

The pre-clear pattern avoids double-sending, but any network error silently drops all buffered engagement data. This may be acceptable for analytics, but a comment noting the tradeoff would help future readers.

11. Avatar fallback uses direct DOM mutation

File: `src/alex_frontend/lbry/src/pages/UserProfilePage.tsx`

```tsx
onError={(e) => {
(e.target as HTMLImageElement).style.display = "none";
(e.target as HTMLImageElement).nextElementSibling?.classList.remove("hidden");
}}
```

This is fragile — it breaks if the DOM structure changes. Use `const [imgError, setImgError] = useState(false)` and conditionally render based on that state instead.

12. skipped counter is always 0 in store_embeddings

File: `src/alex_backend/src/similarity/updates.rs`

`StoreResult` has a `skipped` field, but nothing in `store_embeddings` ever increments it — entries either succeed (`stored += 1`) or push to `errors`. Either remove the field or increment it for duplicate/overwrite entries.

---

Minor

• FilterBar.tsx indentation: The search `` has inconsistent indentation relative to the surrounding `
  `.
• Tab icon accessibility: Removing text labels from sidebar tabs in `Arweave.tsx` (Tags, Comments, Share → icons only) makes them inaccessible to screen readers. Add `aria-label` to each `TabsTrigger`.
• NftContext.Provider with no-op setModal in `NftPage.tsx`: passing `setModal: () => {}` means any child that calls `setModal` will silently fail. A comment noting this is intentional would help.

---

Summary

Severity	Count
Bug	4
Performance	3
Code quality	5
Minor	3

The rename, new pages, and backend engagement features are well-structured. The main blockers before merge are the impression-dedup bug (1), the timestamp regression in comments (3), and the embedding memory-ID magic number (4). The linear search scalability (5) should also have an explicit documented limit before the feature is enabled in production.

[github-actions]

Code Review — PR #141

Overview

This is a well-scoped, multi-feature PR. The valora → perpetua rename is clean and mechanical, the new pages (NFT detail, user profile, analytics) follow the codebase's established patterns, and the similarity module is a thoughtful addition. A few issues below are worth addressing before merge.

---

Bugs

1. DID / Rust struct mismatch in StoreResult

alex_backend.did declares:

type StoreResult = record { stored : nat64; skipped : nat64; errors : vec text };

But the Rust struct in similarity/updates.rs has no skipped field:

pub struct StoreResult {
    pub stored: u64,
    pub errors: Vec<String>,
}

Candid will silently zero-fill skipped on the wire, but any frontend code that reads skipped will always see 0. Either add the field to the struct or remove it from the DID.

---

2. cosine_similarity panics on mismatched slice lengths

for i in 0..a.len() {
    dot += a[i] * b[i];   // panics if b.len() < a.len()

The call sites validate dimensions before calling, but since the function is pub, a future caller (or the search_by_vector path under some edge case) could trigger an index-out-of-bounds panic. Prefer:

for (x, y) in a.iter().zip(b.iter()) { ... }

---

3. Comment.tsx timestamp unit change

The old code was:

convertTimestamp(Number(comment.created_at) / 1000000, 'relative')

The new code passes the raw value:

convertTimestamp(comment.created_at, 'relative')

IC nat64 timestamps are nanoseconds. If convertTimestamp expects milliseconds, every comment timestamp will now be ~1000× too large and display as "in the distant future". Confirm convertTimestamp handles nanoseconds natively, or restore the /1000000 division.

---

Performance

4. N+1 canister calls in AnalyticsPage

For each NFT, the analytics page fires 3 separate IC calls:

const [viewResult, impResult, reactionResult] = await Promise.allSettled([
    alexBackendActor.get_view_count(arweaveId),
    alexBackendActor.get_impressions(arweaveId),
    alexBackendActor.get_reaction_counts(arweaveId),
]);

A creator with 50 NFTs makes 150 canister calls. This will be noticeably slow and cycle-expensive. Consider adding a batch endpoint (e.g. get_engagement_stats_batch) that returns view + impression + reaction counts for a list of arweave IDs in a single call.

---

5. get_indexed_ids() has no limit

fn get_indexed_ids() -> Vec<String>

This returns every indexed arweave ID with no pagination. With hundreds of thousands of embeddings, the response will exceed IC's 2 MB reply limit and trap. Add a limit/cursor parameter, or make this a controller-only debug endpoint.

---

6. get_trending is O(n) on every call

get_trending iterates all entries in VIEWS, counts authenticated viewers per entry, and sorts. For a large dataset this consumes significant cycles per query call. Consider maintaining a pre-sorted or dirty-flagged cache that only rebuilds on write.

---

7. UserProfilePage — unbounded token fetch

const tokenIds = await icrc7.icrc7_tokens_of(account, [], []);

The empty [] means no pagination: all token IDs are fetched in one call. A wallet with thousands of NFTs will hit the response size limit. Pass a limit/offset or implement lazy loading.

---

Code Quality

8. Balance data silently truncated at 49 NFTs

const balResult = await nftManagerActor.get_nft_balances(tokenIds.slice(0, 49));

NFTs beyond index 49 will silently show 0 for ALEX/LBRY balances in the analytics table. Either paginate the balance fetch or show a UI note that balances are capped.

---

9. engagementService flush is only wired in AlexandrianPage

trackImpression is called from NFTCard and trackView from NFTModal, which are shared components used across multiple pages. But flushEngagement is only called from AlexandrianPage. Views and impressions tracked on the NFT detail page, user profile page, and any other page that renders these components will never be flushed. Consider triggering a flush on route change or adding a global flush mechanism (e.g., a React effect on the root layout).

---

10. Redux slice name not updated

perpetuaSlice still registers as "valora":

const perpetuaSlice = createSlice({
    name: "valora",   // should be "perpetua"
    ...
})

And rootReducer.ts still mounts it under the valora key. This is functional (the PerpetuaLayout correctly reads state.valora), but the slice name mismatches its file location and will confuse Redux DevTools. Rename both the slice name and the root reducer key as part of this rename.

---

11. Trending sort drops viewCount value

const trendingOrder = new Map(
    trendingEntries.map(([arweaveId, viewCount]: [string, bigint], index: number) => [arweaveId, index])
);

viewCount is destructured but unused (only index is used as the sort key). Either use _viewCount to signal intent, or—better—use the actual view count as the sort key directly instead of the backend-provided rank index, so ties in the backend are preserved rather than broken by array position.

---

12. UserProfilePage shelves silently capped at 20

{ offset: BigInt(0), limit: BigInt(20) }

With no "load more" button or count indicator, users with >20 shelves see a truncated list with no indication. Either add pagination or show a "+ N more" notice.

---

13. Static SEO title on profile page

<title>User Profile | Alexandria</title>

This is a missed opportunity — the dynamic username is available. Using @${username ?? shorten(principal, 6, 4)} | Alexandria would make shared links more descriptive.

---

Security

14. search_by_vector does not validate float values

NaN or Inf embeddings from a caller will produce NaN similarity scores. The partial_cmp fallback to Equal silently produces a meaningless sort. A controller-only write path is fine, but this is a query open to anyone. Adding a NaN/Inf check on the input vector would make the error explicit.

---

Minor

• FilterBar.tsx: The <Button> for semantic search is outdented one level relative to its sibling <div> — cosmetic, but worth fixing for diff readability.
• CommentUsername fires one query per visible comment. This is fine for small lists but will stack up. Since TanStack Query deduplicates by queryKey, repeated principals are already coalesced — just worth being aware of for large threads.
• The plan docs removal is clean housekeeping.

---

Summary

Severity	Count
Bug	3 (items 1, 2, 3)
Performance	4 (items 4, 5, 6, 7)
Quality / correctness	7 (items 8–14)

The rename and new pages are solid work. The similarity module is well-structured. The main things I'd want fixed before merge are the DID mismatch (#1), the timestamp regression (#3), the N+1 analytics fetch (#4), and the engagement flush gap (#9).

[github-actions]

Code Review — PR #141

Overview

This is a large, multi-concern PR landing several independent features:

1. valora -> perpetua rename — mostly mechanical file/route renames
2. Similarity module — new Rust backend for CLIP-based semantic search
3. Engagement service — impression/view batching to reduce canister calls
4. NFT details, user profile, and analytics pages — new public routes
5. Trending sort + SEO meta tags — incremental improvements to Alexandrian

The work quality is generally solid. There are some meaningful issues to address before merging.

---

Bugs

1. Redux slice name not updated after rename (perpetua/store/slice.ts)

The slice variable was renamed to perpetuaSlice but the internal name field still reads "valora". All dispatched actions will still appear as valora/setFeedType in Redux DevTools, and if the root reducer key is ever renamed the state will silently break.

2. AnalyticsPage balance index mismatch (AnalyticsPage.tsx)

For users with more than 49 NFTs, balances[index] is undefined for indices >= 49, silently showing 0 ALEX/LBRY. Either slice both arrays to the same length, or key balances by token ID.

3. Comment timestamp regression (Comment.tsx)

Before: convertTimestamp(Number(comment.created_at) / 1000000, 'relative')

After: convertTimestamp(comment.created_at, 'relative')

ICP timestamps are nanoseconds. If convertTimestamp expects milliseconds, dropping the division makes all timestamps appear tens of thousands of years in the future.

---

Performance Concerns

4. search_similar / search_by_vector — O(n) full scan (similarity/queries.rs)

Both search functions iterate the entire embedding store on every query call. At 512 dimensions x N embeddings, ICP's per-query instruction limit will be hit with only a few thousand entries. Consider adding a hard cap on embedded entries or documenting the scale limit in the DID until a proper ANN index is added.

5. get_trending — linear scan + full sort on every call (queries.rs)

Scans and sorts all view records with no caching. A pre-computed sorted index or a smaller enforced default limit (e.g. 50) would help.

6. get_indexed_ids — unbounded return (similarity/queries.rs)

Returns all indexed IDs with no limit. Add pagination or a hard cap to avoid exceeding ICP response size limits at scale.

7. AnalyticsPage — N+1 query pattern (AnalyticsPage.tsx)

For 50 NFTs, this fires ~150 canister calls simultaneously (3 per NFT). Consider a batched backend endpoint or limit the per-NFT table to top N by views.

---

Design Issues

8. Engagement flush only wired to AlexandrianPage unmount (engagementService.ts)

flushEngagement is only called in AlexandrianPage's unmount cleanup. Impressions tracked by Card.tsx and views tracked by Modal.tsx accumulate in module-level Sets but are never flushed if the user navigates away without visiting AlexandrianPage first. The flush should live in the app root or a route-change listener.

9. Semantic search has a centralized external dependency (AlexandrianPage.tsx)

Failures silently log to console with no user-facing feedback. At minimum, show a toast when the embedding server is unreachable.

10. createTokenFetcher uses unauthenticated actor for trending (createTokenFetcher.ts)

The imported alex_backend declaration is an anonymous actor. The rest of the codebase uses authenticated actors from hooks (useAlexBackend()). This works for a public query but is inconsistent with established patterns.

---

Minor Items

11. Anonymous view Vec grows unboundedly (dialectica/api/updates.rs)

Anonymous callers append None to the viewers Vec with no dedup. A single anonymous tab refreshing repeatedly will grow the Vec unboundedly for popular NFTs. Consider capping anonymous entries or counting them in a separate u64 counter instead of appending to the Vec.

12. clear_embeddings is inefficient (similarity/updates.rs)

Iterates and removes keys one by one. The existing reset_embeddings already does this efficiently by reinitializing the map. Consider having clear_embeddings reuse that logic.

13. icrc7_tokens_of is unbounded in UserProfilePage (UserProfilePage.tsx)

Empty prev and take params return all tokens. Add a take limit with pagination for users with large collections.

---

Positives

• The MEMORY_MANAGER consolidation (dialectica/store.rs, nft_users.rs) is a correct and important fix — multiple separate MemoryManager instances against the same stable memory would corrupt state.
• Controller guards on all embedding mutation endpoints are correct.
• The parsedPrincipal validation in UserProfilePage (wrapping Principal.fromText in try/catch) prevents a cascade of query errors on a malformed URL.
• The useSell approval check fix is cleaner and more correct.
• Comment edit/delete UX is a solid improvement.
• The arweave_id length check in record_engagement_batch is a good input guard.