chore(editor): trim review-flagged comments that restate or narrate

Drop comments that restate the code or narrate a test assertion: the value-sync
intro (the skip-echo block below already explains it), the two PM descendants
return-value notes, the marked-cast type trivia, the 6-line aria-label essay,
the corpus-test harness/per-assertion narration (the it() title already lists
the checks), and two table/nesting assertion labels. Tightened the knowledge-form
"server's canonical document" note to its load-bearing line.

Kept the genuinely load-bearing ones the pass also flagged: the performSave
navigation/cycle warning (removing it invites reintroducing the guard cycle) and
the test "sanity" assertions (they prove the split/hard-break case is exercised).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Sergey Kozyrenko
2026-06-30 22:15:02 +07:00
co-authored by Claude Opus 4.8
parent 7c8f017b27
commit 768d733c88
7 changed files with 4 additions and 28 deletions
@@ -18,7 +18,7 @@ export const collectInlineMatches = (doc: PMNode, regex: RegExp): InlineMatch[]
doc.descendants((node, pos) => {
if (!node.isTextblock) {
return; // a container — recurse into it to reach its textblocks
return;
}
let text = '';
@@ -46,7 +46,7 @@ export const collectInlineMatches = (doc: PMNode, regex: RegExp): InlineMatch[]
}
}
return false; // inline content handled — don't recurse into the text nodes
return false;
});
return matches;
@@ -77,8 +77,6 @@ export const MarkdownTable = Table.extend({
},
});
// `new Marked()` is a Marked instance, but the `marked` option is typed as the default singleton (which
// carries extra statics like getDefaults); the cast bridges that structural gap.
export const createMarkdownLayer = () => [
Markdown.configure({ marked: createFaithfulMarked() as unknown as typeof import('marked').marked }),
FaithfulMarkdownText,
@@ -15,9 +15,6 @@ beforeAll(() => {
({ item: () => null, length: 0, [Symbol.iterator]: [][Symbol.iterator] }) as unknown as DOMRectList;
});
// Node-fs corpus test: reads the real backend prompt templates (XML-tag-heavy Go templates) and asserts
// the @tiptap/markdown round-trip preserves their CONTENT on EVERY one. Excluded from the app `tsc` build
// (uses node APIs); validated at runtime by vitest.
const roundTrip = (content: string): string => {
const editor = new Editor({ content, contentType: 'markdown', extensions: createMarkdownExtensions() });
const out = editor.getMarkdown();
@@ -39,16 +36,12 @@ describe('corpus — every real prompt .tmpl survives the round-trip with no con
const save1 = roundTrip(src);
const save2 = roundTrip(save1);
// tags stay literal — no HTML-entity escaping introduced.
expect(save1).not.toContain('&lt;');
expect(save1).not.toContain('&gt;');
// every {{ }} action survives (set + order).
expect(variables(save1)).toEqual(variables(src));
// no source word is dropped (cosmetic whitespace reformatting aside).
const after = new Set(words(save2));
const lost = [...new Set(words(src))].filter((w) => !after.has(w));
expect(lost).toEqual([]);
// converges — the canonical form is stable on resave.
expect(roundTrip(save2)).toBe(save2);
});
}
@@ -100,7 +100,6 @@ describe('MarkdownTable — cell pipes escaped + alignment preserved, idempotent
it('preserves per-column alignment (left :--- / center :---: / right ---:)', () => {
const save1 = roundTrip('| L | C | R |\n| :-- | :-: | --: |\n| a | b | c |');
// renderTableToMarkdown emits the alignment colons (dash count padded to column width, min 3).
expect(save1).toContain('| :--- | :---: | ---: |');
expect(roundTrip(save1)).toBe(save1);
});
@@ -120,7 +119,6 @@ describe('nesting & sequencing — content preserved and converges (≤2 saves)'
const save1 = roundTrip(src);
const save2 = roundTrip(save1);
// converges (canonicalizes once, then stable) and no word is dropped.
expect(save2).toBe(save1);
sameWords(src, save2);
});
@@ -190,8 +190,6 @@ function MarkdownEditor({
[editor],
);
// Keep external value in sync (e.g. on form reset). Avoid resetting if
// the editor already reflects the same markdown to keep cursor stable.
useEffect(() => {
if (!editor) {
return;
@@ -237,11 +237,8 @@ export function KnowledgeForm({ initialValues, isNew, knowledge, onSubmit }: Kno
// `isDirty`/`canSubmit` at submit time.
const result = await onSubmit(values, form.formState.dirtyFields as DirtyFlags);
// Prefer the server's view of the document — backend may have
// trimmed/normalized fields, attached derived data, or filled
// optional fields. Falling back to the local `values` keeps
// the form stable when the mutation hook can't return the
// saved fragment for some reason.
// After save, show the server's canonical (trimmed/normalized) document; fall back to the
// submitted values if the mutation hook didn't return the saved fragment.
const resetValues = result.document ? documentToFormValues(result.document) : values;
// Reset BEFORE the caller navigates so `isDirty` is false by the
@@ -68,14 +68,6 @@ export function SettingsProvidersHeader() {
<div className="flex items-center justify-between gap-4">
<p className="text-muted-foreground min-w-0 flex-1 truncate">Manage language model providers</p>
{/*
* "Create Provider" is a dropdown trigger, not a submit-style action it
* opens a menu listing provider types (OpenAI, Anthropic, Custom, ). The
* `<ChevronDown />` icon plus Radix's `aria-haspopup="menu"` already signal
* "menu opens" to sighted and AT users; the explicit aria-label adds the
* intent ("create provider") so screen readers don't just announce
* "Create Provider, menu" but "Create provider, choose type, menu".
*/}
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button