Repository navigation
Add TraceManager.RetainWords to keep trace text, not Words - #571
johnml1135 wants to merge 1 commit 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:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| return word?.GetTraceText(IsAnalysis(_type)); | ||
| } | ||
|
|
||
| internal static bool IsAnalysis(TraceType type) |
There was a problem hiding this comment.
FYI: F1. IsAnalysis omits CompoundingRuleAnalysis, so that node's public InputText is the plain form, while the InputText doc promises the regular-expression form for analysis node types. Keeping CLI output byte-identical is a fair reason; name the exception in the doc so a library caller is not surprised.
| /// types and the plain form otherwise. It is <c>null</c> when the node has no input. With retained words | ||
| /// it is computed from <see cref="Input"/> on each read. | ||
| /// </summary> | ||
| public string InputText => _inputText ?? FormatText(Input); |
There was a problem hiding this comment.
FYI: F2. With RetainWords = true, reading InputText/OutputText on a frozen word fills Word._traceTextRegex/_traceTextPlain, so parse results returned to the caller carry those strings for their lifetime. Harmless for the CLI, which runs lean; worth knowing for a retained-mode consumer that renders the trace.
| /// Gets or sets whether trace nodes keep the words they describe. When <c>true</c> (the default), | ||
| /// <see cref="Trace.Input"/> and <see cref="Trace.Output"/> hold the words, and some nodes hold clones. | ||
| /// When <c>false</c>, both are always <c>null</c> and each node keeps only <see cref="Trace.InputText"/> and | ||
| /// <see cref="Trace.OutputText"/>, so a trace no longer keeps every intermediate shape alive. The tree and |
There was a problem hiding this comment.
FYI: F3. "so a trace no longer keeps every intermediate shape alive" is history framing, which .claude/skills/code-comments/SKILL.md rule 2 forbids. Present tense works: "so the trace does not keep each intermediate shape alive".
| return count; | ||
| } | ||
|
|
||
| // The rule the CLI used before it read InputText and OutputText. |
There was a problem hiding this comment.
FYI: F4. "The rule the CLI used before it read InputText and OutputText." is history framing (.claude/skills/code-comments/SKILL.md rule 2). Say what it is now, e.g. "An independent copy of the display-form rule, to check InputText against."
Public API: adds Findings: F1 new, F2 new, F3 new, F4 new. Reviewed at 9f7828e |
Start here:
src/SIL.Machine.Morphology.HermitCrab/TraceManager.cs--RetainWordsand theNode(...)helper that every trace callback now goes through.Status: open; waits on #534, which rewrites most of the same callbacks. CI green on this head (the
claudejob is skipped).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.Stack and verification: lands after #534 (open); this rebases over it. 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). Not run: the full./local_check.sh --agent-strict; no coverage run.Next: approve the opt-in switch, or tell me to wait for #534 and I will rebase.
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.Interaction with open PR #534
#534 rewrites most of the same
TraceManagercallbacks andParseCommand.cs, so the two conflict textually; this one rebases over it. The fields #534 adds (FailureObject,Allomorph,BlockingEntry, slot fields, lookup counts) are small values or object references. To project them the same way, copy the strings or ids at the callback whenRetainWordsis false. ItsoutWordrejection nodes would takeNode(..., input: outWord), which keeps only text in lean mode. ItsConditionalWeakTable<Word, Trace>keys only the lookup input and holds noWordin a node.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