fix(forward): terminate resynced lecture files with a newline (#116) - #266
Merged
Conversation
The translator trims every model response and nothing on the forward path put a terminator back, so each resynced file was written with its last line unterminated — `\ No newline at end of file` in all 69 Track B PR diffs on lecture-python.zh-cn, dirtying the next edit's diff on each file and tripping POSIX-minded tooling. The newline is added in `finalizeResyncContent`, which already returns the exact bytes that get written (the structural-parity guard runs on its output), so the local write and the --github commit both inherit it from one place rather than each write site remembering. --test mode terminates its mock the same way so the smoke path exercises the same bytes. Narrow by construction: already-terminated content is byte-identical, trailing blank lines are preserved rather than collapsed, and empty content stays empty so it remains falsy and still skips the write. Not fixed here: `translate init` writes `injectHeadingMap(applyTypography(…))` of the same trimmed model output and carries the same defect — out of #116's scope, recorded in the .dev log entry as a sibling worth filing. The action's sync path was never affected (`reconstructDocument` has always ended with `.trim() + '\n'`), and forward's state files come from `yaml.dump`, which terminates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes translate forward resync output to always end with a trailing newline, preventing “No newline at end of file” noise in downstream diffs and tooling while keeping the change narrowly scoped to avoid altering already-terminated content, trailing blank lines, or empty outputs.
Changes:
- Added an
ensureTrailingNewlinehelper and applied it infinalizeResyncContentso both forward output routes (local write and--githubcommit) inherit the invariant. - Updated
--testmode output to be newline-terminated like the real resync bytes. - Added targeted Jest coverage for finalize + helper behavior and for on-disk/committed bytes, plus documented the fix in the changelog and dev log.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/cli/commands/forward.ts | Introduces ensureTrailingNewline and applies it at the end of finalizeResyncContent; aligns --test output with real write bytes. |
| src/cli/tests/forward.test.ts | Adds assertions that written/committed forward outputs end with a newline. |
| src/cli/tests/forward-finalize.test.ts | Adds finalize-level coverage for newline termination and unit tests for ensureTrailingNewline. |
| CHANGELOG.md | Records the user-visible fix under [Unreleased]. |
| .dev/log/2026-08-11-116-trailing-newline.md | Adds a maintainer log entry documenting the defect, fix location, and verification. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`format:check` is a separate CI step from `lint`; the new #116 test's multi-line call fits on one line under the repo's print width. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #232 (@LunaMeerkats) fixed the same defect independently and carried explicit tests that already-terminated content — CRLF included — survives byte-for-byte. The behavior was already guaranteed here (`endsWith('\n')` is true for `\r\n`), but the pin is worth keeping; adopted with credit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 11, 2026
mmcky
added a commit
that referenced
this pull request
Aug 11, 2026
Copilot review: the #266 entry's tail clause ("carries the same defect and is not addressed here") was written when the init fix did not exist and is contradicted by the init entry directly above it. Both entries are still under [Unreleased], so the stale clause is rewritten in place to point at the sibling entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky
added a commit
that referenced
this pull request
Aug 11, 2026
* fix(init): terminate seeded lecture files with a newline The sibling of #116, found while fixing it and recorded there as out of scope: the translator trims the model's response and neither applyTypography nor injectHeadingMap restores the terminator, so every file `translate init` seeded was written with its last line unterminated — the same state that put `\ No newline at end of file` into all 69 Track B diffs. Fixed the same way as forward (#266): `ensureTrailingNewline` applied where `finalContent` is computed, before the structural-parity guard, so the guard checks the exact bytes that get written. The helper is imported from commands/forward.ts rather than duplicated (cross-command import follows the existing forward -> status precedent; no cycle — nothing forward imports reaches init). When #172's finalizeTranslatedDocument choke point lands, both callers collapse into it. With this, all three writers of model output terminate their files: sync (reconstructDocument, always did), forward (#266), init (this change). Regression test drives the real translateLecture write path with trimmed mock output and fails on the pre-fix source. dist-action/ is unchanged — the bundle entry is the action, which does not include the CLI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: the forward changelog entry no longer says init is unaddressed Copilot review: the #266 entry's tail clause ("carries the same defect and is not addressed here") was written when the init fix did not exist and is contradicted by the init entry directly above it. Both entries are still under [Unreleased], so the stale clause is rewritten in place to point at the sibling entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 11, 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.
Fixes #116.
What was wrong
translator.tstrims every model response, and nothing on the forward path put a terminator back — so each resynced file was written with its last line unterminated. The Track B wave put\ No newline at end of filein all 69 PR diffs onlecture-python.zh-cn, which dirties the next edit's diff on every one of those files and trips POSIX-minded tooling.The fix
ensureTrailingNewlineis applied at the end offinalizeResyncContent, not at the two write sites. Finalize already returns the exact bytes that get written — the structural-parity guard runs on its output, and the comment there says so — so one call covers both output routes (the local write and the--githubcommit) and the invariant does not depend on a future writer remembering.--testmode terminates its mock output the same way, so the smoke path exercises the same bytes. When #172'sfinalizeTranslatedDocumentlands, this moves with finalize rather than being re-derived.Narrow on purpose: content that already ends with a newline is byte-identical, trailing blank lines are preserved rather than collapsed to one, and empty content stays empty so it remains falsy and still skips the write instead of becoming a one-byte file.
Verification
The new tests fail on the pre-fix source and pass after — checked by reverting
commands/forward.tsand re-running, not assumed. They cover the local-write bytes, the--githubcommitted bytes, finalize terminating trimmed model output without doubling an existing newline, and the helper's four cases. Full suite 1508/1508; lint clean.dist-action/is unchanged because the bundle's entry is the action (src/index.ts), which does not include the CLI.Sibling defect, deliberately not fixed here
translate inithas the same defect.init.ts:302writesinjectHeadingMap(applyTypography(<trimmed model output>), …), and I verified against the builtdist/that neither step adds a terminator — so every seeded edition file is written unterminated too. That is outside #116's scope (its title and body are about the forward write path), so it is recorded in the.dev/log entry as a sibling worth filing; the fix is the same one-liner. Happy to fold it in here if you would rather it ship together.Two paths that are already fine, for the record: the action's sync path has always ended
reconstructDocumentwith.trim() + '\n', and forward's.translate/state/files come fromyaml.dump, which terminates.🤖 Generated with Claude Code