Give back the file that was given, past the values that were edited - #48
Merged
Conversation
Closes #47. A statement's region ends at its terminator, so a comment after the semicolon on the same line sat in the gap. That is invisible while the object is copied, because the gap is copied too, and wrong the moment it is reformatted: the writer emits its own field comment and the author's then arrives from the gap on the line below. It is not even a duplicate, because the ordinary writer drops the unit, so `!- North Axis {deg}` reads as a stray fragment under `!- North Axis`. The comment written on this file's walk defended the old behaviour on the ground that the alternative is a writer guessing which comments belong to which object. That defence is right about the general case and does not apply to this one: "on the same line as the terminator" is a positional fact, and it is the one case where the owner is not in question. The writer had just emitted its own version of the very comment it then copied. It reads as rarer than it is. IDFEditor writes a field comment on every line of every object including the terminator line, so about half the statements in a typical file gained a stray line the moment they were edited. THE FIRST LANGUAGE ALREADY DID THIS. Its concrete syntax tree runs a node to the end of the line, so it absorbed the comment and replaced it, and it drops the comment on removal too. This is TypeScript catching up rather than a new rule, which is why no divergence entry is added and none goes stale. Three consequences, decided rather than discovered: - A file written unchanged is byte-identical still: the extent grows and the gap shrinks by exactly the same characters. - Removing an object takes that comment with it, where it used to be left on a line of its own describing a field that no longer exists. The test that pinned the old behaviour now pins this one. - A comment on the next line, or after a blank one, stays in the gap. That is where deciding which object a comment belongs to actually becomes a guess.
Supersedes the first attempt at #47, which deleted the comment instead. That attempt absorbed a terminator-line comment into the statement so it could not arrive twice, and reached agreement with Python by matching it on the more destructive of two behaviours. A reader pointed out that the example used `!- North Axis {deg}`, which the writer can regenerate, so deletion and regeneration looked identical. A genuinely custom comment told them apart: before, as shipped in 0.3.0-rc.1: survives, on a line of its own below after that attempt: deleted now: kept, in place, once An edit asks for a value to be re-rendered and not for the object's comments to be rebuilt. Rebuilding them destroyed the field's unit, which the generated label does not carry, and any note the author wrote there. `writeObject` takes the author's comment per field where there is one, and the layer supplies them: a field's comment is the one after its delimiter on the same line, which is the same positional rule the terminator case established, generalised. A comment on its own line belongs to no field and stays in the gap. This subsumes the duplicate. The comment is emitted once, by the writer, in the place it was written, so there is nothing left in the gap to arrive below it.
A reformatted object grew the file by a blank line, and would grow it again on every save. A statement's extent ends at its terminator, or at the comment on that same line, and in neither case includes the line break: the break is the first character of the gap. `writeObject` ends with one because it also writes whole documents, so the break went in twice. This was there before the comment work and was invisible. The misplaced terminator comment sat between the two breaks, so it read as one blank line; removing the comment left the second break with nothing in front of it. Found by a consumer measuring a one-field edit: 476 lines out against 475 in, on a file where the requirement being written against is that a one-object edit changes only the lines of that object. A blank line after the object is a line outside it.
Mirrors idfkit's change of the same name; the two behave identically on every case measured. A comment on its own line INSIDE an object was lost when the object was reformatted. One between two objects is carried by the gap and was always safe; one inside is carried by nothing. It is now emitted with the field below it. A field the author left bare gained a generated label. Absence is as much a thing the author wrote as the words are, so bare stays bare, and `fieldComments: 'generate'` is the escape hatch for a caller who wants the file annotated: it adds labels and never costs a comment line. `changedObjects()` answers which objects a preserving write will rewrite, which a consumer cannot derive: a rename clears the record on every object that referred to the renamed one, so counting from an edit log reports one where the answer is nine. CI is red until governance-2026.15 is published and pinned. The naming gate reads the built surface and fires on the change that adds a name, which is the register-lands-first rule doing its job.
It yields objects a write will REWRITE, and a consumer reading the old comment could take it for everything that will differ. A removal separates the two: the removed object is no longer in the document to be yielded, so this returns nothing for a write that changes the file, and treating an empty result as 'the file is unchanged' is wrong on every removal. Comparing the write with rawText is the question that answers; this one is how much is being written afresh. Found by a consumer whose save outcome already did it correctly and who pointed out the next one would not.
A reformatted object was written one value per line whatever the source said. Measured across the 693 EnergyPlus 22.1.0 example files, that is not a corner case: 21.5 percent of statements group several values on a line, 690 of the 693 files contain at least one, and a full reformat of the corpus would add 20.2 percent to its line count with 89.6 percent of that from this one shape. A four-line surface became twelve lines. `FieldAnnotation` gains `startsLine`, which the delimiter scan already knew and was discarding, and the emitter builds a line rather than pushing one per field. Editing one wall of 1ZoneUncontrolled leaves the file at 462 lines rather than 470. The type name is the line under construction rather than something pushed ahead of it, so an object written `Timestep,4;` comes back on one line. That is another 11.3 percent of statements and the case that surprises on a file with no geometry in it: nobody thinks of `Timestep,4;` as formatting they chose. That restructuring broke `comments: false` output, which emitted the type name last and no longer parsed. The corpus caught it; the test that granted the output form now reparses what it got.
samuelduchesne
changed the title
…hor wrote Two defects a peer found by forcing 263,471 objects from the 693 EnergyPlus 22.1.0 example files through the writer and attributing every difference. Both predate the preserving writer; both are the kind that only shows once a write is expected to give the file back. THE COLUMN. `commentColumn` is documented as a column and was applied as an index, so `!-` landed one place right of where the files it imitates put it. Across 1,504,802 comment lines whose content came back byte-identical, 91 percent moved by exactly one. `1ZoneUncontrolled.idf` writes the marker at index 29 on 223 of its 231 commented lines; this writer wrote 30. On a preserving write that is the difference that shows. A rewritten object's comments stood one column clear of every untouched object around it, so every save left a visible seam at the edit. THE BLANKS. The writer stops at the last field that is SET, so a run of commas the author wrote out is dropped and the field-name comments go with them. One `Sizing:System` went from 38 lines to 22 on a single-field edit; corpus-wide it is 20,571 lines, more than any other difference a rewrite makes, led by ComponentCost:LineItem at 4,548 and Coil:Heating:Water at 2,475. A field written out as a blank is as much a thing the author wrote as a field left bare of its comment, and that second rule is one this path already follows. Reaching the opposite answer on the first was an inconsistency, not a decision. Only on the preserving path: the annotations are the author's own field count and are absent everywhere else, so a write with nothing to reproduce trims as it always has. The extensible rule still wins over both, since a missing fixed slot lands every group in the wrong position. The tutorial's rendered output moves one column with everything else.
`changedObjects()` was published for a consumer that could not use it. Turning an edit into the smallest possible change to a file takes three things: WHICH objects will be rewritten, WHAT text each becomes, and WHERE the old one was. The first two were public. The third was reachable only through the anchoring, which is internal on purpose, so a consumer had to write the whole file and diff it, which is the work `changedObjects()` exists to avoid. Registered in idfkit/idfkit-conformance#6, and found by the language server team reading the branch before it merged, which is what asking them to read it was for. Two things it is NOT. It is not `SOURCE`. That symbol is cleared the moment an object is touched, because its absence is what marks the object for rewriting, so it is useless for locating exactly the objects worth locating. `ORIGIN` is the same number recorded once and never cleared, and `regionOf` still checks anchor identity, so an object carrying an index from a file it is no longer in is not handed a range from this one. It is not `statement.region`. The extent a preserving write replaces reaches past the semicolon to a comment on the terminator's own line, which is the last field's comment. A consumer replacing the shorter range would leave that comment behind, describing a field that had just moved, which is the defect `extentEnds` exists to close. Handing out a range the writer does not use would have reopened it outside the writer. Computed once per document, since the retained source does not change after the read. What the method does NOT settle is where the replacement TEXT comes from, and `writeObject` is not the answer: a preserving write hands it the author's own per-field annotations, which are internal, so options built by hand come back with the author's units and notes as generated labels. The doc comment says so rather than leaving a consumer to discover it. That gap is open.
`regionOf` told a consumer where to put text it could not correctly generate. The
one public function shaped to fill the hole is `writeObject`, and a preserving
write hands that function the author's own per-field annotations, which are
internal, so a caller building options by hand gets the author's units and notes
back as generated labels. `!- North Axis {deg}` came back `!- North Axis`. A unit
lost from an engineering model by an editor asked to save a file.
The doc comment said so, in bold, which is not good enough: we had already agreed
a doc comment is not a load-bearing place for a correctness constraint when the
same argument was made about what `changedObjects()` does not answer. Two readers
reached the same conclusion independently, from the language service and from the
web editor, that the range alone changed nothing they would build.
Registered in idfkit/idfkit-conformance#6.
`renderStatement` is factored out of the preserving walk rather than reimplemented
beside it. Two copies would be two answers to one question, and the question is
which bytes go in the file.
ONE OPTION, `fieldComments`, because it is the only one a preserving write
honours. `indent`, `commentColumn`, `ordering` and `versionFirst` are refused by
`writeIdf` alongside `preserveFormatting`; `comments: false` and `compressed`
defeat preservation entirely and send the document down the formatting path. The
first shape of this method took all of them, which would have let a caller render
one object on terms the surrounding file was not written on, reintroducing the
divergence one layer down.
The test that matters splices every changed object's render into its own range and
asserts the result equals `writeIdf` byte for byte. If that ever fails, an editor
built on these three names is silently writing a different file.
A cleanup pass over the preserving writer. No output moves: both corpora pass with the same counts and all 779 tests hold. THE ONE THAT MATTERED. `annotations` started its token cursor at zero and seeked forward to the statement it wanted, which is a full prefix scan of the token stream per statement and quadratic in the file. Reformatting every object of HospitalLowEnergy.idf, 6,874 statements over 233,925 tokens, took 647 ms. It now takes 76 ms. Neither benchmark covers it. Both time an UNCHANGED preserving write, where every statement is copied verbatim and `annotations` is never reached, so the budget gate would have held at any cost on this path. I nearly missed it twice: my first A/B showed no difference because the harness set each field to the value it already held, which the writer correctly treats as no edit at all, so I was timing the same untouched path the benchmarks time. The fix is real; the first measurement of it was not. `extentEnds` and the cursor now come from one memo on the retained source, which never changes after the read. Before this, the extent was derived in two places with two lifetimes: `writePreserved` recomputed it on every write, and `regionOf` cached it in a field of its own. One immutable derivation with two caches is a disagreement waiting to happen, and the doc comment saying `extentEnds` was exported "so regionOf answers with the same extent" was the tell. THE REST, none of which changes behaviour: The anchor rule lives once, as `originOf`, beside the `isUntouched` it mirrors. `regionOf` and `renderObject` are documented as declining the same set and were deciding it separately. `preservingOptions` resolves what a preserving write resolves. `renderObject` had its own copy of the defaults, including the comment column whose value carries a six-line argument from the example corpus; moving that argument would have left the accessor on the old number, silently breaking the byte-for-byte agreement its own doc comment promises. A private field sat between `regionOf`'s doc comment and `regionOf`, so the whole comment, worked example included, documented the field and the method shipped bare. Removing the field fixed it. `annotations` no longer pre-fills entries the loop overwrites unconditionally: a placeholder is a second, contradictory statement of what an entry defaults to. The `ORIGIN` comment no longer names the strip tag in prose. `stripInternal` matches the tag as TEXT anywhere in a comment, so explaining the hazard triggered it, dropped the symbol from the emitted types and broke the build. The comment now says so without saying it. Two tests read one fixture rather than two identical copies, so a change to it cannot leave them asserting against different files while both pass.
Prettier's own check, which `npm run lint` does not include, so the two files the last commit reflowed only failed once CI reached them.
A peer measuring across four models found that "a preserving write is 7x to 45x faster than a formatting one" is an UNEDITED-document number and does not hold in general. It inverts once most objects have changed: this path renders each changed object and walks the tiling, which is strictly more work than formatting alone. On a 13 MB model with every object edited it is 409 ms against 104 ms. Not a defect and not worth a guard. What a caller actually pays is nought to a hundred objects edited, which is 0.1 ms to 4.4 ms, and a whole model rewrite is what `preserveFormatting: false` is for. But an unstated crossover is how somebody benchmarks the wrong path and reports the wrong number, which is exactly what happened, so the doc comment says it. Both libraries, same words.
The runner level first. The case set is unchanged from 2026.10, 69 cases and 211 assertions, and this library sees no new expectation. What it sees is a runner that stops reporting a false failure on preserve-edit-one-field, and the terminator-comment rule mirrored into the JavaScript runner so the two agree about where a statement's text ends. The governance level carries the five names this feature adds, none of which renames anything: changedObjects, regionOf, Region, renderObject and fieldComments. The naming gate passes with 136 of 136 public names resolving. That order is the rule rather than an accident. The entries were published in idfkit-conformance and the tag cut from its main before this pin moved, so a gate here could never have gone green against something a reviewer had not seen.
The constant FR-044 rests on, moved only after checking the other library rather than after moving this one's pin. Evidence taken today against the corpus checked out AT THE TAG, not read off a badge: idfkit-js packages/core/package.json idfkit.conformance = conformance-2026.11 idfkit pyproject.toml [tool.idfkit.conformance] = conformance-2026.11 idfkit-js npm run check:release PASS at that level idfkit uv run python scripts/check_release_conformance.py PASS at that level 2026.11 changes no case. 69 cases and 211 assertions, as 2026.10 had; what moved is the runners, which stopped reporting a false failure on preserve-edit-one-field. So the precondition is met on the same evidence it was, rather than on a weaker one. The gate caught this: bumping the pin without moving the constant failed `emit-conformance.mjs --check`, which is the cheap failure it was added to produce after the pair went stale once between 2026.7 and 2026.8 and surfaced only when a release was attempted.
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
1 participant
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.
Companion to idfkit/idfkit#199. Both languages had the same defects and they are fixed the same way in each.
An edit asks for a value to be re-rendered. It does not ask for the object's comments to be rebuilt, or for its lines to be redrawn. This branch closes four ways the writer went further than it was asked to, and adds the one question a consumer could not answer for itself.
1. The author's field comments
became, on changing
north_axis:The unit is gone, and so is the note, with nothing in the output to say it existed. Neither is a formatting difference; it is text the file contained and the output does not, from a writer whose purpose on this path is to give the file back.
writeObjectnow takes the object's original text and reuses its per-field comments, generating one only for a field the author never wrote one for. A field's comment is the one after its delimiter on the same line, the convention every writer of these files follows and the one case where which field a comment belongs to is not a guess. A comment on its own line belongs to no field and is untouched.A field the author left BARE stays bare, because absence is also something the author wrote.
fieldComments: 'generate'is the opt-in for a caller who wants labels on everything.2. Everything else on the object
The same argument applies past comments: blank lines the author left inside an object, and the header block above it, are the author's too. They now survive an edit.
3. The line a value was written on
The writer put one value per line whatever the source said. Measured across the 693 EnergyPlus 22.1.0 example files, that is not a corner case:
A full reformat of that corpus would add 20.2% to its line count, 89.6% of it from this one shape. A four-line surface became twelve.
The per-field annotation gains
startsLine, which the delimiter scan already knew and was discarding, and the emitter builds a line rather than pushing one per field. Editing one wall of1ZoneUncontrolled.idfleaves the file at 462 lines rather than 470.The type name is part of the line under construction rather than pushed ahead of it, so an object written
Timestep,4;comes back on one line. That is the second row of the table, and the case that surprises on a file with no geometry in it: nobody thinks ofTimestep,4;as formatting they chose.4.
changedObjects()Which objects a preserving write will write afresh rather than reproduce. Empty for a document read with
preserveFormatting: trueand not edited since.It is the part a consumer cannot work out for itself: a rename clears the record on every object that referred to the renamed one, so counting from your own edit log reports one where the answer is nine. It is not "everything that will differ" — a removed object is no longer there to be yielded, so it can return empty for a write that changes the file. The docstring says so, because a consumer reading an empty result as "unchanged" would be wrong on every removal.
Registered in idfkit/idfkit-conformance#6. The naming register gate needs that merged and
governance-2026.15cut before this branch's pin can move, which is the publish-before-you-pin order working, not a regression.Two defects that fell out
An object at the end of a file gained a blank line on every reformat: the writer appended a fixed
\n\nwhere the node's own trailing newlines, one for the last object, were the right thing to reuse. The conformance runner's span reconstruction had the same bug, fixed in idfkit/idfkit-conformance#5.comments: falseoutput stopped parsing partway through the restructuring, because the type name is the line still open when the field loop starts and the branch that returned early emitted it last. The corpus caught it, and the suite now reparses what a bare write produced.How the first one was found
A reader of the TypeScript change pointed out that its example used
{deg}, which the writer can regenerate, so deletion and regeneration looked identical. A genuinely custom comment separated them and showed both languages were destroying it. The matrix now agrees exactly across the two:State
765 tests pass. Both corpora pass locally at
conformance-2026.10with the runner fix applied. TheCross-library conformancejob here fails until idfkit/idfkit-conformance#5 merges and a level carrying it is cut, because CI checks the corpus out at the pinned tag and that tag predates the runner fix.