feat(font-system): attach docfonts verdict evidence to font reports#3672
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21856fa785
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
A report row's reason (bundled_substitute / category_fallback) says SuperDoc substituted, not how faithful it is - so a consumer can't tell Calibri -> Carlito (metric_safe) from Cambria -> Caladea (visual_only). Add an optional evidence field, present ONLY when SuperDoc rendered the recommended substitute. The projection is docfonts': bump @docfonts/fallbacks to 0.4.0 and read it off the resolved fallback instead of re-deriving locally: - buildFontReport copies it from getRenderableFallback (top-level verdict, all glyph exceptions). - buildFaceReport copies it from getRenderableFallbackForFace (per-face verdict, only that face's exceptions) - so Cambria Regular reads metric_safe with no Bold-Italic exception, Bold Italic reads visual_only and carries it. - Fields copied into SuperDoc's LOCAL ResolvedFontEvidence (evidenceId, policyAction, verdict, lineBreakSafe, glyphExceptions), so @docfonts/fallbacks never leaks into the emitted .d.ts. Additive and behavior-preserving: reason / missing / loadStatus unchanged; as_requested / custom_mapping / registered_face / fallback_face_absent carry no evidence.
caio-pizzol
force-pushed
the
caio-pizzol/verdict-aware-reporting
branch
from
21856fa to
e101be3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A font report row's
reason(bundled_substitute/category_fallback) tells you SuperDoc substituted, not how faithful the substitute is - so a consumer can't tell Calibri -> Carlito (metric_safe, a clean clone) from Cambria -> Caladea (visual_only, qualified by the Bold Italic grave accent). This adds the docfonts verdict to the report so that distinction is visible.New optional
evidencefield onFontResolutionRecord, populated ONLY when SuperDoc actually rendered the recommended substitute (bundled_substitute/category_fallback):evidenceId,policyAction,verdict,lineBreakSafe, andglyphExceptions.buildFontReport): the row's top-level worst-face verdict and all glyph exceptions.buildFaceReport): the per-face verdict (faceVerdicts[slot] ?? verdict) and only that slot's glyph exceptions - Cambria Regular readsmetric_safewith no Bold-Italic exception; Bold Italic readsvisual_onlyand carries it.SUBSTITUTION_EVIDENCE; local types only, so@docfonts/fallbacksnever leaks into the emitted.d.ts(the disciplinesubstitution-evidence.tsalready documents).Additive and behavior-preserving:
reason/missing/loadStatusare unchanged. Per the agreed scope, only rendered substitutes get evidence -as_requested/custom_mapping/registered_face/fallback_face_absentcarry none (an advisory/recommendation surface is a separate future field, since this one can't say "docfonts recommends X but it didn't render").Review: check
report.tsevidence sourcing + the face-slot filtering; the new tests inreport.test.tspin Calibri (metric_safe), Calibri Light (category_fallback/visual_only), Cambria family (visual_only + exceptions), Cambria Regular (metric_safe, no stray exception), Cambria Bold Italic (visual_only + its exception), and no-evidence for the four non-substitute reasons. Per repo rule I did not run the suite locally - CI is the gate; I checked that no other test asserts full substitute report rows (resolver tests assert resolver output, the integration tests assert names/config).