feat: Implement Plan 006-3d - Entity Disambiguation

[simonas-notcat]

Summary

Implements Plan 006-3d: Entity Disambiguation with search badges and selected atom badges to help users distinguish between ambiguous entities like "Apple" (company vs. fruit) or "Jordan" (person vs. country).

Changes

Data Model

• Extended AtomReference interface with entity disambiguation fields:
  • entityType: Type of entity (person, organization, concept, etc.)
  • disambiguation: Human-readable clarification text
  • entityConfidence: 0-1 confidence score

AtomSearchInput Component

• Added entityHint state to store LLM-provided entity suggestions
• Implemented setEntityHint() method for setting entity metadata
• Enhanced search result rendering:
  • "🤖 AI suggests" badge for LLM-recommended entities
  • Entity type badges showing entity classification
  • Disambiguation text from LLM hints
  • Visual highlighting for LLM suggestions
• Updated selection callbacks to include entity metadata

ClaimModal Component

• Added entity badge elements for subject, predicate, and object fields
• Implemented updateEntityBadge() method with confidence-based color coding:
  • Green (≥80%): High confidence
  • Yellow (≥50%): Medium confidence
  • Red (<50%): Low confidence
• Modified setEntityHintsFromLLM() to automatically set hints from LLM metadata
• Updated field label rows to include badge display area

Styling

• Added CSS for LLM suggestion badges in search results
• Entity type badge styling
• Entity disambiguation text styling
• Confidence-based color coding for selected atom badges
• Smooth fade-in animations for badges

Testing

• Added comprehensive unit tests for entity disambiguation
• Tests cover hint setting, selection with metadata, and behavior without hints
• All 245 tests passing

User Experience

Before

User types: "Apple"
Search results:
  - Apple Inc. (0x1234...abcd)
  - Apple (0x5678...efgh)
  - Apple Computer (0x9abc...def0)

User thinks: "Which one? 🤔"
User guesses → 33% chance of wrong selection

After

User types: "Apple"
Search results:
  - 🤖 AI suggests: Apple Inc. (Technology company)
  - Apple Inc. (Organization) ✓
  - Apple (Fruit)
  - Apple Computer (Historical name)

User sees badge after selection:
  Subject: [Apple Inc. ▼]  Organization 95%

User thinks: "Ah, the company. Correct!" ✓

Testing

• ✅ All 245 unit tests passing
• ✅ TypeScript type checking successful
• ✅ Production build successful
• ✅ Entity hints properly set from LLM metadata
• ✅ Badges display with correct confidence colors
• ✅ Search results show LLM suggestions prominently

Plan Compliance

Implements all acceptance criteria from Plan 006-3d:

• ✅ LLM suggestion badge in search results
• ✅ Entity type shown for all results
• ✅ Disambiguation text in search dropdown
• ✅ Entity type badge after selection
• ✅ Confidence-based color coding
• ✅ Tooltip on hover with disambiguation details

🤖 Generated with Claude Code

[claude]

Code Review: Plan 006-3d - Entity Disambiguation

Overall Assessment

✅ LGTM with minor suggestions - This is a well-implemented feature that follows the project's architecture patterns and coding standards. The implementation aligns closely with the detailed plan specification.

---

Strengths

1. Excellent Plan Adherence

• All acceptance criteria from Plan 006-3d are met
• Implementation follows the recommended "Option C (Hybrid)" approach
• Proper separation of concerns between search UI and entity metadata

2. Clean Type Extensions

// src/types/search.ts - Clean, well-documented extension
entityType?: 'person' | 'organization' | 'concept' | 'thing' | 'place' | 'event' | 'unknown';
disambiguation?: string;
entityConfidence?: number;

The type union for entityType is comprehensive and covers common use cases.

3. Comprehensive Test Coverage

• 117 new test lines added for entity disambiguation
• Tests cover: hint setting, selection with metadata, and behavior without hints
• Good edge case coverage (existing atoms, new atoms, missing hints)
• All 245 tests passing ✅

4. User Experience Design

• Clear visual hierarchy: "🤖 AI suggests" badge stands out
• Confidence-based color coding is intuitive (green ≥80%, yellow ≥50%, red <50%)
• Smooth animations with opacity transitions
• Tooltips provide additional context without cluttering UI

5. CSS Implementation

• Uses CSS custom properties for theme compatibility
• Follows Obsidian's design system (--background-modifier-*, --text-*)
• Accessibility-friendly with ARIA labels and semantic HTML

---

Issues & Suggestions

🔴 Critical: Type Safety Issue

Location: src/ui/components/atom-search-input.ts:238, 250, 346, 527, 537

entityType: this.entityHint?.type as any,  // ⚠️ Uses 'as any'

Problem: The entityHint.type is typed as string | undefined, but AtomReference.entityType expects a strict union type. Using as any bypasses type checking and could allow invalid values.

Fix:

// Add validation helper
private normalizeEntityType(type?: string): AtomReference['entityType'] {
    const validTypes = ['person', 'organization', 'concept', 'thing', 'place', 'event', 'unknown'];
    if (type && validTypes.includes(type)) {
        return type as AtomReference['entityType'];
    }
    return undefined;
}

// Use in selections
entityType: this.normalizeEntityType(this.entityHint?.type),

Or update entityHint to use the proper type:

private entityHint: {
    type?: AtomReference['entityType'];  // ✅ Reuse the union type
    disambiguation?: string;
    confidence?: number;
} | null = null;

---

🟡 Medium: Potential Logic Issue

Location: src/ui/components/atom-search-input.ts:545-548

private isLLMSuggestion(atom: AtomData): boolean {
    if (!this.entityHint) return false;
    
    const labelMatch = atom.label.toLowerCase() === this.inputEl.value.toLowerCase();
    const typeMatch = !!(this.entityHint.type && atom.type === this.entityHint.type);
    
    return labelMatch && typeMatch;  // ⚠️ Requires BOTH matches
}

Problem:

1. If atom.type is undefined, typeMatch will always be false
2. This means valid LLM suggestions might not be highlighted if GraphQL doesn't return type metadata
3. The !! operator doesn't prevent the match from failing if atom.type is undefined

Suggested Fix:

private isLLMSuggestion(atom: AtomData): boolean {
    if (!this.entityHint) return false;
    
    const labelMatch = atom.label.toLowerCase() === this.inputEl.value.toLowerCase();
    
    // If no entity type hint, match by label only
    if (!this.entityHint.type) return labelMatch;
    
    // If type hint exists, prefer exact type match but fallback to label match
    const typeMatch = atom.type?.toLowerCase() === this.entityHint.type.toLowerCase();
    return labelMatch && (typeMatch || !atom.type);  // Allow match if atom has no type
}

---

🟡 Medium: Missing EntityHint Reset

Problem: entityHint is set via setEntityHint() but never cleared. If a user:

1. Opens modal with LLM suggestion (hint set)
2. Closes modal
3. Opens modal again manually (no LLM)

The old entityHint will still be active and might show stale suggestions.

Fix: Add cleanup in modal close or search component reset.

---

🟢 Minor: Code Duplication

Location: src/ui/components/atom-search-input.ts:573, claim-modal.ts:1120

Both files implement identical capitalizeFirst() utility. Consider extracting to shared utils.

---

Performance

✅ No concerns - Changes are UI-only with minimal computational overhead

---

Security

✅ No concerns - All data flows are internal, no user-provided HTML injection

---

Recommendation

APPROVE with suggested fixes for:

1. 🔴 Remove as any type assertions (critical for type safety)
2. 🟡 Fix isLLMSuggestion() logic for atoms without type
3. 🟡 Add entity hint cleanup/reset

Optional improvements:

• Extract capitalizeFirst() to shared utils
• Add test cases for edge cases

---

Summary Stats

• Files changed: 5
• Lines added: 403
• Lines removed: 13
• Test coverage: ✅ Comprehensive (117 new test lines)
• Type safety: ⚠️ Needs fix (remove as any)
• Plan compliance: ✅ 100%

Great work on a feature that significantly improves UX! 🎉

[claude]

Pull Request Review: Entity Disambiguation (Plan 006-3d)

Summary

This PR successfully implements entity disambiguation for the Intuition Obsidian plugin, adding AI-powered entity type detection and visual badges to help users distinguish between ambiguous entities. The implementation follows the detailed plan specification and maintains the project's high code quality standards.

✅ Strengths

1. Excellent Plan Adherence

• All acceptance criteria from Plan 006-3d are met
• Implementation closely follows the hybrid approach (Option C) outlined in the plan
• User experience matches the documented "Before/After" scenarios

2. Clean Code Quality

• Type Safety: Properly typed entity hint state and AtomReference extensions
• Shared Utilities: Excellent refactoring of capitalizeFirst() to utils/helpers.ts with comprehensive JSDoc
• Defensive Programming: Proper null checks and fallback handling throughout
• No Type Assertions: The second commit successfully removed as any type assertions

3. Comprehensive Testing

• 245 tests passing (all unit tests)
• New tests cover key scenarios:
  • Entity hints with existing atoms
  • Entity hints for new atoms
  • Graceful degradation without hints
• Tests verify data flow from hint → selection → callback

4. Thoughtful UX Design

• Progressive Enhancement: Works with and without LLM hints
• Visual Hierarchy: AI suggestions clearly highlighted with 🤖 badge and accent border
• Confidence Indicators: Color-coded badges (green ≥80%, yellow ≥50%, red <50%)
• Tooltips: Disambiguation text accessible on hover
• Smooth Animations: Fade-in transitions for badges

5. Security Conscious

• Uses aria-label and title attributes for accessibility
• No XSS risks in entity type rendering
• Input validation preserved

---

🔍 Code Quality Observations

atom-search-input.ts

Excellent:

• isLLMSuggestion() handles edge cases well (lines 574-586):
  • Gracefully handles missing entity type hints
  • Falls back to label matching when atom has no type
  • Case-insensitive comparison

Good:

• Entity hint cleanup in clear() method (line 494) prevents state leakage
• Entity metadata properly propagated in all selection paths

claim-modal.ts

Excellent:

• Badge color-coding logic is clear and maintainable (lines 1087-1098)
• Proper cleanup of previous confidence classes before applying new ones
• setEntityHintsFromLLM() integration (lines 941-961) follows single responsibility principle

Minor Note:

• The getBadgeElement() method (lines 1110-1115) could potentially use a map for scalability, but current implementation is perfectly fine for 3 fields

helpers.ts

Excellent:

• capitalizeFirst() has comprehensive JSDoc with examples
• Edge case handling (empty string check)
• Pure function with no side effects

---

🐛 Potential Issues

1. Minor: Entity Type Comparison Case Sensitivity

In atom-search-input.ts:584:

const typeMatch = atom.type?.toLowerCase() === this.entityHint.type.toLowerCase();

Issue: this.entityHint.type is typed as AtomReference['entityType'] (union type), but might be undefined. The ?.toLowerCase() would fail if this.entityHint.type is undefined.

Fix: Add optional chaining:

const typeMatch = atom.type?.toLowerCase() === this.entityHint.type?.toLowerCase();

Impact: Low - line 581 already checks if (!this.entityHint.type), so this code path won't execute with undefined type. However, explicit safety is better.

2. Missing: Badge Hover State Indicator

The plan mentions "Badge clickable to show full entity details (optional tooltip)" (line 68 of plan), but the current implementation only uses title attribute.

Suggestion: Consider adding a visual hint that badges are interactive (if they are meant to be):

.entity-badge[title] {
    cursor: help;
}

Impact: Very low - this is a "nice to have" from the plan.

---

🎯 Performance Considerations

Positive:

• No additional API calls or LLM invocations (uses cached metadata)
• Badge rendering is lazy (only on selection)
• Entity hints are local state (no persistence overhead)

Potential Optimization:

• Entity type capitalization happens on every render. Consider memoizing if performance becomes an issue (unlikely with current result counts).

---

🧪 Test Coverage

Strengths:

• Unit tests cover the happy path and edge cases
• Tests verify entity metadata propagation through the selection callback
• Proper mocking of IntuitionService

Suggestions for Enhancement:

1. Integration Tests: Consider testing LLM metadata flow end-to-end:

   • ClaimModal.autoExtract() → setEntityHintsFromLLM() → AtomSearchInput.setEntityHint() → selection

2. Visual Regression Tests: Badge color-coding and CSS styles could benefit from snapshot tests (not critical for this PR)

3. Edge Case: Test behavior when entityHint.confidence is 0 or negative (should be caught by validation, but worth testing)

---

🔐 Security Review

✅ No Security Concerns Identified

• Entity types are from controlled enums
• Disambiguation text is escaped by Obsidian's DOM methods
• No direct HTML injection
• No sensitive data in entity hints

---

📚 Documentation

Excellent:

• Commit messages follow project conventions
• JSDoc comments added for new public methods
• Code is self-documenting with clear variable names

Minor Suggestion:

• Consider adding a comment in claim-modal.ts explaining why predicates don't get entity hints in setEntityHintsFromLLM() (line 957-958)

---

🎨 UI/UX Review

Visual Design:

• Badge positioning in field-label-row uses flexbox with space-between ✅
• Confidence colors use Obsidian CSS variables for theme compatibility ✅
• LLM suggestion badge uses accent color for visibility ✅

Accessibility:

• aria-label attributes present ✅
• Title tooltips for disambiguation text ✅
• Color is not the only indicator (text + percentage shown) ✅

Suggestion:

• Consider adding role="tooltip" to badges for better screen reader support

---

📋 Plan Compliance Checklist

Reviewing against Plan 006-3d acceptance criteria:

Criterion	Status	Notes
✅ LLM "AI suggests" badge in search	✅ Pass	Lines 300-308 in atom-search-input.ts
✅ Entity type shown for all results	✅ Pass	Lines 315-321
✅ Disambiguation text in dropdown	✅ Pass	Lines 325-331
✅ Entity type badge after selection	✅ Pass	updateEntityBadge() implementation
✅ Confidence-based color coding	✅ Pass	Lines 1087-1098 in claim-modal.ts
✅ Tooltip on hover	✅ Pass	Lines 1101-1104

---

🚀 Recommendations

Must Address:

None - this is production-ready code.

Nice to Have:

1. Add optional chaining in atom-search-input.ts:584 for defensive safety
2. Add cursor: help CSS for badges with tooltips
3. Add comment explaining why predicates lack entity hints

Future Enhancements (Post-PR):

1. Consider lazy-loading disambiguation for manual searches (Plan 006-3d Option C phase 3)
2. Track user selection patterns to improve entity suggestions over time
3. Add "Did you mean?" warnings for low-confidence selections

---

📊 Overall Assessment

Score: 9.5/10

This is excellent work that:

• ✅ Implements all plan requirements
• ✅ Maintains high code quality
• ✅ Provides comprehensive test coverage
• ✅ Follows project conventions
• ✅ Introduces no regressions
• ✅ Enhances user experience significantly

The entity disambiguation feature will meaningfully improve claim quality by reducing ambiguous entity selection errors. The implementation is clean, maintainable, and well-tested.

---

✅ Approval Recommendation

APPROVE - Ready to merge after minor considerations:

1. (Optional) Add optional chaining in type comparison for extra safety
2. (Optional) Add cursor style for tooltips
3. All tests passing (245/245)
4. TypeScript compilation successful
5. No breaking changes

Great work on this feature! The attention to UX details (color coding, AI suggestions, smooth animations) really elevates the user experience. 🎉

---

Review conducted following CLAUDE.md guidelines. Focused on: code quality, TypeScript best practices, service architecture patterns, test coverage, security, and plan compliance.