Show full-depth reference in CharmDetails panel #931
Merged
Conversation
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
…yCharms - Fixed infinite recursion by replacing deep traversal with top-level property scanning - Added proper entity ID tracking to avoid duplicate references - Removed dependency on findAllAliasedDocs which was causing runtime issues - Fixed type issues in tests and cleaned up debug logging - Both reference directions now correctly show all references between charms Fixes #758 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add proper error handling by rethrowing instead of silently failing - Replace comments with TODO items for future recursive implementation - Remove findAllAliasedDocs export that isn't being used - Fix type checking errors in tests 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Based on PR review feedback in #909 (comment) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>\
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>\
…ndling - Added recursive traversal of sourceCell chains to find resultRefs - Implemented safe traversal to avoid triggering Copy trap errors - Fixed cycle detection by tracking object references directly - Added improved error handling to gracefully handle issues without crashing - Added safeguards to prevent array access issues and property access errors - Limited the maximum number of references to avoid UI overload - Thoroughly tested with nested reference chains Fixes #758
- Refactored to use followAliases and isAlias for better alias handling - Added type checking and proper error boundaries - Used existing utility functions instead of reimplementing them - Reduced code verbosity and improved readability - Safely handles Cell objects to avoid Copy trap errors Continues work on #758
dcce75e to
42d4daa
Compare
bfollington
commented
Mar 31, 2025
Comment on lines
+557
to
+599
| if (Array.isArray(value)) { | ||
| for (let i = 0; i < value.length; i++) { | ||
| // Skip null/undefined items | ||
| if (value[i] == null) continue; | ||
|
|
||
| // Skip items that might be cells to avoid Copy trap | ||
| if (typeof value[i] === 'object' && | ||
| (isCell(value[i]) || isDoc(value[i]) || isCellLink(value[i]))) { | ||
| try { | ||
| // Process each cell directly | ||
| processValue(value[i], parent, new Set([...visited]), depth + 1); | ||
| } catch (err) { | ||
| console.debug(`Error processing special array item at index ${i}:`, err); | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| // Process regular items | ||
| try { | ||
| processValue(value[i], parent, new Set([...visited]), depth + 1); | ||
| } catch (err) { | ||
| console.debug(`Error processing array item at index ${i}:`, err); | ||
| } | ||
| } | ||
| } else if (typeof value === 'object') { | ||
| // Process regular object properties | ||
| const keys = Object.keys(value); | ||
| for (let i = 0; i < keys.length; i++) { | ||
| const key = keys[i]; | ||
|
|
||
| // Skip properties that might be or contain Cell objects | ||
| if (key === 'sourceCell' || key === 'cell' || key === 'value' || | ||
| key === 'getAsCellLink' || key === 'getSourceCell') { | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| processValue(value[key], parent, new Set([...visited]), depth + 1); | ||
| } catch (err) { | ||
| console.debug(`Error processing object property '${key}':`, err); | ||
| } | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
I don't really understand how or why, but I was triggering the copy trap by naively traversing.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR aims to enhance the CharmDetails panel by showing full-depth references and improving the UI for reference sections. The changes include updating the runner export to include followAliases, refactoring the combined references section in the CharmDetailView, and adding a new test for n-depth reference detection.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| runner/src/index.ts | Added re-export for followAliases to support full-depth reference detection. |
| jumble/src/views/CharmDetailView.tsx | Refactored reference sections by combining "Reading From" and "Read By" into a unified UI and renaming/toggling states. |
| charm/test/charm-references.test.ts | Introduced a test to verify chained reference detection through sourceCell and resultRef. |
Comments suppressed due to low confidence (2)
jumble/src/views/CharmDetailView.tsx:1190
- [nitpick] Consider renaming 'isReferencesExpanded' to a more descriptive state variable that clearly indicates it controls the unified references section, to avoid confusion with the legacy 'Lineage' toggle.
onClick={() => setIsReferencesExpanded(!isReferencesExpanded)}
jumble/src/views/CharmDetailView.tsx:1272
- [nitpick] The use of 'isLineageExpanded' here is inconsistent with the revised combined references approach; consider unifying the toggle logic with 'isReferencesExpanded' to reduce ambiguity in the UI.
onClick={() => setIsLineageExpanded(!isLineageExpanded)}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #910 #912