Print the analysis as JSON with --json (and fix two defects it exposed) - #4
Merged
Conversation
produceGitLog lost commits nondeterministically, depending on how the OS
happened to split git's stdout into chunks. Measured against ground truth:
this repository returned 31 or 30 of 32 commits, and a 518-file repository
returned 393 or 387 against a true 420 — roughly 7% of its history. Every
metric Charlie produces is derived from that log, so hotspots, revisions,
SOC, coupling percentages, word counts and ownership have all been computed
from truncated input, in the HTML report as much as anywhere else. This
repository's own hotspot count rises from 14 to 21 with the fix in place.
Two independent causes: destructuring only two elements from
buffer.split('\n\n') discarded everything past the second, so a chunk
carrying three or more commits lost all but the first two; and onClose
resolved without draining the trailing buffer, while `git log
--pretty=format:` emits no trailing separator, so the final commit was
dropped whenever it was still buffered.
Rather than patch the buffer draining, remove it. A blank line is all a
separator ever was, so parseGitLog walks every line of the accumulated
text: header lines start a commit, non-empty non-header lines are file
entries on the current commit, blank lines are skipped. Both defects become
unrepresentable instead of fixed. Accumulating before parsing is not a
behavioural change — produceGitLog only ever resolved on close, so nothing
downstream consumed partial results — and parsing a 22.87 MB, 356,435-line
log costs ~100 ms against the ~10 s git itself takes.
The parser moves to the Core, which is what the FIXME on this function
already asked for: it is pure text-in, LogItem[]-out, and it went untested
against realistic input only because it was stuck in the Shell. It now sits
under the Core's 100% coverage and 95% mutation thresholds. The
error-reporting port the FIXME said this needed is onMalformedLine, an
effect the Core declares in its own terms, replacing a console.log(e) that
swallowed malformed file entries.
The existing test could not have caught either defect. It randomised chunk
sizes, so the intent was right, but its fixture joined commits with a
single newline and appended one blank-line separator at the very end —
exactly one separator, in the one position that triggers neither bug. It
now uses three commits in git's real format, with a comment saying why both
details matter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
coupling() re-scanned the entire pairs array once for every SOC file. On a repository with 6,762 SOC files and 1,211,043 pairs that is 8.2 billion comparisons, measured at 32.5 seconds — 77% of a 44-second run, against 24 milliseconds for parsing the same repository's log. Building a Map from file to partners in a single pass brings it to 165 ms, a 184x improvement, with byte-identical output verified on three repositories of very different sizes. This matters beyond the CLI. Until now data-loader.ts was the only caller, which means the HTML report has always stalled for around thirty seconds on load for a repository this size, in the browser, with no indication of progress. Anyone who tried Charlie on a large repository and concluded it had hung was right. One subtlety made the first attempt wrong. A file can appear twice in a single commit's numstat, producing a pair whose two sides are the same file; the old filter matched such a pair once, while an index keyed on both sides counts it twice. The guard is explicit and the case is pinned by a test that explains why it exists. The four characterisation tests added to coupling.test.ts were written before the rewrite and pass against the old implementation, which is what makes them a regression net rather than a description of the new code. coupledPairs() also built a throwaway Set per pair purely to size a union. Sizing it arithmetically as |A| + |B| - |A intersect B| allocates nothing and takes 950 ms down to 502 ms, again with identical output. An earlier version walked the smaller set first for a further 96 ms, but which set you walk cannot change the result, so no test could ever catch that branch breaking; it was removed rather than left permanently unverifiable, and the comment records the trade so it is not reintroduced. Removing it also lifts this file's standalone mutation score from a pre-existing 94.44% to 95.08%, which matters because CI mutates only the Core files a PR changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Charlie could only ever report through an HTML file, which is no use in a terminal and no use to an agent running the tool to read its own codebase. `charlie --json` now prints the analysis to stdout and writes nothing: charlie --json charlie --json /path/to/repo # path may precede or follow the flag charlie --json > analysis.json # stdout is JSON only The document has two keys, hotspots and coupling, passed through from the existing Core types with no reshaping, 2-space indented so it reads as-is. An object rather than a bare array, so word count or ownership can be added later without a breaking change. Progress output moves from stdout to stderr. Without that, the per-file `reading <path>` lines — 14 on this repository, hundreds on a real one — interleave with the JSON and make it unparseable. Progress belongs on stderr regardless; a terminal still shows it exactly as before. Argument parsing is new because there was none: process.argv[2] was taken as the repository path unconditionally, so any flag would have been resolved as a directory name. parseArgs is a pure Core function rather than commander, which is a listed but entirely unused dependency: at ten lines the decision stays in the Core under the mutation and coverage gates, and brings no help text or process.exit behaviour that CI cannot check. Unknown flags are ignored; validation would mean inventing error behaviour nobody asked for. A full analysis is unreadable — 8.5 MB and roughly 2.2 million tokens on a 518-file repository — so --json bounds it: the top 5% by percentile, a floor of 30 entries so small repositories are not reduced to one or two, and at most 10 coupled partners per file. That is 53 KB on the same repository. --all removes every bound, expressed as absent limits rather than a boolean threaded through the Core, so there is one code path. The README states what --all costs. Worth knowing about the bounds: with a floor of 30 the floor does the work on ordinary repositories, and above roughly 600 files the percentile starts to govern and grows the output as the repository grows, which is the opposite of what a token budget wants. Deliberate, and documented in the work notes. The coupling computation moves into the Core as couplingAnalysis and is now shared: data-loader.ts calls it instead of composing soc, coupledPairs and coupling itself. The bounds are applied by a separate jsonPayload, so the HTML report keeps showing everything. coupling-analysis re-exports the types its consumers need, so they no longer reach past the facade into its internals. socPercentile is deleted. It had no caller before this change or after, and its dependency on the new shared percentile module made soc.ts less stable than coupling.ts, which depends on it — a Stable Dependency Principle violation. The arithmetic it held is now shared through percentile.ts rather than duplicated, which was the point of keeping it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Explore, Challenge, Specify, Plan, Verification and Lessons for this branch, per DISCIPLINE.md. Deleted from main on merge by the docs-cleanup job. 05-verification.md records two things a reviewer should read rather than rediscover: the halt when Task 1's runtime checks exposed the git log data loss, and the Stable Dependency Principle failures that only appeared once the whole branch was assembled, because instability is a whole-graph property that per-task checks cannot see. 06-lessons.md carries nine nominations. The two worth a decision before the folder is erased are that Charlie's prior output was wrong and users may have acted on it, and the UTF-8 corruption at chunk boundaries that was measured and deliberately deferred. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 17, 2026
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.
Charlie could only report through an HTML file.
charlie --jsonnow prints the analysis to stdout so it can be read in a terminal — or by an agent running Charlie against its own codebase.Two keys,
hotspotsandcoupling, passed through from the existing Core types, 2-space indented. Bounded by default — the full analysis is 8.5 MB and ~2.2M tokens on a 518-file repo, which defeats the purpose — via percentile 0.95, a floor of 30 entries, and at most 10 partners per file. That gives 53 KB on the same repo.--allremoves every bound.Spec:
03-spec.md· Decisions:02-decisions.md· Evidence:05-verification.mdTwo defects this work exposed, both pre-existing
Neither was planned. Both were found by runtime verification and fixed with the driver's agreement.
Charlie was silently dropping commits.
produceGitLogreturned a different number of commits on identical runs, depending on how the OS split git's stdout. Against ground truth: 31 or 30 of 32 here; 393 or 387 against a true 420 on a 518-file repo — ~7% of its history. Two causes:buffer.split('\n\n')destructured only two elements and discarded the rest, andonClosenever drained the trailing buffer whilegit log --pretty=format:emits no trailing separator.Every metric Charlie has ever produced — HTML report included — was computed from a truncated log. This repository's own hotspot count goes from 14 to 21. Now exact and deterministic:
The buffer draining was deleted rather than patched: a separator is just a blank line, so the parser walks lines and both defects become unrepresentable. It moved to the Core, which is what the existing
FIXMEon that function asked for, so it now sits under the 100%-coverage and 95%-mutation gates.coupling()was quadratic. It re-scanned all 1,211,043 pairs once per SOC file — 8.2 billion comparisons, 32.5 s, 77% of a 44 s run. Indexed into aMapin one pass: 165 ms, 184x, byte-identical output on three repos.This was only ever called by
data-loader.tsbefore, which means the HTML report has always frozen for ~30 s on load for a repo this size, in the browser, with no progress indication.Verification
npm run verifygreen: 137 tests, Core coverage 100% statements/functions/lines and 97.91% branches, whole-Core mutation 97.66%, 0 clones, no dependency violations. Every new or changed Core file is at 100% mutation exceptcoupled-pairs.tsat 95.08%.Each commit compiles and tests green standalone (96 -> 100 -> 137 tests), so bisect works.
Things to push back on
socPercentilewas deleted. The driver chose it as the bounding mechanism, and its dependency on the new sharedpercentile.tsmadesoc.tsless stable thancoupling.ts, which depends on it — an SDP violation. The arithmetic is still shared rather than duplicated, which was the point; the named wrapper had no caller before or after.Owed before merge
coupling()fix, this also confirms large-repo report load is now fast.Lessons nominated
06-lessons.mdhas nine. Please decide on these before the folder is erased on merge:FIXMEnow carries the numbers.coupled-pairs.tswas already at 94.44%. Also: a mutant that cannot change observable output is a signal to delete the code, not to write a cleverer test.Also worth noting: I branched from a stale
mainand reportedcheck:depsas passing when that base had 6 violations — all already fixed by PR #3, which had merged that morning. Corrected by resetting ontoorigin/main; lesson 4 is "git fetchbefore branching".🤖 Generated with Claude Code