Repository navigation
Add TraceManager.RetainWords to keep trace text, not Words - #571
Open
johnml1135 wants to merge 2 commits into
Open
johnml1135 wants to merge 2 commits into
johnml1135 wants to merge 2 commits into
Conversation
With RetainWords false, trace nodes store the shape text of their input and output (InputText, OutputText) instead of Word references and clones. The default is unchanged and the CLI turns lean mode on by setting it false. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #571 +/- ##
==========================================
+ Coverage 74.57% 75.18% +0.60%
==========================================
Files 453 453
Lines 38342 38361 +19
Branches 5284 5297 +13
==========================================
+ Hits 28593 28840 +247
+ Misses 8583 8379 -204
+ Partials 1166 1142 -24 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
Contributor
Public API: adds Findings: F1 addressed, F2 addressed, F3 addressed, F4 addressed. Reviewed at 3f45fbe |
This branch has not been deployed
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
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.
Start here:
src/SIL.Machine.Morphology.HermitCrab/TraceManager.cs--RetainWordsand theNode(...)helper that every trace callback now goes through.Status: open; stands alone on master (#534 was closed). Review fixes F1–F4 are in 3f45fbe.
TraceManagergets an opt-inRetainWordsswitch. Set it to false and each trace node keeps shape text (InputText/OutputText) instead of theWord. On Mbugwe yɔɔna the retained trace falls from 1,035.0 MiB to 22.6 MiB with the same 120,147 nodes. The default stays true, the search is untouched, and the HermitCrab CLI (HCContext) sets it false. Closes #563.What a reviewer arrives wondering: does lean mode change what is printed or parsed? Node-by-node tests say no; the CLI output was byte-identical on Indonesian (timing lines aside, one processor).
Where to look:
LeanTrace_MatchesFullTraceNodeForNode.Word; parses are equal with tracing off, full and lean --LeanTrace_KeepsNoWords,ParseWord_IsIdenticalWithTracingOffFullAndLean.Wordand is not copied to clones --FrozenWord_CachesItsTraceTextAndCopiesDoNotInherit.Trace.Input/Outputare null in lean mode; callers readInputText/OutputText, which work in both modes.Not here:
Trace.Input/Outputare public, so that is a breaking choice for the owner.Verification: run on this branch:
dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests -c Release(0 failed),dotnet csharpier check .andpwsh -NoProfile -File ./scripts/comment-hygiene.ps1 -BaseRef origin/master(both clean). The full./local_check.sh --agent-strictalso passes at 3f45fbe; no coverage run.Next: approve the opt-in switch.
Reading this a year from now -- start here
Every trace callback used to store the
Wordit was handed, or a deepClone()of it (six places), and the tree is never trimmed. AWordowns its shape, feature structures and dictionaries, so the trace held about 17 KB per node. Both real consumers read only strings: the CLI prints the shape text, and FieldWorks' production code does not useTraceManager(it implementsITraceManagerin its own trace class; not checkable from this repository).With
RetainWords = false,Trace.InputandTrace.Outputare always null andInputText/OutputTextare filled when the node is made. The text follows the old CLI rule: regular-expression form for the analysis node types, plain form otherwise. A frozenWordcaches each form once in_traceTextRegexand_traceTextPlain, which the copy constructor does not copy, so many not-unapplied nodes that share one input format it once. Unfrozen words are formatted fresh. In full mode the two properties format lazily fromInput/Output.What remains in lean mode is the node objects and two short strings each, about 190 bytes a node on yɔɔna (estimate from the totals, not a heap count).
Decisions, and why
CompoundingRuleAnalysiskeeps the plain form, because the old CLI switch did not list it as an analysis type. Byte-identical output wins over tidiness.Word, freed with it.CompoundingRuleAnalysistemp clone; phonological rule outputs, which the manager already cloned). None is mutated after its callback, and the full-vs-lean test covers all three, so lean text equals full text.Evidence
Retained trace MiB, full to lean, measured with the census context harness; same parse counts and node counts in every word compared:
Steps are unchanged: the census (its own trace manager) gives identical rows with and without this change on Indonesian and on yɔɔna (21,200 analysis events, 41,130 synthesis steps).
Coverage gap: compounding-rule synthesis failures and the
GenerateWordsroot, whichParseWordnever makes, are not covered by the new tests.🤖 Generated with Claude Code
This change is