diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index ede38ae5..d80e54c6 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -67,7 +67,7 @@ import { type DiffType, getVcsContext, runVcsDiff, gitRuntime } from "@plannotat import { loadConfig, resolveDefaultDiffType, resolveUseJina } from "@plannotator/shared/config"; import { stripAtPrefix, resolveAtReference } from "@plannotator/shared/at-reference"; import { htmlToMarkdown } from "@plannotator/shared/html-to-markdown"; -import { urlToMarkdown } from "@plannotator/shared/url-to-markdown"; +import { urlToMarkdown, isConvertedSource } from "@plannotator/shared/url-to-markdown"; import { fetchRef, createWorktree, removeWorktree, ensureObjectAvailable } from "@plannotator/shared/worktree"; import { createWorktreePool, type WorktreePool } from "@plannotator/shared/worktree-pool"; import { parsePRUrl, checkPRAuth, fetchPR, getCliName, getCliInstallUrl, getMRLabel, getMRNumberLabel, getDisplayRepo } from "@plannotator/server/pr"; @@ -580,6 +580,7 @@ if (args[0] === "sessions") { let folderPath: string | undefined; let annotateMode: "annotate" | "annotate-folder" = "annotate"; let sourceInfo: string | undefined; + let sourceConverted = false; // --- URL annotation --- const isUrl = /^https?:\/\//i.test(filePath); @@ -590,6 +591,7 @@ if (args[0] === "sessions") { try { const result = await urlToMarkdown(filePath, { useJina }); markdown = result.markdown; + sourceConverted = isConvertedSource(result.source); if (process.env.PLANNOTATOR_DEBUG) { console.error(`[DEBUG] Fetched via ${result.source} (${markdown.length} chars)`); } @@ -636,6 +638,7 @@ if (args[0] === "sessions") { markdown = htmlToMarkdown(html); absolutePath = resolvedArg; sourceInfo = path.basename(resolvedArg); + sourceConverted = true; console.error(`Converted: ${absolutePath}`); } else { // Single markdown file annotation mode @@ -674,6 +677,7 @@ if (args[0] === "sessions") { mode: annotateMode, folderPath, sourceInfo, + sourceConverted, sharingEnabled, shareBaseUrl, pasteApiUrl, diff --git a/apps/opencode-plugin/commands.ts b/apps/opencode-plugin/commands.ts index 26b2ae5a..29693777 100644 --- a/apps/opencode-plugin/commands.ts +++ b/apps/opencode-plugin/commands.ts @@ -25,7 +25,7 @@ import { resolveMarkdownFile, resolveUserPath, hasMarkdownFiles } from "@plannot import { FILE_BROWSER_EXCLUDED } from "@plannotator/shared/reference-common"; import { htmlToMarkdown } from "@plannotator/shared/html-to-markdown"; import { parseAnnotateArgs } from "@plannotator/shared/annotate-args"; -import { urlToMarkdown } from "@plannotator/shared/url-to-markdown"; +import { urlToMarkdown, isConvertedSource } from "@plannotator/shared/url-to-markdown"; import { statSync } from "fs"; import path from "path"; @@ -170,6 +170,7 @@ export async function handleAnnotateCommand( let folderPath: string | undefined; let annotateMode: "annotate" | "annotate-folder" = "annotate"; let sourceInfo: string | undefined; + let sourceConverted = false; // --- URL annotation --- const isUrl = /^https?:\/\//i.test(filePath); @@ -180,6 +181,7 @@ export async function handleAnnotateCommand( try { const result = await urlToMarkdown(filePath, { useJina }); markdown = result.markdown; + sourceConverted = isConvertedSource(result.source); } catch (err) { client.app.log({ level: "error", message: `Failed to fetch URL: ${err instanceof Error ? err.message : String(err)}` }); return; @@ -223,6 +225,7 @@ export async function handleAnnotateCommand( markdown = htmlToMarkdown(html); absolutePath = resolvedArg; sourceInfo = path.basename(resolvedArg); + sourceConverted = true; client.app.log({ level: "info", message: `Converted: ${absolutePath}` }); } else { // Markdown file annotation @@ -258,6 +261,7 @@ export async function handleAnnotateCommand( mode: annotateMode, folderPath, sourceInfo, + sourceConverted, sharingEnabled: await getSharingEnabled(), shareBaseUrl: getShareBaseUrl(), pasteApiUrl: getPasteApiUrl(), diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index bb95fc06..dd8bfb43 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -35,7 +35,7 @@ import { planDenyFeedback } from "./generated/feedback-templates.js"; import { hasMarkdownFiles, resolveUserPath } from "./generated/resolve-file.js"; import { FILE_BROWSER_EXCLUDED } from "./generated/reference-common.js"; import { htmlToMarkdown } from "./generated/html-to-markdown.js"; -import { urlToMarkdown } from "./generated/url-to-markdown.js"; +import { urlToMarkdown, isConvertedSource } from "./generated/url-to-markdown.js"; import { loadConfig, resolveUseJina } from "./generated/config.js"; import { parseAnnotateArgs } from "./generated/annotate-args.js"; import { resolveAtReference } from "./generated/at-reference.js"; @@ -361,6 +361,7 @@ export default function plannotator(pi: ExtensionAPI): void { let folderPath: string | undefined; let mode: "annotate" | "annotate-folder" | undefined; let sourceInfo: string | undefined; + let sourceConverted = false; let isFolder = false; // --- URL annotation --- @@ -372,6 +373,7 @@ export default function plannotator(pi: ExtensionAPI): void { try { const result = await urlToMarkdown(filePath, { useJina }); markdown = result.markdown; + sourceConverted = isConvertedSource(result.source); } catch (err) { ctx.ui.notify(`Failed to fetch URL: ${err instanceof Error ? err.message : String(err)}`, "error"); return; @@ -420,6 +422,7 @@ export default function plannotator(pi: ExtensionAPI): void { const html = readFileSync(absolutePath, "utf-8"); markdown = htmlToMarkdown(html); sourceInfo = basename(absolutePath); + sourceConverted = true; ctx.ui.notify(`Opening annotation UI for ${filePath}...`, "info"); } else { markdown = readFileSync(absolutePath, "utf-8"); @@ -428,7 +431,7 @@ export default function plannotator(pi: ExtensionAPI): void { } try { - const result = await openMarkdownAnnotation(ctx, absolutePath, markdown, mode ?? "annotate", folderPath, sourceInfo, gate); + const result = await openMarkdownAnnotation(ctx, absolutePath, markdown, mode ?? "annotate", folderPath, sourceInfo, sourceConverted, gate); if (result.approved) { ctx.ui.notify("Annotation approved.", "info"); } else if (result.exit) { diff --git a/apps/pi-extension/plannotator-browser.ts b/apps/pi-extension/plannotator-browser.ts index 0bdd8cbd..418a0c8a 100644 --- a/apps/pi-extension/plannotator-browser.ts +++ b/apps/pi-extension/plannotator-browser.ts @@ -391,6 +391,7 @@ export async function openMarkdownAnnotation( mode: AnnotateMode, folderPath?: string, sourceInfo?: string, + sourceConverted?: boolean, gate?: boolean, ): Promise<{ feedback: string; exit?: boolean; approved?: boolean }> { if (!ctx.hasUI || !planHtmlContent) { @@ -416,6 +417,7 @@ export async function openMarkdownAnnotation( mode, folderPath, sourceInfo, + sourceConverted, gate, htmlContent: planHtmlContent, sharingEnabled: process.env.PLANNOTATOR_SHARE !== "disabled", @@ -431,7 +433,7 @@ export async function openLastMessageAnnotation( lastText: string, gate?: boolean, ): Promise<{ feedback: string; exit?: boolean; approved?: boolean }> { - return openMarkdownAnnotation(ctx, "last-message", lastText, "annotate-last", undefined, undefined, gate); + return openMarkdownAnnotation(ctx, "last-message", lastText, "annotate-last", undefined, undefined, undefined, gate); } export async function openArchiveBrowserAction( diff --git a/apps/pi-extension/plannotator-events.ts b/apps/pi-extension/plannotator-events.ts index 2b94d3d5..655d40fb 100644 --- a/apps/pi-extension/plannotator-events.ts +++ b/apps/pi-extension/plannotator-events.ts @@ -272,6 +272,7 @@ export function registerPlannotatorEventListeners(pi: ExtensionAPI): void { request.respond({ status: "error", error: "Missing filePath for annotate request." }); return; } + const sourceConverted = /\.html?$/i.test(payload.filePath) || /^https?:\/\//i.test(payload.filePath); const result = await openMarkdownAnnotation( ctx, payload.filePath, @@ -279,6 +280,7 @@ export function registerPlannotatorEventListeners(pi: ExtensionAPI): void { payload.mode ?? "annotate", payload.folderPath, undefined, + sourceConverted, payload.gate, ); request.respond({ status: "handled", result }); diff --git a/apps/pi-extension/server/reference.ts b/apps/pi-extension/server/reference.ts index d58dc136..d4563ec7 100644 --- a/apps/pi-extension/server/reference.ts +++ b/apps/pi-extension/server/reference.ts @@ -77,8 +77,9 @@ export function handleDocRequest(res: Res, url: URL): void { try { if (existsSync(fromBase)) { const raw = readFileSync(fromBase, "utf-8"); - const markdown = /\.html?$/i.test(requestedPath) ? htmlToMarkdown(raw) : raw; - json(res, { markdown, filepath: fromBase }); + const isHtml = /\.html?$/i.test(requestedPath); + const markdown = isHtml ? htmlToMarkdown(raw) : raw; + json(res, { markdown, filepath: fromBase, isConverted: isHtml }); return; } } catch { @@ -97,7 +98,7 @@ export function handleDocRequest(res: Res, url: URL): void { try { if (existsSync(resolvedHtml)) { const html = readFileSync(resolvedHtml, "utf-8"); - json(res, { markdown: htmlToMarkdown(html), filepath: resolvedHtml }); + json(res, { markdown: htmlToMarkdown(html), filepath: resolvedHtml, isConverted: true }); return; } } catch { /* fall through to 404 */ } diff --git a/apps/pi-extension/server/serverAnnotate.ts b/apps/pi-extension/server/serverAnnotate.ts index 75e3e9ad..2407d048 100644 --- a/apps/pi-extension/server/serverAnnotate.ts +++ b/apps/pi-extension/server/serverAnnotate.ts @@ -43,6 +43,7 @@ export async function startAnnotateServer(options: { shareBaseUrl?: string; pasteApiUrl?: string; sourceInfo?: string; + sourceConverted?: boolean; gate?: boolean; }): Promise { const gitUser = detectGitUser(); @@ -92,6 +93,7 @@ export async function startAnnotateServer(options: { mode: options.mode || "annotate", filePath: options.filePath, sourceInfo: options.sourceInfo, + sourceConverted: options.sourceConverted ?? false, gate: options.gate ?? false, sharingEnabled, shareBaseUrl, diff --git a/packages/editor/App.tsx b/packages/editor/App.tsx index 3c6c94a0..971d4e2d 100644 --- a/packages/editor/App.tsx +++ b/packages/editor/App.tsx @@ -1,6 +1,6 @@ import React, { useState, useEffect, useLayoutEffect, useMemo, useRef, useCallback } from 'react'; import { type Origin, getAgentName } from '@plannotator/shared/agents'; -import { parseMarkdownToBlocks, exportAnnotations, exportLinkedDocAnnotations, exportEditorAnnotations, extractFrontmatter, wrapFeedbackForAgent, Frontmatter } from '@plannotator/ui/utils/parser'; +import { parseMarkdownToBlocks, exportAnnotations, exportLinkedDocAnnotations, exportEditorAnnotations, extractFrontmatter, wrapFeedbackForAgent, Frontmatter, type LinkedDocAnnotationEntry } from '@plannotator/ui/utils/parser'; import { Viewer, ViewerHandle } from '@plannotator/ui/components/Viewer'; import { AnnotationPanel } from '@plannotator/ui/components/AnnotationPanel'; import { ExportModal } from '@plannotator/ui/components/ExportModal'; @@ -150,6 +150,7 @@ const App: React.FC = () => { const [gate, setGate] = useState(false); const [annotateSource, setAnnotateSource] = useState<'file' | 'message' | 'folder' | null>(null); const [sourceInfo, setSourceInfo] = useState(); + const [sourceConverted, setSourceConverted] = useState(false); const [sourceFilePath, setSourceFilePath] = useState(); const [imageBaseDir, setImageBaseDir] = useState(undefined); const [isLoading, setIsLoading] = useState(true); @@ -304,7 +305,7 @@ const App: React.FC = () => { const linkedDocHook = useLinkedDoc({ markdown, annotations, selectedAnnotationId, globalAttachments, setMarkdown, setAnnotations, setSelectedAnnotationId, setGlobalAttachments, - viewerRef, sidebar: linkedDocSidebar, sourceFilePath, + viewerRef, sidebar: linkedDocSidebar, sourceFilePath, sourceConverted, }); // Archive browser @@ -652,7 +653,7 @@ const App: React.FC = () => { if (!res.ok) throw new Error('Not in API mode'); return res.json(); }) - .then((data: { plan: string; origin?: Origin; mode?: 'annotate' | 'annotate-last' | 'annotate-folder' | 'archive'; filePath?: string; sourceInfo?: string; gate?: boolean; sharingEnabled?: boolean; shareBaseUrl?: string; pasteApiUrl?: string; repoInfo?: { display: string; branch?: string; host?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string } }) => { + .then((data: { plan: string; origin?: Origin; mode?: 'annotate' | 'annotate-last' | 'annotate-folder' | 'archive'; filePath?: string; sourceInfo?: string; sourceConverted?: boolean; gate?: boolean; sharingEnabled?: boolean; shareBaseUrl?: string; pasteApiUrl?: string; repoInfo?: { display: string; branch?: string; host?: string }; previousPlan?: string | null; versionInfo?: { version: number; totalVersions: number; project: string }; archivePlans?: ArchivedPlan[]; projectRoot?: string; isWSL?: boolean; serverConfig?: { displayName?: string; gitUser?: string } }) => { // Initialize config store with server-provided values (config file > cookie > default) configStore.init(data.serverConfig); // gitUser drives the "Use git name" button in Settings; stays undefined (button hidden) when unavailable @@ -682,6 +683,7 @@ const App: React.FC = () => { setAnnotateSource(data.mode === 'annotate-last' ? 'message' : data.mode === 'annotate-folder' ? 'folder' : 'file'); } setSourceInfo(data.sourceInfo ?? undefined); + setSourceConverted(!!data.sourceConverted); if (data.filePath) { setImageBaseDir(data.mode === 'annotate-folder' ? data.filePath : data.filePath.replace(/\/[^/]+$/, '')); if (data.mode === 'annotate') { @@ -1184,12 +1186,33 @@ const App: React.FC = () => { return 'User reviewed the document and has no feedback.'; } + // Derive the conversion flag for the currently-displayed document: + // when viewing a linked doc, use that doc's isConverted; otherwise use the root flag. + const activeConverted = linkedDocHook.isActive + ? (docAnnotations.get(linkedDocHook.filepath ?? '')?.isConverted ?? false) + : sourceConverted; + let output = hasPlanAnnotations - ? exportAnnotations(blocks, allAnnotations, globalAttachments, annotateSource === 'message' ? 'Message Feedback' : annotateSource === 'folder' ? 'Folder Feedback' : annotateSource === 'file' ? 'File Feedback' : 'Plan Feedback', annotateSource ?? 'plan') + ? exportAnnotations( + blocks, + allAnnotations, + globalAttachments, + annotateSource === 'message' ? 'Message Feedback' : annotateSource === 'folder' ? 'Folder Feedback' : annotateSource === 'file' ? 'File Feedback' : 'Plan Feedback', + annotateSource ?? 'plan', + { sourceConverted: activeConverted }, + ) : ''; if (hasDocAnnotations) { - output += exportLinkedDocAnnotations(docAnnotations); + // Parse blocks for each linked doc's cached markdown so the exporter + // can attach source line numbers per annotation. + const enriched: Map = new Map(docAnnotations); + for (const [filepath, entry] of enriched) { + if (entry.markdown) { + enriched.set(filepath, { ...entry, blocks: parseMarkdownToBlocks(entry.markdown) }); + } + } + output += exportLinkedDocAnnotations(enriched); } if (hasEditorAnnotations) { @@ -1197,7 +1220,7 @@ const App: React.FC = () => { } return output; - }, [blocks, allAnnotations, globalAttachments, linkedDocHook.getDocAnnotations, editorAnnotations]); + }, [blocks, allAnnotations, globalAttachments, linkedDocHook.getDocAnnotations, editorAnnotations, sourceConverted, annotateSource, linkedDocHook.isActive, linkedDocHook.filepath]); // Bot callback config — read once from URL search params (?cb=&ct=) const callbackConfig = React.useMemo(() => getCallbackConfig(), []); diff --git a/packages/server/annotate.ts b/packages/server/annotate.ts index 2370a35c..b3271474 100644 --- a/packages/server/annotate.ts +++ b/packages/server/annotate.ts @@ -50,6 +50,9 @@ export interface AnnotateServerOptions { pasteApiUrl?: string; /** Source attribution: original URL or filename (e.g. "https://..." or "index.html") */ sourceInfo?: string; + /** True when `markdown` was produced by Turndown/Jina (HTML or URL) — + * feedback line numbers won't match the original source. */ + sourceConverted?: boolean; /** Enable review-gate UX: adds an Approve button alongside Close/Send Annotations (#570) */ gate?: boolean; /** Called when server starts with the URL, remote status, and port */ @@ -98,6 +101,7 @@ export async function startAnnotateServer( mode = "annotate", folderPath, sourceInfo, + sourceConverted, sharingEnabled = true, shareBaseUrl, pasteApiUrl, @@ -155,6 +159,7 @@ export async function startAnnotateServer( mode, filePath, sourceInfo, + sourceConverted: sourceConverted ?? false, gate, sharingEnabled, shareBaseUrl, diff --git a/packages/server/reference-handlers.ts b/packages/server/reference-handlers.ts index 6d3c9055..a81d7689 100644 --- a/packages/server/reference-handlers.ts +++ b/packages/server/reference-handlers.ts @@ -45,8 +45,9 @@ export async function handleDoc(req: Request): Promise { const file = Bun.file(fromBase); if (await file.exists()) { const raw = await file.text(); - const markdown = /\.html?$/i.test(requestedPath) ? htmlToMarkdown(raw) : raw; - return Response.json({ markdown, filepath: fromBase }); + const isHtml = /\.html?$/i.test(requestedPath); + const markdown = isHtml ? htmlToMarkdown(raw) : raw; + return Response.json({ markdown, filepath: fromBase, isConverted: isHtml }); } } catch { /* fall through to standard resolution */ @@ -65,7 +66,7 @@ export async function handleDoc(req: Request): Promise { if (await file.exists()) { const html = await file.text(); const markdown = htmlToMarkdown(html); - return Response.json({ markdown, filepath: resolvedHtml }); + return Response.json({ markdown, filepath: resolvedHtml, isConverted: true }); } } catch { /* fall through */ } return Response.json({ error: `File not found: ${requestedPath}` }, { status: 404 }); diff --git a/packages/shared/url-to-markdown.ts b/packages/shared/url-to-markdown.ts index 3be7510d..b6c96f55 100644 --- a/packages/shared/url-to-markdown.ts +++ b/packages/shared/url-to-markdown.ts @@ -17,6 +17,11 @@ export interface UrlToMarkdownResult { source: "jina" | "fetch+turndown" | "fetch-raw" | "content-negotiation"; } +/** True when the source indicates the markdown was converted from HTML, + * not returned as-is from the origin. */ +export const isConvertedSource = (source: UrlToMarkdownResult["source"]): boolean => + source === "jina" || source === "fetch+turndown"; + const FETCH_TIMEOUT_MS = 30_000; const MAX_BODY_BYTES = 10 * 1024 * 1024; // 10 MB — matches local HTML file guard diff --git a/packages/ui/hooks/useLinkedDoc.ts b/packages/ui/hooks/useLinkedDoc.ts index 5ac01bc6..c7d3a3d4 100644 --- a/packages/ui/hooks/useLinkedDoc.ts +++ b/packages/ui/hooks/useLinkedDoc.ts @@ -25,6 +25,9 @@ export interface UseLinkedDocOptions { /** Absolute path of the primary document — enables getDocAnnotations() to include * stashed original-file annotations when viewing a linked doc. */ sourceFilePath?: string; + /** Whether the primary document was converted from HTML/URL — propagated to the + * stashed entry so feedback caveats survive cross-doc navigation. */ + sourceConverted?: boolean; } interface SavedPlanState { @@ -37,6 +40,8 @@ interface SavedPlanState { export interface CachedDocState { annotations: Annotation[]; globalAttachments: ImageAttachment[]; + markdown?: string; + isConverted?: boolean; } export interface UseLinkedDocReturn { @@ -75,9 +80,10 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { viewerRef, sidebar, sourceFilePath, + sourceConverted, } = options; - const [linkedDoc, setLinkedDoc] = useState<{ filepath: string } | null>(null); + const [linkedDoc, setLinkedDoc] = useState<{ filepath: string; isConverted?: boolean; markdown?: string } | null>(null); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(false); const [docAnnotationCount, setDocAnnotationCount] = useState(0); @@ -104,6 +110,7 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { const data = (await res.json()) as { markdown?: string; filepath?: string; + isConverted?: boolean; error?: string; matches?: string[]; }; @@ -147,6 +154,8 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { docCache.current.set(linkedDoc.filepath, { annotations: [...annotations], globalAttachments: [...globalAttachments], + markdown: linkedDoc.markdown, + isConverted: linkedDoc.isConverted, }); let total = 0; for (const [fp, cached] of docCache.current.entries()) { @@ -167,7 +176,11 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { setAnnotations(cached?.annotations ?? []); setGlobalAttachments(cached?.globalAttachments ?? []); setSelectedAnnotationId(null); - setLinkedDoc({ filepath: data.filepath! }); + setLinkedDoc({ + filepath: data.filepath!, + isConverted: !!data.isConverted, + markdown: data.markdown, + }); sidebar.open(targetTab ?? "toc"); // Re-apply cached annotations after DOM settles @@ -209,6 +222,8 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { docCache.current.set(linkedDoc.filepath, { annotations: [...annotations], globalAttachments: [...globalAttachments], + markdown: linkedDoc.markdown, + isConverted: linkedDoc.isConverted, }); // Update reactive count so button labels can respond let total = 0; @@ -255,16 +270,20 @@ export function useLinkedDoc(options: UseLinkedDocOptions): UseLinkedDocReturn { result.set(sourceFilePath, { annotations: [...savedPlanState.current.annotations], globalAttachments: [...savedPlanState.current.globalAttachments], + markdown: savedPlanState.current.markdown, + isConverted: !!sourceConverted, }); } if (linkedDoc) { result.set(linkedDoc.filepath, { annotations: [...annotations], globalAttachments: [...globalAttachments], + markdown: linkedDoc.markdown, + isConverted: linkedDoc.isConverted, }); } return result; - }, [linkedDoc, annotations, globalAttachments, sourceFilePath]); + }, [linkedDoc, annotations, globalAttachments, sourceFilePath, sourceConverted]); return { isActive: linkedDoc !== null, diff --git a/packages/ui/utils/parser.test.ts b/packages/ui/utils/parser.test.ts index ad16e331..524d24f1 100644 --- a/packages/ui/utils/parser.test.ts +++ b/packages/ui/utils/parser.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "bun:test"; -import { parseMarkdownToBlocks, computeListIndices } from "./parser"; +import { parseMarkdownToBlocks, computeListIndices, extractFrontmatter, exportAnnotations } from "./parser"; import type { Block } from "../types"; /** Tiny factory for list-item blocks used by computeListIndices tests. */ @@ -833,3 +833,105 @@ describe("computeListIndices", () => { expect(computeListIndices(blocks)).toEqual([1]); }); }); + +describe("extractFrontmatter — contentStartLine", () => { + test("no frontmatter → contentStartLine is 1", () => { + const { contentStartLine } = extractFrontmatter("# Hello\nworld"); + expect(contentStartLine).toBe(1); + }); + + test("standard frontmatter offsets correctly", () => { + const md = "---\ntitle: foo\ndate: bar\n---\n# Hello"; + const { contentStartLine, content } = extractFrontmatter(md); + expect(content).toBe("# Hello"); + expect(contentStartLine).toBe(5); + }); + + test("frontmatter with blank line before content", () => { + const md = "---\ntitle: foo\n---\n\n# Hello"; + const { contentStartLine } = extractFrontmatter(md); + expect(contentStartLine).toBe(5); + }); + + test("unclosed frontmatter treated as no frontmatter", () => { + const md = "---\ntitle: foo\n# Not closed"; + const { contentStartLine, content } = extractFrontmatter(md); + expect(contentStartLine).toBe(1); + expect(content).toBe(md); + }); +}); + +describe("parseMarkdownToBlocks — startLine accuracy", () => { + test("basic blocks get correct startLine", () => { + const md = "# Heading\n\nParagraph\n\n- Item"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks[0].startLine).toBe(1); // # Heading + expect(blocks[1].startLine).toBe(3); // Paragraph + expect(blocks[2].startLine).toBe(5); // - Item + }); + + test("code block startLine points to the opening fence", () => { + const md = "intro\n\n```ts\nconst x = 1;\nconst y = 2;\n```\n\noutro"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks[0].startLine).toBe(1); // intro + expect(blocks[1].startLine).toBe(3); // ```ts + expect(blocks[1].type).toBe("code"); + expect(blocks[2].startLine).toBe(8); // outro + }); + + test("frontmatter shifts all startLines", () => { + const md = "---\ntitle: test\n---\n\n# Heading\n\nParagraph"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks[0].type).toBe("heading"); + expect(blocks[0].startLine).toBe(5); + expect(blocks[1].type).toBe("paragraph"); + expect(blocks[1].startLine).toBe(7); + }); + + test("table gets correct startLine", () => { + const md = "# Title\n\n| A | B |\n|---|---|\n| 1 | 2 |"; + const blocks = parseMarkdownToBlocks(md); + expect(blocks[1].type).toBe("table"); + expect(blocks[1].startLine).toBe(3); + }); +}); + +describe("exportAnnotations — line labels", () => { + const blocks = parseMarkdownToBlocks("# Heading\n\nShort paragraph\n\n```ts\nline1\nline2\nline3\n```"); + + test("single-line block shows 'line N'", () => { + const anns = [{ blockId: blocks[0].id, type: "COMMENT", text: "fix this", originalText: "Heading", startOffset: 0 }]; + const output = exportAnnotations(blocks, anns); + expect(output).toContain("(line 1)"); + }); + + test("multi-line code block shows line range", () => { + const codeBlock = blocks.find(b => b.type === "code")!; + const anns = [{ blockId: codeBlock.id, type: "COMMENT", text: "refactor", originalText: "line1", startOffset: 0 }]; + const output = exportAnnotations(blocks, anns); + expect(output).toMatch(/\(lines 5–9\)/); + }); + + test("GLOBAL_COMMENT has no line label", () => { + const anns = [{ blockId: "global", type: "GLOBAL_COMMENT", text: "overall feedback", originalText: "", startOffset: 0 }]; + const output = exportAnnotations(blocks, anns); + expect(output).not.toMatch(/\(line/); + }); + + test("diff-context annotation shows label instead of line number", () => { + const anns = [{ blockId: blocks[0].id, type: "COMMENT", text: "change", originalText: "Heading", startOffset: 0, diffContext: "added" }]; + const output = exportAnnotations(blocks, anns); + expect(output).not.toMatch(/\(line/); + expect(output).toContain("[In diff content]"); + }); + + test("sourceConverted adds caveat", () => { + const output = exportAnnotations(blocks, [{ blockId: blocks[0].id, type: "COMMENT", text: "ok", originalText: "Heading", startOffset: 0 }], [], "Feedback", "plan", { sourceConverted: true }); + expect(output).toContain("converted markdown"); + }); + + test("no sourceConverted means no caveat", () => { + const output = exportAnnotations(blocks, [{ blockId: blocks[0].id, type: "COMMENT", text: "ok", originalText: "Heading", startOffset: 0 }]); + expect(output).not.toContain("converted markdown"); + }); +}); diff --git a/packages/ui/utils/parser.ts b/packages/ui/utils/parser.ts index 9f2c75b8..4d392f5b 100644 --- a/packages/ui/utils/parser.ts +++ b/packages/ui/utils/parser.ts @@ -10,23 +10,34 @@ export interface Frontmatter { /** * Extract YAML frontmatter from markdown if present. - * Returns both the parsed frontmatter and the remaining markdown. + * Returns the parsed frontmatter, the remaining markdown, and the 1-based + * line number where content begins in the original file (so downstream + * line references stay accurate). */ -export function extractFrontmatter(markdown: string): { frontmatter: Frontmatter | null; content: string } { +export function extractFrontmatter(markdown: string): { frontmatter: Frontmatter | null; content: string; contentStartLine: number } { const trimmed = markdown.trimStart(); if (!trimmed.startsWith('---')) { - return { frontmatter: null, content: markdown }; + return { frontmatter: null, content: markdown, contentStartLine: 1 }; } // Find the closing --- const endIndex = trimmed.indexOf('\n---', 3); if (endIndex === -1) { - return { frontmatter: null, content: markdown }; + return { frontmatter: null, content: markdown, contentStartLine: 1 }; } // Extract frontmatter content (between the --- delimiters) const frontmatterRaw = trimmed.slice(4, endIndex).trim(); - const afterFrontmatter = trimmed.slice(endIndex + 4).trimStart(); + const rawAfterFrontmatter = trimmed.slice(endIndex + 4); + const afterFrontmatter = rawAfterFrontmatter.trimStart(); + + // Compute the 1-based line where content begins in the original file. + // Account for: leading whitespace trimmed from original, the frontmatter + // block itself, and any blank lines between closing --- and first content. + const leadingChars = markdown.length - trimmed.length; + const consumedInTrimmed = endIndex + 4 + (rawAfterFrontmatter.length - afterFrontmatter.length); + const consumedTotal = leadingChars + consumedInTrimmed; + const contentStartLine = (markdown.slice(0, consumedTotal).match(/\n/g) || []).length + 1; // Parse simple YAML (key: value pairs) const frontmatter: Frontmatter = {}; @@ -60,7 +71,7 @@ export function extractFrontmatter(markdown: string): { frontmatter: Frontmatter } } - return { frontmatter, content: afterFrontmatter }; + return { frontmatter, content: afterFrontmatter, contentStartLine }; } /** @@ -88,7 +99,7 @@ const HTML_BLOCK_OPEN_RE = /^<\/?([a-zA-Z][a-zA-Z0-9]*)(?:\s|>|\/|$)/; * but for this demo, we want predictable text-anchoring. */ export const parseMarkdownToBlocks = (markdown: string): Block[] => { - const { content: cleanMarkdown } = extractFrontmatter(markdown); + const { content: cleanMarkdown, contentStartLine } = extractFrontmatter(markdown); const lines = cleanMarkdown.split('\n'); const blocks: Block[] = []; let currentId = 0; @@ -96,7 +107,7 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { let buffer: string[] = []; let currentType: Block['type'] = 'paragraph'; let currentLevel = 0; - let bufferStartLine = 1; // Track the start line of the current buffer + let bufferStartLine = contentStartLine; let lastLineWasBlank = false; const flush = () => { @@ -117,7 +128,7 @@ export const parseMarkdownToBlocks = (markdown: string): Block[] => { for (let i = 0; i < lines.length; i++) { const line = lines[i]; const trimmed = line.trim(); - const currentLineNum = i + 1; // 1-based index + const currentLineNum = i + contentStartLine; const prevLineWasBlank = lastLineWasBlank; lastLineWasBlank = false; @@ -447,7 +458,40 @@ export const computeListIndices = (blocks: Block[]): (number | null)[] => { export const wrapFeedbackForAgent = (feedback: string): string => planDenyFeedback(feedback); -export const exportAnnotations = (blocks: Block[], annotations: any[], globalAttachments: ImageAttachment[] = [], title: string = 'Plan Feedback', subject: string = 'plan'): string => { +export interface ExportAnnotationsOptions { + sourceConverted?: boolean; +} + +/** Compute the end line of a block from its content and type. */ +const blockEndLine = (block: Block): number => { + if (!block.content) return block.startLine; + const contentLines = block.content.split('\n').length; + if (block.type === 'code') return block.startLine + contentLines + 1; + if (block.type === 'directive') return block.startLine + contentLines + 1; + if (block.alertKind) return block.startLine + contentLines; + return block.startLine + contentLines - 1; +}; + +/** Resolve the source-line label for a single annotation. + * Returns null for global comments, diff-view annotations, or missing blocks. */ +const lineLabelForAnnotation = (blocks: Block[], ann: any): string | null => { + if (!ann.blockId || ann.type === 'GLOBAL_COMMENT') return null; + if (typeof ann.blockId === 'string' && ann.blockId.startsWith('diff-block-')) return null; + const block = blocks.find(b => b.id === ann.blockId); + if (!block || typeof block.startLine !== 'number') return null; + const end = blockEndLine(block); + if (end <= block.startLine) return `line ${block.startLine}`; + return `lines ${block.startLine}–${end}`; +}; + +export const exportAnnotations = ( + blocks: Block[], + annotations: any[], + globalAttachments: ImageAttachment[] = [], + title: string = 'Plan Feedback', + subject: string = 'plan', + opts: ExportAnnotationsOptions = {}, +): string => { if (annotations.length === 0 && globalAttachments.length === 0) { return 'No changes detected.'; } @@ -462,6 +506,10 @@ export const exportAnnotations = (blocks: Block[], annotations: any[], globalAtt let output = `# ${title}\n\n`; + if (opts.sourceConverted) { + output += `> Note: Line numbers below refer to the converted markdown, not the original HTML/URL source.\n\n`; + } + // Add global reference images section if any if (globalAttachments.length > 0) { output += `## Reference Images\n`; @@ -477,13 +525,14 @@ export const exportAnnotations = (blocks: Block[], annotations: any[], globalAtt } sortedAnns.forEach((ann, index) => { - const block = blocks.find(b => b.id === ann.blockId); - output += `## ${index + 1}. `; // Add diff context label if annotation was created in diff view if (ann.diffContext) { output += `[In diff content] `; + } else { + const lineLabel = lineLabelForAnnotation(blocks, ann); + if (lineLabel) output += `(${lineLabel}) `; } switch (ann.type) { @@ -542,15 +591,23 @@ export const exportAnnotations = (blocks: Block[], annotations: any[], globalAtt return output; }; +export interface LinkedDocAnnotationEntry { + annotations: Annotation[]; + globalAttachments: ImageAttachment[]; + markdown?: string; + blocks?: Block[]; + isConverted?: boolean; +} + export const exportLinkedDocAnnotations = ( - docAnnotations: Map + docAnnotations: Map ): string => { let output = `\n# Linked Document Feedback\n\nThe following feedback is on documents referenced in the plan.\n\n`; - for (const [filepath, { annotations, globalAttachments }] of docAnnotations) { + for (const [filepath, { annotations, globalAttachments, blocks: docBlocks, isConverted }] of docAnnotations) { if (annotations.length === 0 && globalAttachments.length === 0) continue; - output += `## ${filepath}\n\n`; + output += `## ${filepath}${isConverted ? ' (converted from HTML — line numbers refer to converted markdown)' : ''}\n\n`; if (globalAttachments.length > 0) { output += `### Reference Images\n`; @@ -572,6 +629,9 @@ export const exportLinkedDocAnnotations = ( sortedAnns.forEach((ann, index) => { output += `### ${index + 1}. `; + const lineLabel = docBlocks ? lineLabelForAnnotation(docBlocks, ann) : null; + if (lineLabel) output += `(${lineLabel}) `; + switch (ann.type) { case 'DELETION': output += `Remove this\n`;