refactor: remove BlockAssemblyDetails and improve UI components by ordishs · Pull Request #8 · bsv-blockchain/teranode

ordishs · 2025-10-15T14:56:42Z

Summary

Remove BlockAssemblyDetails from P2P node status notifications (fixes typecheck errors)
Add clear sort button to network table
Fix current node positioning in network table when sorting is active
Hide ancestors link from sidebar navigation (page remains accessible)
Add tooltip and copy functionality to invalidated blocks hash column in admin page

Backend Changes

services/p2p/Server.go: Removed BlockAssemblyDetails field from node status notifications and removed unused code
services/p2p/HandleWebsocket.go: Updated notification message handling

UI Changes

Network table: Added icon-only clear sort button that appears when sorting is active
Network table: Fixed logic to only place current node at top when no user sorting is applied
Sidebar: Commented out ancestors link while keeping the page accessible via direct URL
Admin page: Replaced custom hash rendering with reusable RenderHashWithMiner component for consistent tooltip and copy functionality

Test plan

Verify typecheck passes (make lint)
Verify network table displays current node at top by default
Verify clear sort button appears when sorting and removes sorting when clicked
Verify ancestors page is still accessible at /ancestors but not in sidebar
Verify hash column in admin invalidated blocks table shows tooltip and copy button
Verify hash is clickable and links to block viewer

…ions Replace detailed block assembly state with simpler subtree_count field in websocket notifications to reduce message size and coupling

Add icon button to clear sorting on network nodes table and ensure current node appears at top only when no user sorting is active

Keep ancestors page accessible via direct URL but remove from menu

Replace custom hash rendering with reusable RenderHashWithMiner component to provide consistent tooltip display and copy-to-clipboard functionality across the admin page's recently invalidated blocks table.

…l pointer panic Remove BlockAssemblyDetails field from NodeStatusMessage struct and eliminate attempts to access SubtreeCount/TxCount fields that caused segmentation fault in handleNodeStatusTopic. Also remove unused blockassembly_api import.

oskarszoon · 2025-10-16T10:44:33Z

/claude-review

claude · 2025-10-16T10:44:52Z

Claude finished @oskarszoon's task —— View job

Code Review Complete ✅

Summary

This PR successfully removes the BlockAssemblyDetails field from P2P node status notifications and adds several UI improvements to the network table and admin page. The changes are clean, well-structured, and improve code maintainability by removing unused functionality and consolidating UI components.

Backend Changes ✅

services/p2p/Server.go & HandleWebsocket.go

✅ Cleanly removed BlockAssemblyDetails field from NodeStatusMessage struct and notificationMsg struct
✅ Removed unused import blockassembly/blockassembly_api
✅ Removed code that fetched block assembly state in getNodeStatusMessage()
✅ Updated all struct instantiation sites consistently
✅ Replaced removed field with existing SubtreeCount field in notificationMsg (line 41 in HandleWebsocket.go)
✅ No breaking changes to JSON API since field had omitempty tag
✅ Code formatting and alignment improved

UI Changes ✅

Network Table (connected-nodes-card/index.svelte & network/+page.svelte)

✅ Added clear sort button that appears conditionally when sorting is active
✅ Button has proper hover/active states and accessibility (title attribute)
✅ Fixed logic to only position current node at top when no user sorting is applied (lines 253-256 in network/+page.svelte)
✅ Properly clears sort from localStorage when sort is cleared
✅ Good conditional rendering: {#if hasSorting}

Admin Page (admin/+page.svelte)

✅ Replaced custom hash rendering with reusable RenderHashWithMiner component
✅ Removed duplicate .copy-button CSS styles (cleaned up ~65 lines)
✅ Component usage is correct with proper props:
- hash, hashUrl, shortHash, showCopyButton, copyTooltip, tooltip
✅ Maintains same functionality (tooltip, copy, clickable link to block viewer)

Layout (+layout.svelte)

✅ Properly commented out ancestors navigation link
✅ Page remains accessible via direct URL as stated in PR description

Observations

Code Quality

Clean refactoring with no apparent logic errors
Consistent formatting and alignment
Good component reusability (RenderHashWithMiner)
Proper conditional rendering and state management

Potential Considerations (not blocking)

The SubtreeCount field added to notificationMsg struct (line 41 in HandleWebsocket.go) is not populated anywhere in the code I reviewed - it may need to be set in getNodeStatusMessage() if it's intended to be used
Consider adding a comment explaining why ancestors link is commented out rather than removed (for future maintainers)

Test Plan Coverage

The test plan covers all the changes appropriately:

✅ Typecheck verification
✅ Network table current node positioning
✅ Clear sort button functionality
✅ Ancestors page accessibility
✅ Admin page hash column functionality

⚠️ Reminder: Since this is a fork PR, I cannot run make lint or tests. Please ensure these pass in CI before merging.

Recommendation

Approve ✅ - The changes are well-implemented, maintain backward compatibility (thanks to omitempty), and improve code quality through better component reuse. The logic fixes for the network table are correct.

⚠️ Note: This is a fork PR - review based on static code analysis only. Cannot execute code or run tests for security reasons.

Branch: fix/remove-subtrees-from-nodeStatus

ordishs and others added 6 commits October 15, 2025 16:29

refactor(p2p): remove BlockAssemblyDetails from node status notificat…

1cf2c38

…ions Replace detailed block assembly state with simpler subtree_count field in websocket notifications to reduce message size and coupling

feat(ui): add clear sort button and fix current node positioning

afce252

Add icon button to clear sorting on network nodes table and ensure current node appears at top only when no user sorting is active

refactor(ui): hide ancestors link from sidebar navigation

4837f16

Keep ancestors page accessible via direct URL but remove from menu

feat(ui): add tooltip and copy functionality to invalidated blocks hash

3a5ea00

Replace custom hash rendering with reusable RenderHashWithMiner component to provide consistent tooltip display and copy-to-clipboard functionality across the admin page's recently invalidated blocks table.

Merge branch 'main' into fix/remove-subtrees-from-nodeStatus

57568c4

oskarszoon approved these changes Oct 16, 2025

View reviewed changes

oskarszoon merged commit 00331b5 into bsv-blockchain:main Oct 16, 2025
5 checks passed

oskarszoon assigned ordishs Oct 16, 2025

ordishs mentioned this pull request Oct 17, 2025

feat(p2p): add tx_count and subtree_count to node_status message #24

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: remove BlockAssemblyDetails and improve UI components#8

refactor: remove BlockAssemblyDetails and improve UI components#8
oskarszoon merged 6 commits into
bsv-blockchain:mainfrom
ordishs:fix/remove-subtrees-from-nodeStatus

ordishs commented Oct 15, 2025

Uh oh!

oskarszoon commented Oct 16, 2025

Uh oh!

claude Bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ordishs commented Oct 15, 2025

Summary

Backend Changes

UI Changes

Test plan

Uh oh!

oskarszoon commented Oct 16, 2025

Uh oh!

claude Bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Complete ✅

Backend Changes ✅

UI Changes ✅

Observations

Test Plan Coverage

Recommendation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Oct 16, 2025 •

edited

Loading