Skip to content

Commit 94dbaab

Browse files
ozgesolidkeyclaude
andcommitted
Robust navigation redesign: single navigateTo(), fix off-by-1, fix blank flash
navigateTo(absLine): - Single canonical async navigation function replacing scattered goToLine calls - Pre-sets visibleStartLine/endLine, pre-warms line cache (loadVisibleLines) before setting scrollTop — eliminates blank-frame flash on cold jumps - Handles filtered views, proportional scroll for out-of-filter lines - goToLine() kept as legacy sync thin wrapper for internal scroll-event paths Off-by-1 fixes (analysis/time-gaps showed 0-based numbers, off by 1 from display): - Crash items: display c.lineNumber+1 in text and title - Time gaps: display gap.lineNumber+1 - All click guards: if (line > 0) → if (line >= 0) so line 0 is navigable Line number layer cleanup: - HTTP API (/api/navigate, /api/annotate) reverted to 0-based (internal format) - MCP tool layer (logan_navigate, logan_annotate, logan_report_finding) now converts 1-based viewerLine inputs to 0-based before calling the API - Auto-annotate code in triage/investigate tools passes 0-based directly (correct) All primary navigation entry points (search results, crashes, components, bookmarks, time-gaps, annotations) now go through navigateTo(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6716aae commit 94dbaab

3 files changed

Lines changed: 99 additions & 65 deletions

File tree

src/main/api-server.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -519,13 +519,10 @@ export function startApiServer(ctx: ApiContext): void {
519519
// --- Agent Annotations ---
520520
if (url === '/api/annotate') {
521521
if (body.lineNumber === undefined || !body.text) return sendError(res, 'lineNumber and text required');
522-
// Accept 1-based viewerLine numbers (as displayed in LOGAN) — convert to 0-based internally
523-
const ln0 = Math.max(0, body.lineNumber - 1);
524-
const el0 = body.endLine !== undefined ? Math.max(0, body.endLine - 1) : undefined;
525522
const annotation: Annotation = {
526523
id: `ann-${Date.now()}-${Math.random().toString(36).substring(2, 7)}`,
527-
lineNumber: ln0,
528-
...(el0 !== undefined ? { endLine: el0 } : {}),
524+
lineNumber: body.lineNumber,
525+
...(body.endLine !== undefined ? { endLine: body.endLine } : {}),
529526
text: body.text,
530527
agentName: body.agentName || activeAgent?.name || 'Agent',
531528
timestamp: Date.now(),
@@ -580,9 +577,7 @@ export function startApiServer(ctx: ApiContext): void {
580577
}
581578

582579
if (url === '/api/navigate') {
583-
// Accept 1-based viewerLine — convert to 0-based for internal use
584-
const lineNumber = Math.max(0, (body.lineNumber ?? 1) - 1);
585-
ctx.navigateToLine(lineNumber);
580+
ctx.navigateToLine(body.lineNumber ?? 0);
586581
sendJson(res, { success: true });
587582
return;
588583
}

src/mcp-server/index.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,8 @@ server.tool(
405405
},
406406
async ({ lineNumber }) => {
407407
try {
408-
const result = await apiCall('POST', '/api/navigate', { lineNumber });
408+
// Convert 1-based viewer line to 0-based internal line number
409+
const result = await apiCall('POST', '/api/navigate', { lineNumber: Math.max(0, lineNumber - 1) });
409410
return { content: [{ type: 'text', text: JSON.stringify(result, null, 2) }] };
410411
} catch (err: any) {
411412
return { content: [{ type: 'text', text: `Error: ${err.message}` }], isError: true };
@@ -1020,9 +1021,10 @@ server.tool(
10201021
},
10211022
async (params) => {
10221023
try {
1024+
// Convert 1-based viewer lines to 0-based internal line numbers
10231025
const result = await apiCall('POST', '/api/annotate', {
1024-
lineNumber: params.lineNumber,
1025-
...(params.endLine !== undefined ? { endLine: params.endLine } : {}),
1026+
lineNumber: Math.max(0, params.lineNumber - 1),
1027+
...(params.endLine !== undefined ? { endLine: Math.max(0, params.endLine - 1) } : {}),
10261028
text: params.text,
10271029
severity: params.severity || 'info',
10281030
});
@@ -1160,20 +1162,24 @@ server.tool(
11601162
await apiCall('POST', '/api/annotation-clear').catch(() => null);
11611163
}
11621164

1165+
// Convert 1-based viewer lines to 0-based internal line numbers
1166+
const line0 = Math.max(0, lineNumber - 1);
1167+
const end0 = endLine !== undefined ? Math.max(0, endLine - 1) : undefined;
1168+
11631169
// 1. Annotate
1164-
const annBody: Record<string, any> = { lineNumber, text: title, severity };
1165-
if (endLine !== undefined) annBody.endLine = endLine;
1170+
const annBody: Record<string, any> = { lineNumber: line0, text: title, severity };
1171+
if (end0 !== undefined) annBody.endLine = end0;
11661172
results.annotation = await apiCall('POST', '/api/annotate', annBody);
11671173

11681174
// 2. Navigate viewer to the line
11691175
if (navigate) {
1170-
results.navigate = await apiCall('POST', '/api/navigate', { lineNumber }).catch(() => null);
1176+
results.navigate = await apiCall('POST', '/api/navigate', { lineNumber: line0 }).catch(() => null);
11711177
}
11721178

1173-
// 3. Send chat message with full detail
1179+
// 3. Send chat message with full detail (keep 1-based for user display)
11741180
const lineLabel = endLine !== undefined && endLine > lineNumber
1175-
? `Lines ${lineNumber + 1}${endLine + 1}`
1176-
: `Line ${lineNumber + 1}`;
1181+
? `Lines ${lineNumber}${endLine}`
1182+
: `Line ${lineNumber}`;
11771183
const chatText = `**[${severity.toUpperCase()}] ${title}** (${lineLabel})\n\n${detail}`;
11781184
results.message = await apiCall('POST', '/api/agent-message', { message: chatText });
11791185

src/renderer/renderer.ts

Lines changed: 81 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -5644,6 +5644,7 @@ function updateTracebackUI(): void {
56445644
// ─── Agent Chat ──────────────────────────────────────────────────────
56455645

56465646
function addChatMessage(msg: { id?: string; from: string; text: string; timestamp: number }): void {
5647+
if (msg.from === 'agent') hideTypingIndicator();
56475648
const el = document.createElement('div');
56485649
el.className = `chat-message from-${msg.from}`;
56495650
if (msg.id) el.dataset.msgId = msg.id;
@@ -5670,6 +5671,28 @@ function addChatMessage(msg: { id?: string; from: string; text: string; timestam
56705671
elements.chatMessages.scrollTop = elements.chatMessages.scrollHeight;
56715672
}
56725673

5674+
let chatTypingIndicator: HTMLElement | null = null;
5675+
5676+
function showTypingIndicator(): void {
5677+
if (chatTypingIndicator) return;
5678+
const el = document.createElement('div');
5679+
el.className = 'chat-typing-indicator';
5680+
for (let i = 0; i < 3; i++) {
5681+
const dot = document.createElement('div');
5682+
dot.className = 'chat-typing-dot';
5683+
el.appendChild(dot);
5684+
}
5685+
chatTypingIndicator = el;
5686+
elements.chatMessages.appendChild(el);
5687+
elements.chatMessages.scrollTop = elements.chatMessages.scrollHeight;
5688+
}
5689+
5690+
function hideTypingIndicator(): void {
5691+
if (!chatTypingIndicator) return;
5692+
chatTypingIndicator.remove();
5693+
chatTypingIndicator = null;
5694+
}
5695+
56735696
function sendChatMessage(): void {
56745697
const text = elements.chatInput.value.trim();
56755698
if (!text) return;
@@ -5678,6 +5701,7 @@ function sendChatMessage(): void {
56785701
const msg = { from: 'user', text, timestamp: Date.now() };
56795702
addChatMessage(msg);
56805703
elements.chatInput.value = '';
5704+
showTypingIndicator();
56815705

56825706
// Send to main process
56835707
window.api.sendAgentMessage(text).catch(() => {
@@ -6162,28 +6186,7 @@ function rebuildAnnotationIndex(): void {
61626186
}
61636187

61646188
function navigateToAnnotation(ann: any): void {
6165-
if (!logViewerElement) return;
6166-
const lineNum = ann.lineNumber;
6167-
const lineH = getLineHeight();
6168-
6169-
if (!state.isFiltered || !state.filteredLineNumbers) {
6170-
// No filter: scroll directly to the line's pixel position
6171-
logViewerElement.scrollTop = Math.max(0, lineNum * lineH);
6172-
} else {
6173-
const di = getFilteredDisplayIndex(lineNum);
6174-
if (di >= 0) {
6175-
logViewerElement.scrollTop = Math.max(0, di * lineH);
6176-
} else {
6177-
// Line filtered out — proportional position using raw total
6178-
const maxScroll = logViewerElement.scrollHeight - logViewerElement.clientHeight;
6179-
logViewerElement.scrollTop = Math.max(0, (lineNum / state.totalLines) * maxScroll);
6180-
}
6181-
}
6182-
6183-
state.selectedLine = lineNum;
6184-
updateCursorStatus(lineNum);
6185-
requestAnimationFrame(() => { renderVisibleLines(); });
6186-
setActiveAnnotation(ann.id);
6189+
navigateTo(ann.lineNumber).then(() => setActiveAnnotation(ann.id));
61876190
}
61886191

61896192
let activeAnnotationId: string | null = null;
@@ -9938,28 +9941,51 @@ function navigateSearchPrev(): void {
99389941

99399942
function goToSearchResult(index: number): void {
99409943
if (index < 0 || index >= state.searchResults.length) return;
9941-
99429944
state.currentSearchIndex = index;
99439945
const result = state.searchResults[index];
9944-
const di = getFilteredDisplayIndex(result.lineNumber);
9945-
const scrollTarget = di >= 0 ? di : result.lineNumber;
9946-
goToLine(scrollTarget, result.lineNumber);
9947-
updateSearchUI();
9948-
renderVisibleLines();
9949-
updateSearchResultsCurrent();
9950-
scrollSearchResultIntoView();
9946+
navigateTo(result.lineNumber).then(() => {
9947+
updateSearchUI();
9948+
updateSearchResultsCurrent();
9949+
scrollSearchResultIntoView();
9950+
});
99519951
}
99529952

9953-
// Navigate to a line. displayIndex is the position in the current view (filtered or not).
9954-
// originalLineNumber overrides which line gets highlighted/selected — use when the
9955-
// display index differs from the real line number (e.g. filtered search results).
9956-
function goToLine(displayIndex: number, originalLineNumber?: number): void {
9953+
// Canonical navigation entry point. Takes an absolute 0-based line number.
9954+
// Pre-warms the line cache so the render shows content immediately (no blank flash).
9955+
async function navigateTo(absLine: number): Promise<void> {
99579956
if (!logViewerElement) return;
9957+
const total = getTotalLines();
9958+
if (total === 0) return;
9959+
absLine = Math.max(0, Math.min(absLine, total - 1));
9960+
9961+
const di = getFilteredDisplayIndex(absLine);
9962+
const displayLine = di >= 0 ? di : absLine;
9963+
9964+
state.selectedLine = absLine;
9965+
9966+
// Pre-set the visible range so loadVisibleLines fetches the right lines
9967+
const visCount = Math.ceil(logViewerElement.clientHeight / getLineHeight());
9968+
state.visibleStartLine = Math.max(0, displayLine - BUFFER_LINES);
9969+
state.visibleEndLine = Math.min(total - 1, displayLine + visCount + BUFFER_LINES);
9970+
9971+
// Cancel any pending RAF (handleScroll will schedule one; we do it now synchronously)
9972+
if (scrollRAF !== null) { cancelAnimationFrame(scrollRAF); scrollRAF = null; }
99589973

9974+
// Pre-warm cache — eliminates the blank-frame flash on cold jumps
9975+
await loadVisibleLines();
9976+
9977+
// Now set scroll: handleScroll fires, updates state (same range), renders with warm cache
9978+
logViewerElement.scrollTop = Math.max(0, lineToScrollTop(displayLine));
9979+
updateCursorStatus(absLine);
9980+
}
9981+
9982+
// Legacy thin wrapper — kept for callers that cannot be async.
9983+
// Use navigateTo() for all new navigation code.
9984+
function goToLine(displayIndex: number, originalLineNumber?: number): void {
9985+
if (!logViewerElement) return;
99599986
const lineNumber = originalLineNumber ?? displayIndex;
99609987
state.selectedLine = lineNumber;
9961-
const targetScrollTop = lineToScrollTop(displayIndex);
9962-
logViewerElement.scrollTop = Math.max(0, targetScrollTop);
9988+
logViewerElement.scrollTop = Math.max(0, lineToScrollTop(displayIndex));
99639989
updateCursorStatus(lineNumber);
99649990
}
99659991

@@ -10663,7 +10689,7 @@ function renderTimeGaps(gaps: TimeGap[], options?: TimeGapOptions): void {
1066310689
<div class="time-gap-item" data-line="${gap.lineNumber}" data-index="${index}">
1066410690
<div class="gap-header">
1066510691
<span class="gap-duration">${formatDuration(gap.gapSeconds)}</span>
10666-
<span class="gap-line">Line ${gap.lineNumber}</span>
10692+
<span class="gap-line">Line ${gap.lineNumber + 1}</span>
1066710693
</div>
1066810694
<div class="gap-times">${gap.prevTimestamp} → ${gap.currTimestamp}</div>
1066910695
<div class="gap-preview" title="${escapeHtml(gap.linePreview)}">${escapeHtml(gap.linePreview)}</div>
@@ -10676,12 +10702,12 @@ function renderTimeGaps(gaps: TimeGap[], options?: TimeGapOptions): void {
1067610702
elements.timeGapsList.querySelectorAll('.time-gap-item').forEach(item => {
1067710703
item.addEventListener('click', () => {
1067810704
const index = parseInt((item as HTMLElement).dataset.index || '0');
10679-
const lineNumber = parseInt((item as HTMLElement).dataset.line || '0');
10680-
if (lineNumber > 0) {
10705+
const lineNumber = parseInt((item as HTMLElement).dataset.line || '-1');
10706+
if (lineNumber >= 0) {
1068110707
currentGapIndex = index;
1068210708
updateGapNavPosition();
1068310709
highlightCurrentGapItem();
10684-
goToLine(lineNumber);
10710+
navigateTo(lineNumber);
1068510711
}
1068610712
});
1068710713
});
@@ -11012,8 +11038,8 @@ function updateBookmarksUI(): void {
1101211038
const id = target.dataset.id!;
1101311039
editBookmarkComment(id);
1101411040
} else {
11015-
const line = parseInt((item as HTMLElement).dataset.line || '0', 10);
11016-
goToLine(line);
11041+
const line = parseInt((item as HTMLElement).dataset.line || '-1', 10);
11042+
if (line >= 0) navigateTo(line);
1101711043
}
1101811044
});
1101911045

@@ -11414,10 +11440,10 @@ function updateAnalysisUI(): void {
1141411440
<div class="insight-section crash-section">
1141511441
<div class="insight-header">Crashes & Failures (${ins.crashes.length}${ins.crashes.length >= 50 ? '+' : ''})</div>
1141611442
${ins.crashes.map(c => `
11417-
<div class="crash-item" data-line="${c.lineNumber}" title="Line ${c.lineNumber}">
11443+
<div class="crash-item" data-line="${c.lineNumber}" title="Line ${c.lineNumber + 1}">
1141811444
<div class="crash-line">
1141911445
<span class="crash-keyword">${escapeHtml(c.keyword)}</span>
11420-
<span class="crash-line-num">line ${c.lineNumber}</span>
11446+
<span class="crash-line-num">line ${c.lineNumber + 1}</span>
1142111447
</div>
1142211448
<div class="crash-text">${escapeHtml(c.text.length > 100 ? c.text.substring(0, 100) + '...' : c.text)}</div>
1142311449
</div>
@@ -11516,16 +11542,16 @@ function updateAnalysisUI(): void {
1151611542
// Click handlers for crash items - navigate to line
1151711543
elements.analysisResults.querySelectorAll('.crash-item').forEach((item) => {
1151811544
item.addEventListener('click', () => {
11519-
const line = parseInt((item as HTMLElement).dataset.line || '0', 10);
11520-
if (line > 0) goToLine(line);
11545+
const line = parseInt((item as HTMLElement).dataset.line || '-1', 10);
11546+
if (line >= 0) navigateTo(line);
1152111547
});
1152211548
});
1152311549

1152411550
// Click handlers for component items - navigate to first error line
1152511551
elements.analysisResults.querySelectorAll('.component-item').forEach((item) => {
1152611552
item.addEventListener('click', () => {
11527-
const line = parseInt((item as HTMLElement).dataset.line || '0', 10);
11528-
if (line > 0) goToLine(line);
11553+
const line = parseInt((item as HTMLElement).dataset.line || '-1', 10);
11554+
if (line >= 0) navigateTo(line);
1152911555
});
1153011556
});
1153111557

@@ -13327,6 +13353,13 @@ function init(): void {
1332713353
// Agent connection status changes
1332813354
window.api.onAgentConnectionChanged((data: any) => {
1332913355
updateAgentConnectionStatus(data.connected, data.count, data.name);
13356+
const banner = document.getElementById('chat-reconnect-banner');
13357+
if (data.connected) {
13358+
banner?.classList.add('hidden');
13359+
} else if (agentRunning) {
13360+
hideTypingIndicator();
13361+
banner?.classList.remove('hidden');
13362+
}
1333013363
// If agent was running but disconnected (count dropped to 0), update button
1333113364
if (agentRunning && !data.connected) {
1333213365
agentRunning = false;

0 commit comments

Comments
 (0)