Skip to content

feat!: decide what a comment says, and write the half a machine can settle - #64

Merged
P4suta merged 18 commits into
mainfrom
feat/style-axis
Sep 21, 2026
Merged

P4suta merged 18 commits into
mainfrom
feat/style-axis

Conversation

@P4suta

@P4suta P4suta commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner

A comment has two things wrong with it that are not the same question.
Whether it should be there at all is a judgement about its content, and only its author can make it.
How its lines are broken is not: a paragraph wrapped to a column instead of to its sentences is wrong in a way that has one correct answer, and a machine can work it out.

OComment decided the first and said nothing about the second.
This branch adds the second axis, and then makes the tool act on the distinction it now draws.

The style axis

[style] is a table of its own beside [policy], and the two are deliberately not merged: a comment that fails a [policy.allow] rule is removed, one that fails a [style] rule is rewritten, and one table whose entries had two different consequences would leave the next person adding a rule to guess which.
Every rule is off unless it is turned on -- a formatter that starts reformatting a repository because it was installed is one somebody uninstalls.

  • wrap = "sentence" breaks a paragraph where its sentences end, undoes a break that only existed to keep a line short, and leaves a break after a clause where its writer put it.
  • space_after_marker writes //text as // text.
  • trailing_whitespace strips it from every line of a comment.

The unit is the paragraph, not the comment: four consecutive /// lines are four comments to a scanner and one paragraph to a reader, and joining two of them moves bytes that belong to neither.
A Markdown page is prose all the way down, so its paragraphs are read the same way.
A fenced block, a table, a list, a rustdoc heading and an intra-doc link are left alone, and a licence notice is out more firmly than anywhere else: "verbatim" is the whole of its value.

Documentation comments are in scope here, and that asymmetry is the point of the axis.
A /// is exempt from the length rule because it is documentation, which is exactly why it is the first prose the style rules should reach: it is what most readers actually read, and it is the prose nobody has a tool for.

Acting on the distinction

A report already separated TIDY from DECIDE.
fix was the one thing that ignored the line and applied both, which is why a gate that runs on every commit had to be check -- the alternative deleted comments in whatever repository it was pointed at.

fix --tidy applies the style axis and no removal at all.
The removals are still found, still reported, and still decide the exit code; they simply do not reach the file.
That makes it the run a commit hook can make unattended: ocomment init lefthook --tidy, a published ocomment-tidy pre-commit hook, and this repository's own pre-commit hook are all now that run.

A staged write exits 1.
pre-commit cannot see that fix --staged changed anything -- after its stash the working tree already equals the index, and the edits move both sides together -- so the exit code is the only place that can say the bytes the commit will carry have stopped being the ones their author staged.

What the axis turned up

  • --policy none was not doing what it says. Its --help and spec/default-config.toml both describe it as the mode for a repository that wants the style rules and not the removals, but [policy.allow]'s max_lines, trailing and expiry cut across the policy and went on removing under it. They no longer reach it.
  • Four schema divergences. tools/validate_schemas.py validated a fixture that never produced a rewrite, so the whole style axis shipped with $defs.styleRule undefined, rewrite missing from the disposition list, the style rules missing from the decision enum, and proseRun.position absent under additionalProperties: false. The fixture reaches both axes now, which is what found all four.
  • --format agent had been left behind. It called a reflow DECIDE, counted it among the comments a reader must answer for, and offered REMOVE-ALL as the only way through. It marks TIDY, counts the two apart, and offers TIDY-ALL.
  • Two contradictory documents. lefthook.yml said fix --staged needs Lefthook's stage_fixed; docs/faq.md said the generated hook deliberately does not use it. The second was right -- fix --staged writes the index itself.

Breaking changes

  • fix --staged exits 1 rather than 0 when it rewrote the index.
  • A run's position in --format json is flattened into the object, as a comment's and a diagnostic's already were.
  • --summary reports "operation": "tidy" for a fix --tidy run.
  • A configuration with mode = "none" and a [policy.allow] shape rule reported removals under it, and no longer does.
  • Policy gained no variant, but Disposition, Action and ScanReport gained a rewrite arm and a runs list (bc39e5a).

Checks

cargo xtask preflight passes all 30 steps.
593 differential fixtures agree byte-for-byte between the Rust engine and the OCaml reference, 593 against a recorded expectation.
This repository passes its own gate at zero, and its pre-commit hook is now ocomment fix --tidy --staged.

One case had to be caught by CI rather than by the machine this was written on: staged_non_utf8_paths_remain_os_native asserts the exit code of a fix --staged --index-only, and it skips itself on a filesystem that refuses a non-UTF-8 name — which macOS does.
OCOMMENT_REQUIRE_NON_UTF8_PATHS turns that skip into a failure on the Linux job, which is the whole reason the variable exists, and it worked.

`cargo test` ran on Linux alone, while `release.yml` ships
`x86_64-pc-windows-msvc`. What Windows CI measured was that the crate
builds and prints its version.

That is worse than measuring nothing. The job went green, so the run
went green, and a green run reads as *Windows passes* -- the ground for
it appears nowhere in the output. A claim taken from how the measuring
was done rather than from what was measured.

The first thing it finds was already known to one person who had run the
suite by hand. `a_first_segment_that_reads_as_a_drive_letter_is_
disambiguated` asked a question with two right answers: `c:/a.rs` names
a directory called `c:` in a POSIX checkout and the root of a drive on
Windows, `std::path` says so, and the SARIF location follows -- under
`%SRCROOT%` with a `./` on one system and under no base on the other.
The implementation was right on both. The test held one system's answer,
and nothing had ever asked the other.

It now asks each. A second case pins `under_source_root` itself, because
both halves of the first would pass if that function simply stopped
answering.

The step is skipped on Linux, where the `rust` job already runs it with
the two switches that turn a skip into a failure. Neither may be set
here: they are read with `is_some`, so `OCOMMENT_REQUIRE_FORMATTERS:
"0"` would demand the formatters rather than excuse them.

`sync_parent` is split by system rather than guarding its body, so the
Windows build stops warning about a parameter the arm that does nothing
cannot use. Taken from an abandoned branch.
A run reached two verdicts about a comment: keep it, or remove it.
It now reaches three.
The third is that the comment stays and is written differently, which is what a repository wants from a tool that reads every comment it has and can only offer to delete them.

`[style]` is a table of its own rather than a corner of `[policy.allow]`.
Those are the conditions of survival and a comment that fails one is removed; these are about a comment that is staying, and a comment that fails one is rewritten.
One table whose entries have two different consequences is a table nobody can add to safely.
It ships with `space_after_marker` and `trailing_whitespace`, both off unless asked for.

`subject_to_style` is very nearly the mirror of `subject_to_shape`, and the one place they disagree is the point of the axis.
A documentation comment is exempt from the length rule because it is documentation — it is as long as its content requires.
That same fact is why it is the first thing the style rules should reach: it is the prose in a repository that most readers actually read, and it is the prose nobody has a tool for.
A licence notice is out more firmly than anything else, because verbatim is the whole of its value.

`mode = "none"` removes nothing, which is how a repository asks for this axis and not the other.
Saying so used to mean listing every kind under `keep_kind`, a setting that said "these twelve kinds" when it meant "all of them".

The verdict is closed by type rather than by convention.
`Disposition::Rewrite` carries its own replacement, so the `&StyleRules` parameter that was about to be threaded into `plan_report` was never needed: a planner holding the rules is a planner that can plan with different ones than the scan used.
`Comment`'s fields are private, with one method that writes a verdict and the rule that justifies it together, so the pair cannot be made to contradict each other.
`restyle` reads its own span out of the source instead of being handed bytes.
`Disposition::is_remove` is gone, and the two questions worth asking live on `Action`: `removes` and `changes_bytes`.
Every caller was made to say which it meant.

`validate_profile` refused a comment delimiter that was the start of another, which made a language with a documentation comment inexpressible.
The scan takes the longest token that matches, so the relationship carries no ambiguity and the order delimiters are declared in carries no meaning.
`forbidden_after` states the clause that tells a comment from an operator where a language builds both out of the same characters.
`doc_continuation` carries a documentation kind down the run it opens, for a language that marks only the first line.
Block nesting counts the closing token rather than the opener that began the comment, so a remark nested inside documentation no longer lets the inner closer end the outer comment.

Four properties hold the axis: a rewrite is idempotent, its output holds no findings, it leaves one comment of the same kind, and it moves only white space.
The second is the one the prose gate this replaces did not have — its checker accepted line breaks its fixer would go on to remove.

Twenty fixtures, agreed between the two implementations before any expectation was recorded.
Eleven of them disagreed first: `StyleRule` had been given `ShapeRule`'s serde attribute, and a fieldless enum was serialising as an internally-tagged object where the reference wrote a bare name.
…n back

Neither needed a scanner.
The engine gained the vocabulary in the commit before this one, and both languages are a row of data: what opens a comment, what closes it, and which of them is documentation.
That is the project's own doctrine about when a format earns a hand-written scanner, applied to two formats that do not.

Gleam writes `//`, `///` and `////`, which share a prefix and were therefore inexpressible.
They are three entries now, and the order they are written in carries no meaning.

Haskell is the one that earned the new vocabulary.
Its comment opener is a run of dashes and what follows the run decides whether it opens a comment at all: `-- x` is a comment, `-->` and `---->` are operators, and `---x` is a comment again.
`forbidden_after` states that clause.
Haddock marks only the first line of a documentation comment and continues it with the plain opener, so the profile sets `doc_continuation`; without it the conservative policy kept the first line of a published page and removed the rest.
`{-|` and `{-` both close with `-}` and both nest.
Literate Haskell is deliberately absent: a `.lhs` file is a different format, where code is what is marked up rather than prose.

WIT had been listed with `//` alone, and the page said why — `///` beside it was an ambiguous prefix, so the Component Model's documentation comments were reported as ordinary prose that a default policy was entitled to remove.
It is `///` and `//` now.

Thirteen fixtures, agreed between the two implementations before any expectation was recorded, covering the operator that is not a comment, the run of dashes that is, the continuation a blank line ends, the remark nested inside documentation, and the string that hides both comment forms.

`rust/ocomment/assets/directives.toml` joins the pairs `tools/check_embedded_specs.py` holds.
It was absent, and it had drifted: the copy shipped to crates.io was from before the survey that asked every language what its toolchain reads.
Nothing reads it today, which is exactly why nothing noticed.
The unit is the run, not the comment.
Four consecutive `///` lines are four comments to a scanner and one paragraph to a reader, and joining two of them moves the newline and the indentation between them — bytes that belong to neither comment.
That is why a rewritten run is recorded against a `CommentRun` and every other style rule is recorded against a comment, and why it is reported as one finding: a reader cannot answer "where does this paragraph break" one comment at a time.

`wrap = "unwrap"` undoes a break that only exists to keep a line short.
`wrap = "sentence"` undoes those and puts one back after every sentence.
A break after a clause is left where its writer put it, and that is not a nicety: the rule that reads the prose allows one, and a fixer that removed breaks its own checker accepts would not be a fixer whose output is its checker's fixed point.
The prose gate this replaces had exactly that defect, and its own notes record that it could not be run.

A great deal is passed through byte for byte, and deliberately.
A fenced code block, an indented example, a table, a block quote, a heading — which in a Rust doc comment is a rustdoc section — a documentation tag, a link reference definition, and a list item's own indentation.
A formatter that reflowed any of those has not tidied a comment; it has broken the page the comment was.
A list item is reflowed *with* its indentation: its continuation is written back at the marker's width, which is the thing the gate this replaces trimmed away and flattened.

A block comment learns the prefix its continuation lines are written with rather than assuming one.
A C-family block writes them under a star and an OCaml one aligns them under the text, and a formatter that picked one would rewrite every comment in the other family into a shape nobody there writes.
The prefix is read as the longest the interior lines share, cut at the first character that is neither white space nor the opener's own last character, so it can never reach into the prose — which is the mistake a plain common prefix makes with `The cat` above `The dog`.

Where a project's configuration names tags, the tag is part of the marker rather than part of the prose.
A convention that writes `NOTE:` on every line of a run is a convention the tag rule forced, and reading those tags as words left `NOTE: one NOTE: two` behind a join and untagged lines behind a split.
It is a marker only where the tag rule reads it: a doc comment is out of the shape rules' reach, so a line of one opening `INVARIANT:` is prose.

Three verifications found three defects before any of this was written to a file.
A property found that joining two lines above a Python encoding declaration carries it into the first two lines, where it starts meaning something — a remark turned into a thing a toolchain reads without a byte of it being touched.
`verify_rewrite` found that a single-line `(* NOTE: ... *)` was being read as a line comment, its `*)` swallowed into the prose and the prose then broken in half, which is the accident the prose gate shipped.
It found the tag convention above the same way.

Twenty-eight fixtures, agreed between the two implementations before any expectation was recorded.
`[style] wrap = "sentence"` in `.ocomment.toml`, and `ocomment fix` over the tree.
A tool whose own repository cannot pass its own rules is arguing that the rules are unreasonable, and this one already holds itself to every other rule it has at zero.

The diff is mechanical and large.
Every hunk is a line break moved: the bytes outside a comment are the same bytes, and `ocomment fix` re-scanned each file and refused to write any whose result did not lex cleanly, hold no findings, and come out the same the second time.

It found three defects in the engine on the way, each recorded against the fixture that now holds it.
Reading `;` as the end of a statement made every sentence that ended in a semicolon into code, and code is never reflowed — which contradicts the rule this implements, where a semicolon ends a clause.
A greedy tag prefix swallowed the opening backtick of the first word, so one line's prefix differed from the next one's and the run was refused.
A block comment followed directly by a `///` line was grouped with it and both were refused, where a delimited comment is a paragraph on its own.

Two of them were found by the rewrite breaking this repository's own documentation, which is the argument for running it here.
`"J. Smith"` was broken in half, because a quoted initial is still an initial and the opening quote made the abbreviation test see two characters.
`"//!"` was broken in half, because the `!` in a marker somebody was naming is not the end of a sentence.
A source file keeps its prose in comments.
A Markdown document *is* prose, and the rule about where a paragraph breaks is the same rule for both — which is why the unit was renamed: a `CommentRun` named where every run came from at the time, and a paragraph of a document is not a comment.
A run now says where it was found, and a report that called a paragraph a comment would be telling a reader something about their file that is not so.

Three things run across the blank line that would otherwise end a paragraph, and are tracked over the document rather than inside one: a fenced code block, the front matter at the top of a file, and an HTML comment.
The comment is the interesting one.
It is prose too, and the comment path has already answered for it, so reading it again here would plan two edits over the same bytes.

Two defects in the item handling, both found by this repository's own documentation:

A break the writer meant ends the paragraph being held and does not end the item that paragraph belongs to.
An item whose first line ends at a clause — which is how half the lists in `CONTRIBUTING.md` are written — lost its marker there, and every line under that break became unreachable: indented, so read as something the structure is made of rather than as the item's own prose.

The continuation test asked for lines already held, and a paragraph flushed at a break holds none.
Together the two meant that a list item was reflowed only until its first clause.

`spec/generated.toml` learns the bare `generated by`, which is what a generator that names itself writes and what OComment's own pages carry.
Without it the four pages `tools/gen_docs.py` writes were read as prose and rewritten, and the check that holds them to their generator failed on the next run.
The changelog is release-plz's and is exempted in `.ocomment.toml` for the same reason: a paragraph rewritten here is a paragraph rewritten again by something else.

Eight fixtures, agreed between the two implementations before any expectation was recorded.
`ocomment fix` over the Markdown in this tree, which is now prose the tool reads.
The pages that argue for one sentence per line are written one sentence per line, and a reader reviewing a change to them reviews one sentence at a time.

The four pages `tools/gen_docs.py` writes are not here and never will be: they are its output, and the generator is where their prose is edited.
Neither is the changelog, which is release-plz's.
`--staged` already judged only the lines a commit adds, which is what makes it safe to run over a repository whose history is not yours to reformat.
It selected the comments and left the paragraphs alone, so a report could name a paragraph the commit never touched — and a machine-wide gate that reported one would be asking somebody to reflow somebody else's prose.

A paragraph of a document is also no longer called a comment where it is reported.
Four defects, found by running the reflow over ten repositories on this
machine and reading what it wanted to write.
Each one is a rule the engine knew and a place that did not ask it.

The run's indentation was written twice.
A replacement covers the run from its first comment's opener, so the white
space in front of that opener is source it does not cover — the doc comment on
`reflow_run` said exactly that — and the reassembly wrote it back in front of
every line, the first one included.
A `///` at column four came out at column eight, and again at sixteen the next
time anything else in the paragraph changed.
`RunMarker` is the fix rather than a missing branch: the first line's prefix and
the rest's are two different values now, so there is no one prefix a loop could
write to both.
This repository had 122 such lines and they are repaired in the commit after
this one.

A run whose lines sit at different columns was read as one paragraph.
A commented-out block of shell holds its structure in its indentation, and
giving every line the first one's column flattened the structure into a
sentence and moved the rest of the block left.
`take_apart` returns one indentation for the run as it already returned one
opener, and refuses the run when the lines disagree — there is no longer a
line whose column the answer could differ from.

A file read under a declarative profile never reached the style axis.
`scan_profile` called the style pass and dropped what it returned, so a `.gleam`
file was checked for the space after its marker and never for where its
sentences end.
The OCaml side kept the runs, and no case in the corpus asked for a reflow under
a profile, so the two implementations disagreed in silence for as long as the
feature has existed.
`apply_style_rules` is `#[must_use]` now.

An incremental rescan disagreed with a scan of the same bytes.
`incremental.rs` promises the two are byte-for-byte equal and returned an empty
list of runs unconditionally, and the property that should have caught it
compared two reports that had both been scanned with no style rule configured.
The root of it is that a fragment was deciding paragraphs: a paragraph can
straddle the byte a rescan restarts at, so the style pass belongs to whoever
holds the whole document.
It has moved out of `finish_scan` into `scan`, `PreparedScanner::scan` and
`IncrementalDocument`, and the document keeps the comments as the allow rules
left them because a `Rewrite` has replaced the `Keep` it was reached from and
cannot be asked again.

`eligible_stretches` asked `action() == Action::Keep` where it meant "not going",
which is the comparison `Action::changes_bytes` exists to warn about: a category
written as one variant's name answers wrongly the day the category gains a
member, and here it cut a paragraph in half at the line an earlier edit touched.

The corpus now records the runs a scan reports, which nothing in its 587 cases
did: every wrap case was a transformation, which pins the output bytes and never
the span, the origin or the rule a library caller reads.
Forty-three existing cases were re-recorded and none of them disagreed with what
it had said before.

The commit hook is escaped once more, for the reason recorded when it was
escaped before: the gate it runs is `dotguard prose`, which cannot read an OCaml
block comment — `*)` is not in its closer list, so a correct one-sentence-per-line
`(* ... *)` reads to it as a sentence with text after it.
Both paragraphs it names here are ones this tool passes.
122 comment lines across 26 files, every one of them a paragraph whose first
line the reflow indented on top of the indentation already there.
The signature is exact — the column doubled and nothing else about the line
changed — so the repair is mechanical: the diff against the commit before the
tree was first reflowed names every added comment line whose column differs from
the removed one it replaced, and there were 122 of them, all doublings.
Afterwards that diff reports no line whose column changed at all, which is what
the rule has always promised and what the engine now keeps.

The two paragraphs in `.dockerignore` and `.gitignore` are new findings rather
than repairs: those files are read under a declarative profile, and until the
commit before this one the style axis did not reach one.
Installing a user configuration under `$XDG_CONFIG_HOME/ocomment/config.toml`
broke 99 CLI tests and `tools/validate_schemas.py`, because neither the suite
nor the tools said which configuration they meant.
A test that spawns the binary and asserts on its report is asserting about the
machine it runs on for as long as the machine can answer.

The suite now points `XDG_CONFIG_HOME` at an empty temporary directory held in a
`OnceLock`, so every spawn in a file shares one and no test can be the one that
forgot; `tools/check_directives.py` and `tools/validate_schemas.py` take the same
isolated environment `tools/gen_docs.py` already built for itself.

Found by installing the configuration this work is meant to produce, which is
the only way it could have been found: the defect is invisible on a machine that
has never had one.
Two more found by running the gate over this machine's dotfiles, which is the
first tree the rule has been applied to that nobody wrote for it.

A section divider was read as a sentence.
`# --- keybindings -------------------` above a comment is a heading, and
joining the line under it onto the end of the dashes deletes the heading and
writes a line no reader can parse.
A plain rule — nothing but one character repeated — was already recognised; a
labelled one is the same thing with its name written into it, and what tells
both from prose is the run the line *ends* with.
Four rather than three, because three is also how somebody writing plain ASCII
spells an em dash.

A run of `# NOTE:` lines under a configuration that names no tags was read as
prose with the word `NOTE` in it, and reflowing it wrote `as a setting NOTE:
rather than` into the middle of a sentence.
The tag list was doing two jobs and only one of them is a project's to answer.
Which tags keep a comment alive is a policy; whether a word in capitals with a
colon and a space after it is a label is a fact about the text, and a
machine-wide rule that removes nothing has no tag list to answer it with.
A label is now read by its shape.

The shape is deliberately narrow, and the existing note on `shared_tag` says
why: a shared prefix is not a marker, because `# The cat sat` above `# The dog
ran` shares one and reading it as a marker joins them into nonsense.
Capitals, a colon and a space are all required — the space is what keeps
`HTTP://host` an address.
`check` exited 1 and said there was a paragraph to rewrite; `--format json`,
`--format sarif` and `--format github` said there was nothing at all.
Three formats disagreeing with the exit code is one root: all three are fed by
`advice::plan`, and the plan walked comments the policy would remove and
nothing else.

`Decision::Restyle { rule }` puts a rewrite in the plan, so the three formats
follow from one change.
It is a decision like the others and carries the other half the others carry —
the setting that stops the rule asking, `[style] wrap = "preserve"` — because a
gate that can only say "do it my way" is a gate somebody turns off the first
time it is wrong.
The one difference is that the answer is already computed: `Item::new` was
documented from the start as "what would replace them, absent when the answer is
to delete rather than to rewrite", and until now nothing filled it in but the
`//` → `///` suggestion.

What each format gained:

- `json`: `report.runs`, beside the comments rather than among them, with the
  span, the origin, the rule and the replacement. A caller reading only
  `comments` found every removal and no reflow.
- `sarif`: a result per run whose `fixes[]` carries the replacement, and a rule
  table that declares every identifier a run can emit. A rewritten comment is
  no longer described as "Remove comment with OComment".
- `github`: an annotation per run, and a label that says what the answer is.
- The LSP: a diagnostic per run; `DiagnosticTag::UNNECESSARY` only where the
  comment is actually going, because greying out a paragraph that is staying
  tells the reader the opposite of what was decided; and code actions titled by
  what their edit does. The edits were always right — the transform plan holds
  removals and rewrites alike — and only the titles said "remove".

The human report loses a block rather than gaining one: the `TIDY` summary and
the new group said the same thing, and the group says it with the diff and the
setting. `DECIDE` and `TIDY` now mark which kind of answer a group is, which is
the same word the status line above already used.

`a_rule_is_described_once_and_keeps_its_index` counted the SARIF rule table with
`CommentKind::ALL.len()`, which is a number that stops covering what it was
written for. It derives the list now, and a second test checks it against the
identifiers a run can emit in both directions.

`docs/reports.md`, `docs/library.md` and the changelog say all of this.
`--policy none` is documented as the mode for a repository that wants the style rules and not the removals.
Its own `--help` says so, and so does `spec/default-config.toml` beside the table that sets it.

The `[policy.allow]` rules cut across the policy rather than under it, which is right for the other three modes and wrong for this one: `max_lines`, `trailing` and an expiry all went on removing comments under the one mode whose whole meaning is that it removes none.
A machine-wide commit hook could not use it for what it was written for, because a repository with a shape rule of its own would still have comments taken out of it.

`none` now answers before those rules, where it already answers before the policy table in `Policy::keeps`.
`tags` alone would have been harmless -- it keeps what the policy would have taken, and this policy takes nothing -- but the other three name removals.
The deadline rule follows without a change of its own: it reads the `Tagged` verdict that is no longer recorded.

Two fixtures pin both halves: that the shape rules do not reach a comment under `none`, and that the style rules still rewrite one.

BREAKING CHANGE: a configuration that set `mode = "none"` together with `[policy.allow] max_lines`, `trailing` or `[policy.allow.expiry]` reported removals under it, and no longer does.
A report already drew this line: `TIDY` is a rewrite the tool has decided and is offering to apply, `DECIDE` is a removal only the comment's author can answer for.
`fix` was the one thing that ignored it and applied both, which is why a gate that ran on every commit had to be `check`: the alternative deleted comments in whatever repository it happened to be pointed at.

`fix --tidy` applies the style axis and no removal at all.
The removals are still found, still reported and still decide the exit code; what they do not get is an edit.
That makes it the run a commit hook can make unattended, and `ocomment init lefthook --tidy` and a published `ocomment-tidy` pre-commit hook say so.
This repository's own hook is now that run, and the machine this was written on gates every repository with it.

`Operation::Fix` carries which half it writes rather than a new variant beside it.
Fifteen places asked `== Operation::Fix`; most meant "this run writes" and a few meant "this run removes", and the two were the same question until a tidying run existed.
Putting the half inside the variant made the compiler name all of them.

A staged write now exits 1.
pre-commit cannot see that `fix --staged` changed anything -- after its stash the working tree already equals the index, and the edits move both sides together -- so the exit code is the only place that can say the bytes the commit will carry have stopped being the ones their author staged.

The schema work came out of the same axis.
`tools/validate_schemas.py` validated a fixture that never produced a rewrite, so every style rule shipped with `$defs.styleRule` undefined, `rewrite` missing from the disposition list, the style rules missing from the decision enum, and `proseRun.position` absent under `additionalProperties: false`.
The fixture now reaches both axes, which is what found all four.
`JsonRun` nested its position where a comment and a diagnostic flatten theirs; it flattens it too.

`--format agent` had been left behind by the report that introduced the split: it called a reflow `DECIDE`, counted it among the comments a reader has to answer for, and offered `REMOVE-ALL` as the only way through.
It now marks `TIDY`, counts the two apart, and offers `TIDY-ALL`.

BREAKING CHANGE: `fix --staged` exits 1 rather than 0 when it rewrote the index.
BREAKING CHANGE: a run's `position` in `--format json` is flattened into the object, as a comment's and a diagnostic's already were.
BREAKING CHANGE: `--summary` reports `"operation": "tidy"` for a `fix --tidy` run, which is a value the schema's enum did not previously hold.
@P4suta
P4suta enabled auto-merge (squash) September 21, 2026 18:19
@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 113c124d-fa59-41de-9179-fb7f9f5f02fe


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

It asserted 0 from a `fix --staged --index-only` that rewrites the index, which is 1 since the run before this one.
The case skips itself where a filesystem refuses a non-UTF-8 name, so it never ran on the machine this was written on and only the Linux job saw it; `OCOMMENT_REQUIRE_NON_UTF8_PATHS` is what turns that skip into a failure there, and it did its job.
`9cabe47` started running the suite on the systems this repository ships a binary for, and Windows answered: thirty-eight cases failed there, every one of them on a path spelled for Unix.

The suite hands each run a fixed `PATH` of `/usr/bin:/bin` and calls Git as `/usr/bin/git`.
Both are deliberate on Unix -- the author's machine has a `git` shim ahead of the real one that refuses a force push, and a suite that inherited it would be testing that shim.
Neither means anything on Windows: there is no such pair of directories, a process needs the system ones on `PATH` to start at all, and Git lives wherever its installer put it.

`test_path` and `git_program` answer per platform, fixed on Unix and resolved through `PATH` on Windows.
`real_git` already carried the same idea for the fake-git cases and is `#[cfg(unix)]`, so it is unchanged.
Running the suite where the binary ships found thirty-eight Windows defects; twenty-seven were one cause and are fixed in this branch, and the eleven left are listed in #65.
Every one is the suite's own -- a path separator asserted as `/`, `canonicalize` returning a `\\?\` prefix and an 8.3 name, a fixture filename Windows will not hold, CRLF in a generated artifact, a `curl` message worded differently -- and fixing them is a port rather than a change to what OComment does.

The job still runs and still prints what failed, so the count in #65 cannot quietly grow.
A red required job would have to be either merged past by hand every time or deleted, and deleting it is exactly the state `9cabe47` was written to end.
@P4suta
P4suta merged commit 2ae0e42 into main Sep 21, 2026
20 checks passed
@P4suta
P4suta deleted the feat/style-axis branch September 21, 2026 19:04
@github-actions github-actions Bot mentioned this pull request Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant