Skip to content

Strengthen Note HTML smoke assertions - #93

Merged
kiki830621 merged 2 commits into
mainfrom
codex/note-html-smoke-assertions
May 25, 2026
Merged

kiki830621 merged 2 commits into
mainfrom
codex/note-html-smoke-assertions

Conversation

@kiki830621

Copy link
Copy Markdown
Member

Summary

  • Assert Note→HTML output includes a media/ directory with at least one rendered asset.
  • Add a 750 KB index.html size floor based on the current .note fixture reference render.
  • Keep existing HTML shell assertion as a basic sanity check.

Verification

  • swift test --filter NoteHTMLConvertTests
  • swift test --filter Note
  • git diff --check

Refs #86

@kiki830621

Copy link
Copy Markdown
Member Author

Verify Report — PR #93

Engine

6-AI ensemble: 5 general-purpose Agents (Claude reviewers, file-based output) + Codex (gpt-5.5 xhigh, run_in_background). All 5 findings files present + non-empty; no Recovery Protocol invoked.

Aggregate

PASS — 0 blocking, 1 in-scope nit, 3 follow-up findings. Merge recommended after the nit fix (or as-is with the message wording change tracked separately).

Scope coverage


#86 — strengthen Note→HTML smoke assertions (media/ dir + size floor)

Requirements coverage: 2/2 mandatory FULLY addressed; 1/1 optional NOT addressed (reasonable trade-off, documented in issue Key Decisions).

# Severity Finding Source Action
1 MEDIUM Assertion message "media/ SHALL contain at least one stroke, image, or audio asset" is factually wrong about strokes — NoteConverter.swift:50-67 confirms strokes are JSON-embedded in index.html, never written as files in media/. Drop "stroke" from message. agents:logic+devils-advocate In-scope nit (single-line edit)
2 HIGH CI does not exercise the new assertions. test-files/ is .gitignored (.gitignore:103); CLITestHelper.noteFixture() XCTSkips when fixture absent (CLITestHelper.swift:75). On clean clone / GitHub Actions, the strengthened test silently passes via skip — defeats the empty-shell regression detection that motivated #86. agents:codex+devils-advocate Follow-up (overlaps with #79 committable-fixture work)
3 LOW Cross-issue dependency: 750 KB floor is calibrated to the current 33 MB local fixture. If #79 lands a committable mini-fixture (likely < 100 KB), this assertion immediately fails and requires rebaselining. agents:devils-advocate Follow-up (note in PR description or #79 body)
4 LOW Issue body explicitly asks "evaluate whether stroke-count parsing is worth the brittleness vs. size-floor proxy". Author's reasoning is in Current Status Key Decisions but not in PR description / commit body — future maintainers won't see the rationale without digging into the issue. agents:devils-advocate Follow-up (PR description add-on)

Note on apparent disagreement: Sibling Requirements reviewer marked #1 "FULLY"; Devil's Advocate partially refuted as "PARTIAL" on the grounds the wording-vs-converter mismatch creates a contract/fixture coupling issue. Resolution: the checklist items in #86 are satisfied (PASS), but the assertion message is factually wrong about strokes. Treat as in-scope nit rather than re-classify the Requirements verdict.

Scope Check

No scope creep. Diff strictly adds 3 assertion blocks to one test method in NoteHTMLConvertTests.swift (+26/-0, 1 file). No production code, no helper API surface change, no Package.swift change, no dependency additions.

Security

No findings. Test-only change, trusted-fixture trust model preserved. Existing Data(contentsOf:) reads remain bounded by converter output size (~1.57 MB baseline). No hardcoded secrets.

Process Gaps

None. 5/5 reviewer findings produced; no Recovery Protocol triggered; no sentinel files.


Recommendation

Merge as-is OR with 1-line message fix. No blocker. Suggested follow-up actions:

  1. In-scope nit — change line XCTAssertGreaterThanOrEqual(mediaFiles.count, 1, "media/ SHALL contain at least one stroke, image, or audio asset") → "... at least one image or audio asset" (drop "stroke" — strokes are JSON-embedded in index.html, never in media/). Can land in same PR or a follow-up commit.
  2. Follow-up Read .claude guidance for upcoming work #1 — file a new issue (or comment on refactor: extract pdf-to-latex-swift + ocr-swift to PsychQuant repos; commit Tests/WordToMDTests fixtures (continues #78 pattern) #79) noting that without a committable fixture, this test silently XCTSkips in CI and defeats test: strengthen Note→HTML smoke assertions (media/ dir + size floor) #86's empty-shell regression detection. The mini-fixture work in refactor: extract pdf-to-latex-swift + ocr-swift to PsychQuant repos; commit Tests/WordToMDTests fixtures (continues #78 pattern) #79 unblocks this assertion's CI value.
  3. Follow-up feat: AIConfig — AI CLI 工具設定系統 #2 — when refactor: extract pdf-to-latex-swift + ocr-swift to PsychQuant repos; commit Tests/WordToMDTests fixtures (continues #78 pattern) #79 lands a small fixture, the 750 KB floor must be rebaselined to the new fixture's reference render. Note the coupling in refactor: extract pdf-to-latex-swift + ocr-swift to PsychQuant repos; commit Tests/WordToMDTests fixtures (continues #78 pattern) #79 body or in a comment under test: strengthen Note→HTML smoke assertions (media/ dir + size floor) #86.
  4. Follow-up feat: LaTeXNormalizer — 機械式 LaTeX 清理 #3 — author may optionally edit PR description to paste the "size-floor proxy chosen over brittle SVG parsing" rationale from issue Current Status, so future readers see the trade-off without digging.

Strokes are JSON-embedded in index.html via NoteConverter.buildJSON(),
never written as files in media/. The media/ count assertion only
catches missing audio/image assets — the size-floor assertion is what
catches the strokes-drop regression.

Refs #86
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant