Tools: HF-24 close 7 review findings from A+C parallel review

Addresses the must-fix + should-fix findings from the parallel
code-review-quality (A) and sherlock-review (C) passes on PR #1665.

**P0 — `tryAccountingFormat` sign-loss bug** (A's only must-fix bug):
docs adapter `customStringifyCurrency` in `currency-handling.md`
mis-rendered negative values when the negative section was plain
`$#,##0.00` (no parens). `Math.abs(value)` was formatted without ever
adding the `-` prefix, so `value = -1234.5` against format
`"$#,##0.00;$#,##0.00"` returned `$1,234.50` (positive-looking) instead
of `-$1,234.50`. New branch: `parenMatch ? "(" + formatted + ")" : "-" + formatted`.
Regression test for this path landed in hyperformula-tests `85979a0`.

**`extract-doc-snippets.js` hardening** (C's main concerns):
- Detect malformed `<!--snippet:NAME-->` markers (no spaces around the
  body) and fail loudly instead of silently skipping. Pre-fix, a typo
  in a docs author's marker meant the snippet was silently dropped from
  `test-utils/snippets/` while `snippets:check` still passed — drift
  undetected.
- Skip symbolic links during `docs/` recursion (loop / sandbox-escape
  guard).
- Cap recursion at `MAX_DEPTH = 8`.
- Sort `readdirSync` output for deterministic generated content across
  platforms (CI-determinism, no more "works on my Mac" drift).
- Prune orphan `*.generated.ts` files when a snippet is renamed or
  removed (otherwise rename leaves a tracked stale file behind, which
  `snippets:check` still treats as drift-free because it just looks for
  new diffs).
- Render generated file with a `// @ts-nocheck` pragma so the body
  (verbatim untyped JS from the docs snippet) doesn't block the
  TypeScript compile when consumed.
- Extra blank line between banner and content for grep-friendliness.

**CI ordering** (A's should-fix): `npm run snippets:check` now runs
AFTER `npm run lint` in `.github/workflows/lint.yml` — an extractor
crash on a future malformed marker no longer masks a real lint failure
as a docs-tooling failure.

**tsconfig.test.json**: `test-utils` added to `include` so the
generated snippet IS in the test build graph (combined with
`@ts-nocheck`, the file is reachable for `import` without breaking
compile).

Together these close findings B5 (duplicate-reply pattern is documented
in memory as part of the same review), plus all the should-fix items A
and C surfaced on the codegen MVP. The follow-up A path (expand docs
adapter so the test inline copy can be replaced by an `import` from
`test-utils/snippets/currency-adapter.generated`) remains a product
decision for Sequba and is tracked in [[project_hf_24_followups]].

Author: marcin-kordas-hoc

.github/workflows/lint.yml

@@ -44,8 +44,10 @@ jobs:
       - name: Install dependencies
         run: npm ci
 
-      - name: Check docs snippets are in sync with extracted source-of-truth
-        run: npm run snippets:check
-
       - name: Run linter
         run: npm run lint
+
+      - name: Check docs snippets are in sync with extracted source-of-truth
+        # After lint so an extractor crash doesn't mask lint failures
+        # downstream consumers care about.
+        run: npm run snippets:check

docs/guide/currency-handling.md

@@ -150,6 +150,7 @@ const CURRENCY_RULES = [
 ]
 
 // Accounting: $#,##0.00;($#,##0.00) — positive;negative with parentheses
+// (or $#,##0.00;$#,##0.00 — both sections plain, sign rendered explicitly)
 function tryAccountingFormat(value, format) {
   const sections = format.split(';')
   if (sections.length !== 2) return undefined
@@ -166,7 +167,8 @@ function tryAccountingFormat(value, format) {
     maximumFractionDigits: fractionDigits,
   })
   const formatted = nf.format(Math.abs(value))
-  return isNegative && parenMatch ? `(${formatted})` : formatted
+  if (!isNegative) return formatted
+  return parenMatch ? `(${formatted})` : `-${formatted}`
 }
 
 export const customStringifyCurrency = (value, currencyFormat) => {

script/extract-doc-snippets.js

@@ -19,10 +19,20 @@
  * The script intentionally has zero npm deps so it runs in the same Node we
  * use for `compile` without adding to package.json.
  *
+ * Constraints:
+ *   - Snippet bodies must NOT contain nested triple-backtick fences. The
+ *     closing fence matcher accepts any `^```\s*$` line, so a nested fenced
+ *     block inside a snippet would terminate the outer match early.
+ *   - Symlinks under `docs/` are not followed (loop / sandbox-escape guard).
+ *   - Recursion depth is capped at MAX_DEPTH to prevent runaway walks.
+ *   - File enumeration order is stabilised via sort() so generated content
+ *     is byte-identical across platforms (CI-determinism).
+ *
  * Exit codes:
- *   0 — snippets extracted successfully
- *   1 — duplicate snippet name across files, mismatched markers, or other
- *       structural error
+ *   0 — snippets extracted successfully (or no markers present)
+ *   1 — any structural error: malformed marker, mismatched markers,
+ *       duplicate name, fence opened-but-not-closed, marker mismatch,
+ *       or missing docs dir
  */
 'use strict'
 
@@ -32,30 +42,60 @@ const path = require('path')
 const REPO_ROOT = path.resolve(__dirname, '..')
 const DOCS_DIR = path.join(REPO_ROOT, 'docs')
 const OUT_DIR = path.join(REPO_ROOT, 'test-utils', 'snippets')
+const MAX_DEPTH = 8
 
-/** Recursively list every `.md` file under `dir`. */
-function listMarkdown(dir) {
+/**
+ * Recursively list every `.md` file under `dir`. Skips symlinks and bails
+ * past MAX_DEPTH so a stray loop under `docs/` can't hang the build.
+ * Entries are sorted at every level for deterministic ordering across
+ * platforms / filesystems.
+ */
+function listMarkdown(dir, depth = 0) {
+  if (depth > MAX_DEPTH) {
+    console.error(`extract-doc-snippets: depth limit (${MAX_DEPTH}) exceeded under ${dir} — refusing to recurse further`)
+    return []
+  }
+  const entries = fs.readdirSync(dir, { withFileTypes: true })
+    .filter((e) => !e.isSymbolicLink())  // never follow symlinks
+    .sort((a, b) => a.name.localeCompare(b.name))
   const out = []
-  for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+  for (const entry of entries) {
     const p = path.join(dir, entry.name)
-    if (entry.isDirectory()) out.push(...listMarkdown(p))
+    if (entry.isDirectory()) out.push(...listMarkdown(p, depth + 1))
     else if (entry.isFile() && entry.name.endsWith('.md')) out.push(p)
   }
   return out
 }
 
+/** Throw with a precise diagnostic when a line looks like a snippet marker
+ *  but doesn't match the strict grammar (open or close). Catches typos such
+ *  as `<!--snippet:foo-->` (no spaces) which the strict regex skips silently. */
+const malformedSniffRe = /<!--\s*\/?\s*snippet\b/i
+
 /** Parse a markdown file and yield { name, lang, code, sourceFile, line }. */
 function * extractSnippets(filePath) {
   const text = fs.readFileSync(filePath, 'utf8')
   const lines = text.split('\n')
-  const openRe = /^<!--\s*snippet:([a-zA-Z][a-zA-Z0-9_-]*)\s*-->\s*$/
-  const closeRe = /^<!--\s*\/snippet:([a-zA-Z][a-zA-Z0-9_-]*)\s*-->\s*$/
+  const openRe = /^<!--\s+snippet:([a-zA-Z][a-zA-Z0-9_-]*)\s+-->\s*$/
+  const closeRe = /^<!--\s+\/snippet:([a-zA-Z][a-zA-Z0-9_-]*)\s+-->\s*$/
   const fenceRe = /^```([a-zA-Z0-9]*)\s*$/
 
   let i = 0
   while (i < lines.length) {
-    const openMatch = openRe.exec(lines[i])
-    if (!openMatch) { i++; continue }
+    const line = lines[i]
+    const openMatch = openRe.exec(line)
+    if (!openMatch) {
+      // Distinguish "not a marker at all" from "looks like a malformed marker".
+      if (malformedSniffRe.test(line) && !closeRe.exec(line)) {
+        throw new Error(
+          `${filePath}:${i + 1} — line looks like a snippet marker but doesn't match the grammar. ` +
+          `Required form: \`<!-- snippet:NAME -->\` (note the spaces around the marker body). ` +
+          `Got: \`${line.trim()}\``,
+        )
+      }
+      i++
+      continue
+    }
 
     const name = openMatch[1]
     const startLine = i + 1
@@ -103,12 +143,32 @@ function render({ name, lang, code, sourceFile, line }) {
     '// Edit the source markdown then run `npm run snippets:extract`.',
     '// CI fails if this file drifts from the source.',
     '',
+    // Docs snippets are written in JS without TypeScript annotations (they
+    // need to copy-paste runnable for the reader). The .ts extension lets
+    // consumers `import { … }` without a tsconfig.allowJs change, but the
+    // body is intentionally untyped — `@ts-nocheck` keeps tsc quiet without
+    // forcing every snippet author to learn TypeScript.
+    '// @ts-nocheck',
+    '',
   ].join('\n')
   // Snippets in docs are JS for readability; the generated `.ts` file consumes
   // them as TypeScript. Preserve the original content byte-for-byte.
   return banner + code + (code.endsWith('\n') ? '' : '\n')
 }
 
+/** Remove `*.generated.ts` files in OUT_DIR that aren't in `keep`. */
+function pruneOrphans(keep) {
+  if (!fs.existsSync(OUT_DIR)) return []
+  const removed = []
+  for (const name of fs.readdirSync(OUT_DIR).sort()) {
+    if (!name.endsWith('.generated.ts')) continue
+    if (keep.has(name)) continue
+    fs.unlinkSync(path.join(OUT_DIR, name))
+    removed.push(path.relative(REPO_ROOT, path.join(OUT_DIR, name)))
+  }
+  return removed
+}
+
 function main() {
   if (!fs.existsSync(DOCS_DIR)) {
     console.error(`extract-doc-snippets: docs dir not found at ${DOCS_DIR}`)
@@ -118,6 +178,7 @@ function main() {
 
   const seen = new Map()
   const written = []
+  const keepBasenames = new Set()
   for (const file of listMarkdown(DOCS_DIR)) {
     for (const snippet of extractSnippets(file)) {
       if (seen.has(snippet.name)) {
@@ -128,18 +189,28 @@ function main() {
         process.exit(1)
       }
       seen.set(snippet.name, snippet)
-      const outPath = path.join(OUT_DIR, `${snippet.name}.generated.ts`)
+      const basename = `${snippet.name}.generated.ts`
+      const outPath = path.join(OUT_DIR, basename)
       fs.writeFileSync(outPath, render(snippet))
       written.push(path.relative(REPO_ROOT, outPath))
+      keepBasenames.add(basename)
     }
   }
 
-  if (written.length === 0) {
+  const removed = pruneOrphans(keepBasenames)
+
+  if (written.length === 0 && removed.length === 0) {
     console.log('extract-doc-snippets: no <!-- snippet:NAME --> blocks found')
     return
   }
-  console.log(`extract-doc-snippets: wrote ${written.length} file(s)`)
-  for (const w of written) console.log(`  ${w}`)
+  if (written.length > 0) {
+    console.log(`extract-doc-snippets: wrote ${written.length} file(s)`)
+    for (const w of written) console.log(`  ${w}`)
+  }
+  if (removed.length > 0) {
+    console.log(`extract-doc-snippets: pruned ${removed.length} orphan(s)`)
+    for (const r of removed) console.log(`  - ${r}`)
+  }
 }
 
 main()

test-utils/snippets/currency-adapter.generated.ts

@@ -2,6 +2,8 @@
 // Source: docs/guide/currency-handling.md:111 (snippet:currency-adapter)
 // Edit the source markdown then run `npm run snippets:extract`.
 // CI fails if this file drifts from the source.
+
+// @ts-nocheck
 // Minimal Excel-format-string → Intl.NumberFormat adapter.
 // Extend the LCID_TO_LOCALE map and CURRENCY_RULES list to cover more formats.
 
@@ -42,6 +44,7 @@ const CURRENCY_RULES = [
 ]
 
 // Accounting: $#,##0.00;($#,##0.00) — positive;negative with parentheses
+// (or $#,##0.00;$#,##0.00 — both sections plain, sign rendered explicitly)
 function tryAccountingFormat(value, format) {
   const sections = format.split(';')
   if (sections.length !== 2) return undefined
@@ -58,7 +61,8 @@ function tryAccountingFormat(value, format) {
     maximumFractionDigits: fractionDigits,
   })
   const formatted = nf.format(Math.abs(value))
-  return isNegative && parenMatch ? `(${formatted})` : formatted
+  if (!isNegative) return formatted
+  return parenMatch ? `(${formatted})` : `-${formatted}`
 }
 
 export const customStringifyCurrency = (value, currencyFormat) => {

tsconfig.test.json

@@ -7,7 +7,7 @@
     "sourceMap": true
   },
   "extends": "./tsconfig",
-  "include": ["src", "test"],
+  "include": ["src", "test", "test-utils"],
   /* Exclude files that are specific for jest setup */
   "exclude": ["test/_setupFiles/jest"]
 }