Skip to content

Commit 6439396

Browse files
committed
refactor(file-viewer): audit fixes — stale docs, DRY settle-lock, language detection
- rich-markdown-editor: rewrite the now-stale single-surface docstring (no PreviewPanel); extract a shared lockSettled() helper used by both the mount and stream-settle paths; guard the settle re-seed so it only setContent's when the body actually changed (no redundant doc rebuild) - detect-language: stop misreading generics (List<String>) as HTML markup; detect Go type/struct - code-block: export LANGUAGE_OPTIONS + add a test asserting every picker language has a registered Prism grammar (prevents picker/highlighter drift)
1 parent 0eb6ce2 commit 6439396

5 files changed

Lines changed: 79 additions & 28 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/code-block.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ import { detectLanguage } from './detect-language'
1717

1818
const PLAIN = 'plain'
1919

20-
/** Languages the Prism highlighter has registered (see {@link CodeBlockHighlight}). */
21-
const LANGUAGE_OPTIONS = [
20+
/** Languages the Prism highlighter has registered (see {@link CodeBlockHighlight}). Every non-plain
21+
* value MUST have a grammar registered in {@link CodeBlockHighlight} — enforced by a unit test. */
22+
export const LANGUAGE_OPTIONS = [
2223
{ value: PLAIN, label: 'Plain text' },
2324
{ value: 'bash', label: 'Bash' },
2425
{ value: 'c', label: 'C' },
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*
4+
* Guards against drift between the code-block language picker and the Prism grammars actually
5+
* registered by CodeBlockHighlight: every selectable language must have a registered grammar, or it
6+
* would silently fall back to no highlighting.
7+
*/
8+
import Prism from 'prismjs'
9+
import { describe, expect, it } from 'vitest'
10+
import { LANGUAGE_OPTIONS } from './code-block'
11+
// Importing the highlighter registers all the prism-* grammars as a side effect.
12+
import './code-highlight'
13+
14+
describe('code-block languages', () => {
15+
it('every selectable language has a registered Prism grammar', () => {
16+
for (const { value } of LANGUAGE_OPTIONS) {
17+
if (value === 'plain') continue
18+
expect(Prism.languages[value], `no Prism grammar registered for "${value}"`).toBeDefined()
19+
}
20+
})
21+
})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/detect-language.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,17 @@ describe('detectLanguage', () => {
2222
it('does not misclassify a JS object as JSON', () => {
2323
expect(detectLanguage('const x = { a: 1 }')).toBe('javascript')
2424
})
25+
26+
it('detects Go, Rust, Java', () => {
27+
expect(detectLanguage('package main\n\nfunc main() {\n\tfmt.Println("hi")\n}')).toBe('go')
28+
expect(detectLanguage('type User struct {\n\tName string\n}')).toBe('go')
29+
expect(detectLanguage('fn main() {\n let mut x = 1;\n println!("{}", x);\n}')).toBe('rust')
30+
expect(detectLanguage('public class Box {\n private int n;\n}')).toBe('java')
31+
})
32+
33+
it('does not misread generics as HTML markup', () => {
34+
expect(detectLanguage('public class Box { private List<String> items; }')).toBe('java')
35+
expect(detectLanguage('let v: Vec<String> = Vec::new();\nfn f() {}')).toBe('rust')
36+
expect(detectLanguage('func Map[T any](s []T) {}\npackage x')).toBe('go')
37+
})
2538
})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/detect-language.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
* {@link CodeBlockHighlight} actually registers with Prism; returns `null` when unsure.
66
*/
77
const DETECTORS: ReadonlyArray<{ language: string; test: RegExp }> = [
8-
{ language: 'markup', test: /<\/?[a-z][\w-]*(\s[^>]*)?\/?>/i },
8+
// Real HTML: a closing tag, an opening tag with an attribute, or a doctype/comment. Deliberately
9+
// NOT a bare `<Word>` so generics (`List<String>`, `Vec<T>`) aren't misread as markup.
10+
{ language: 'markup', test: /<\/[a-z][\w-]*\s*>|<[a-z][\w-]*\s+[\w:-]+=|<!(?:doctype\b|--)/i },
911
{
1012
language: 'sql',
1113
test: /\b(?:select\s+[\w*]|insert\s+into|update\s+\w+\s+set|delete\s+from|create\s+table)/i,
@@ -17,7 +19,7 @@ const DETECTORS: ReadonlyArray<{ language: string; test: RegExp }> = [
1719
},
1820
{
1921
language: 'go',
20-
test: /^\s*package\s+\w+|\bfunc\s+(\(\w[^)]*\)\s+)?\w+\s*\(|\bfmt\.\w|:=/m,
22+
test: /^\s*package\s+\w+|\bfunc\s+(\(\w[^)]*\)\s+)?\w+\s*\(|\btype\s+\w+\s+(struct|interface)\b|\bfmt\.\w|:=/m,
2123
},
2224
{
2325
language: 'rust',

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,15 @@ interface RichMarkdownEditorProps {
4343

4444
/**
4545
* Inline WYSIWYG markdown editor (TipTap/ProseMirror) for markdown files — a single editing surface
46-
* (markdown transformed inline as you type), no raw/preview split. Owns the file lifecycle through a
47-
* single {@link useEditableFileContent} engine: while agent output streams in (and during the
48-
* post-stream reconcile) it shows the read-only {@link PreviewPanel} with autosave disabled, so the
49-
* editor never races the agent's server-side write. Once content is loaded and settled it mounts the
50-
* actual editor.
46+
* (markdown transformed inline as you type), no raw/preview split and no separate streaming preview.
47+
* Owns the file lifecycle through a single {@link useEditableFileContent} engine, and the TipTap
48+
* editor is the ONLY thing the user ever sees: while agent output streams in it renders that content
49+
* read-only (synced per chunk), then the same editor instance becomes editable once the stream
50+
* settles — so the stream→edit transition has no renderer swap or flash.
5151
*
52-
* The editor is mounted only once content is ready, and is keyed by file id — so the loaded markdown
53-
* is the editor's *initial* `content` (parsed at create time), not pushed in by a sync effect. That
54-
* keeps it robust to TipTap's strict-mode/SSR instance lifecycle: there is no content-sync effect to
55-
* race, so a freshly created (or strict-mode-recreated) editor is always born with the right document.
52+
* The editor is keyed by file id (+ streaming context). A file opened outside a stream uses the plain
53+
* create-time initial-content model (no sync). See {@link LoadedRichMarkdownEditor} for the
54+
* read-only-stream → editable hand-off.
5655
*/
5756
export const RichMarkdownEditor = memo(function RichMarkdownEditor({
5857
file,
@@ -126,6 +125,22 @@ interface LoadedRichMarkdownEditorProps {
126125
onSaveShortcut: () => Promise<void>
127126
}
128127

128+
interface SettledContent {
129+
frontmatter: string
130+
verdict: boolean
131+
}
132+
133+
/**
134+
* Lock the round-trip verdict + frontmatter on the content the editor "opens" with — once, at mount
135+
* for a settled file or at the moment a stream settles. A round-trip-unsafe document (raw HTML,
136+
* footnotes, >128KB, …) opens read-only so an edit can't corrupt it; a safe one stays editable. Never
137+
* re-derived: a dirty document is safe by construction (the editor only emits safe markdown), so
138+
* flipping editability off mid-edit would only strand edits.
139+
*/
140+
function lockSettled(content: string): SettledContent {
141+
return { frontmatter: splitFrontmatter(content).frontmatter, verdict: isRoundTripSafe(content) }
142+
}
143+
129144
/**
130145
* The single TipTap editor for a markdown file — the only surface the user ever sees. While agent
131146
* output streams in ({@link streaming}) it renders that content read-only and re-syncs each chunk;
@@ -149,21 +164,16 @@ export function LoadedRichMarkdownEditor({
149164
// content until the stream settles; otherwise it uses the plain create-time initial-content model.
150165
const streamingAtMountRef = useRef(streaming)
151166

152-
// The round-trip verdict + frontmatter, locked once on the content the editor "opens" with — at
153-
// mount for a settled file, or at the moment the stream settles for a streamed one. A round-trip-
154-
// unsafe document (raw HTML, footnotes, >128KB, …) opens read-only so an edit can't corrupt it; a
155-
// safe one is editable. Once locked it is never re-derived: a dirty document is safe by construction
156-
// (the editor only emits safe markdown), so flipping editability off mid-edit would strand edits.
157-
const settledRef = useRef<{ frontmatter: string; verdict: boolean } | null>(null)
167+
// The verdict + frontmatter locked via {@link lockSettled} — at mount for a settled file, or at the
168+
// moment a stream settles (in the effect below). Null until then; reads default to read-only.
169+
const settledRef = useRef<SettledContent | null>(null)
158170
if (!streamingAtMountRef.current && settledRef.current === null) {
159-
settledRef.current = {
160-
frontmatter: splitFrontmatter(content).frontmatter,
161-
verdict: isRoundTripSafe(content),
162-
}
171+
settledRef.current = lockSettled(content)
163172
}
164173
const isEditable = canEdit && !streaming && (settledRef.current?.verdict ?? false)
165174

166-
// The body that seeds the editor at create time — empty when streaming (filled by the sync effect).
175+
// The body that seeds the editor at create time. Empty when streaming — the sync effect pushes the
176+
// streamed body in via setContent (this ref is never written again).
167177
const initialBodyRef = useRef(streamingAtMountRef.current ? '' : splitFrontmatter(content).body)
168178
// The frontmatter re-attached on every change. Empty until the content settles (the editor never
169179
// displays frontmatter, so a streamed doc simply shows its body).
@@ -280,10 +290,14 @@ export function LoadedRichMarkdownEditor({
280290
return
281291
}
282292
if (settledRef.current === null) {
283-
const { frontmatter, body } = splitFrontmatter(content)
284-
settledRef.current = { frontmatter, verdict: isRoundTripSafe(content) }
285-
lastSyncedBodyRef.current = body
286-
editor.commands.setContent(body, { contentType: 'markdown', emitUpdate: false })
293+
settledRef.current = lockSettled(content)
294+
// Re-seed only if the settled body differs from the last streamed chunk — it usually doesn't,
295+
// and an extra setContent would needlessly rebuild the doc and drop selection/scroll.
296+
const body = splitFrontmatter(content).body
297+
if (body !== lastSyncedBodyRef.current) {
298+
lastSyncedBodyRef.current = body
299+
editor.commands.setContent(body, { contentType: 'markdown', emitUpdate: false })
300+
}
287301
editor.setEditable(canEdit && settledRef.current.verdict)
288302
if (autoFocus) editor.commands.focus('end')
289303
return

0 commit comments

Comments
 (0)