Remove markfluence fix - #156
Conversation
Closes #151. fix reconciled a file *to* its live page -- the opposite direction from every other write verb -- and #10 makes that worse rather than better: once update enforces space/parent and moves pages, the file is authoritative for coordinates outright, so a verb that pulls coordinates out of Confluence works against the grain instead of complementing it. What the drift use case wants is a read-only report followed by update to push, and a report composes into CI, a hook or a cron where fix structurally cannot. The reporting goes to #154 (markfluence diff), which covers more than fix --dry-run did: the whole file including the body, where fix only compared the metadata it was prepared to write. #154 is not built yet, so this opens a window with no drift report at all -- accepted deliberately, since markfluence is unreleased and both sit on 1.0.0, and the alternative is carrying a verb whose direction is already known to be wrong. It also drops #139's hardest remaining consumer. A pristine file whose metadata lives in a pages: entry has no frontmatter to reconcile, so fix refused outright; making it work meant aiming a nested surgical writer at the manifest in the opposite direction from create's, for a command being deleted. #139's write half is now done. What is genuinely lost is automated repair: "somebody labeled this page in the UI and I want my file to match" becomes reading the page (info shows labels and width, read/export emit them) and editing the file. Locate-by-title becomes find "Title" plus a paste. Both real, neither worth the verb. No shared code becomes dead -- every helper was checked for another caller (labels.Read, pagewidth.Read, SearchPagesByTitle, jsonout.CodeOr, pageref.NotFoundMessage/NotNumericMessage, frontmatter.Normalize/UpdateField). One latent leftover recorded in the plan and deliberately not acted on: fix was the only production caller omitting SearchPagesByTitle's variadic statuses, so its StatusCurrent default now has only a test exercising it. Touching the client while deleting a command is how a removal grows a regression. The schema was edited as text -- enum entry, if/then branch, fixResult and fixSummary -- never through a JSON round-trip, which reformats the whole hand-authored document. checkSummary's description cross-referenced fixSummary's granularity and is reworded rather than left pointing at a $def that is gone. TestCommandEnumMatchesRegisteredCommands and internal/schematest close the loop from both sides, so a half-removal fails the build.
Several of these are rewrites rather than deletions, because what the fix paragraphs explained has to survive in some form. **guarantees.md, L9.** The paragraph placing fix outside the law goes, but the asymmetry it described is worth recording: an absent labels key meant "leave the page alone" to update and "adopt whatever the page carries" to fix, which is the kind of thing a reader has to hold in their head. The law now describes every writing verb with no exception. Status unchanged and said so -- create's default width is what makes L9 partial, and the removal does not touch it. **root-model.md** pulls two ways. "Every command that takes a page accepts a pristine registered file ... the exception is fix" loses its exception, which is a real simplification. But "a wrong space is visible and fix recovers it" was the recovery half of the argument for keeping url out of the project file; the asymmetry survives untouched and only the remedy is restated honestly, as repairable by hand. **cmd/check/check.go** pointed the half-and-half warning at "fix moving the keys is the remedy" -- which fix never did: it refused a manifest-only file outright. Wrong before this change and wrong differently after, so the comment now says the remedy is manual and why. **confluence/labels.md** loses the adoption bullet, which recorded markfluence behaviour that no longer exists. The block-style round-trip bullet is kept and re-attributed to the frontmatter writer, which create's persist step still exercises -- it was a fact about the writer, verified through fix. **README.md** drops the command row, the --dry-run list, and the worked fix section. The replacement says the direction plainly rather than mentioning the removal: nobody used this command, since markfluence is unreleased, so a "was removed" note in a 50,000-foot overview is noise. **markdown_file.md**'s labels and page_width rows lose "fix writes back the live page's ...". The labels one named the capability being lost, so it points at info instead of simply being cut. **json-output.md** loses fix from the status-verb list and the one-result-per- target list. **CLAUDE.md** loses it from the command-package list, corrects the pageref attribution (create and update report both messages; check reports the non-numeric one), and collapses "two output rules" to the one that is left. CONTRIBUTING.md and internal/completion needed nothing -- their hits were the conventional-commit type and "fixed vocabulary".
…t wrong
None was catchable by the sweep the plan specified, which is the lesson: a
removal's survey has to cover prose, code comments, doc examples, and callers
of anything the deleted package called. The plan listed one and a half of those
four.
**frontmatter.UpdateListField lost its only production caller**, contradicting
the plan's own "no shared code becomes dead" table. cmd/fix was its only
non-test caller, and it is the sole entry to the sequence-style-preservation
path -- readsBackInSeqAs and the seq.IsFlowStyle read that stops a block labels:
list becoming an unreadable single flow line. Nothing in the binary writes a
frontmatter sequence surgically now: create persists five scalar fields and
deliberately never writes labels, update writes no files, and read/export go
through Render. Kept rather than deleted -- internal/frontmatter owns the
dialect rather than serving current callers, and deleting adjacent machinery
while removing a command is how a removal grows a regression -- but the claim is
corrected in the plan, and the contract is now pinned only by that package's
unit tests.
**The labels.md re-attribution was false**, and it is the same fact from the
docs side. The bullet was kept and pointed at create's persist step as what
still exercises block-style preservation, but writeBackFrontmatter writes
scalars and persistToManifest documents that labels is deliberately not
written. It now says plainly that no command exercises it. Re-attributing a
verified claim without checking the path reaches the behaviour is the mistake.
Two pre-existing errors this removal turned into contradictions:
docs/markdown_file.md's page_id row claimed update "looks it up by title and
writes it back when missing". It never did -- update fails with "no page id" and
searches for nothing -- and it now contradicted the README paragraph this branch
added.
README's `markfluence schema` example printed an enum with fix and without
check: stale for fix here, already wrong about check. Verified against the
binary now.
And one where my own edit was wrong: README's --dry-run list went from "create,
update and fix" to "create and update", when five commands register the flag.
An understatement preserved while touching the line anyway.
The rest name the deleted command in comments: pageref/message.go (whose count
was also wrong, and which disagreed with the CLAUDE.md sentence this branch
rewrote for that line), pagemeta's package doc, create.go twice, update.go's
previewWidth, jsonout, and labels' Diff ("three commands", now two).
jsonout_test.go also built an envelope with command: "fix", a value no longer in
the enum -- harmless under its substring assertion, but documenting an invalid
document as the example.
|
Code review found eleven stragglers, all documentation, comment or dead-code drift — no correctness or behavioural bugs. Fixed in
Kept rather than deleted, deliberately: The Two pre-existing errors this PR turned into contradictions
And one where my own edit was wrong
The restComments naming the deleted command: The lesson, recorded in the planNot one of these was catchable by the sweep the plan specified (
|
1 participant
Closes #151.
fixreconciled a file to its live page — it located the page, read the width and labels, and wrotepage_id,space,parent,page_width,labelsand a missingtitleback into the frontmatter. That is the opposite direction from every other write verb, and #10 makes it worse rather than better: onceupdateenforcesspace/parentand moves pages, the file is authoritative for coordinates outright, so a verb that pulls coordinates out of Confluence works against the grain instead of complementing it. What the drift use case actually wants is a read-only report followed byupdateto push — and a report composes into CI, a hook or a cron, wherefixstructurally cannot.It also drops #139's hardest remaining consumer. A pristine file whose metadata lives in a
pages:entry has no frontmatter forfixto reconcile, so it refused outright (cannot reconcile a pages: entry yet). Making it work meant aiming a nested surgical writer at the manifest in the opposite direction fromcreate's — the largest unbuilt piece of #139's write half, for a command being deleted. #139's write half is now done.The window this opens, deliberately
#151 said to do this after something else reports drift, because
fix --dry-runwas the only field-by-field comparison of a file against its page that markfluence had. #154 (markfluence diff) now owns that reporting and covers more thanfix --dry-rundid — the whole file including the body, wherefixonly compared the metadata it was prepared to write — but #154 is not built, so this leaves a window with no drift report at all.Accepted rather than overlooked: markfluence is unreleased, both issues sit on 1.0.0, and the alternative is carrying a verb whose direction is already known to be wrong. Recorded in
_plans/040so it reads as a decision.What is genuinely lost is automated repair. "Somebody labeled or resized this page in the UI and I want my file to match" becomes: look at the page (
infoshows labels and width,read/exportemit them) and edit the file. Locate-by-title goes the same way —fixwould find a page from its title alone and write the id in, which is nowfind "Title"plus a paste. Both real, neither worth the verb.No shared code becomes dead
Checked caller-by-caller rather than assumed:
labels.Read(info,read,pagedoc),pagewidth.Read(update,info,read),client.SearchPagesByTitle(create,client/find.go),jsonout.CodeOr(update,attachment-upload,attachfile),pageref.NotFoundMessage(create,update),pageref.NotNumericMessage(create,update,check),frontmatter.NormalizeandUpdateField(create).One latent leftover recorded and deliberately not acted on:
fixwas the only production caller omittingSearchPagesByTitle's variadicstatuses, so itsStatusCurrentdefault now has only a test exercising it. Removing the default is a separate simplification — touching the client while deleting a command is how a removal grows a regression.Schema
Four text edits: the
commandenum entry, theif/thenbranch, and thefixResultandfixSummary$defs. Edited as text, never through a JSON round-trip, which reformats the whole hand-authored document — 1,728 lines of diff for a ten-line change, measured in #139 and reverted. Net 55 deletions, 2 changed lines.checkSummary's description cross-referencedfixSummary's granularity, so it is reworded rather than left pointing at a$defthat is gone.TestCommandEnumMatchesRegisteredCommandsandinternal/schematestclose the loop from both sides, so a half-removal fails the build rather than shipping.Docs, several rewritten rather than deleted
guarantees.mdL9 — the paragraph placingfixoutside the law goes, but the asymmetry it described is worth recording: an absentlabelskey meant "leave the page alone" toupdateand "adopt whatever the page carries" tofix. The law now describes every writing verb with no exception. Status unchanged, and it says so —create's default width is what makes L9 partial and the removal does not touch it.root-model.mdpulls two ways. "Every command that takes a page accepts a pristine registered file … the exception isfix" loses its exception, a real simplification. But "a wrongspaceis visible andfixrecovers it" was the recovery half of the argument for keepingurlout of the project file — the asymmetry survives and only the remedy is restated honestly.cmd/check/check.goexplained the half-and-half warning with "fixmoving the keys is the remedy", whichfixnever did — it refused a manifest-only file outright. Wrong before this change and wrong differently after; the comment now says the remedy is manual and why. Found only by the finalgit ls-files | xargs grepsweep, since a code comment was outside the doc survey.confluence/labels.mdloses the adoption bullet and keeps the block-style round-trip bullet, re-attributed to the frontmatter writer thatcreate's persist step still exercises.README.md,markdown_file.md,json-output.md,CLAUDE.md— rows, lists and paragraphs, detailed in the commit message. The README deliberately does not mention the removal: nobody ever ran this command, so a "was removed" note in a 50,000-foot overview is noise for its audience.CONTRIBUTING.mdandinternal/completionneeded nothing — their apparent hits were the conventional-commit typefixand "fixed vocabulary"._plans/is untouched by design. It records what was built,004_fix-subcommandincluded, and rewriting it would make the record lie.One thing worth reviewing in the history
f043d0dwas amended. The first version staged only the deletions, becausegit addwas run with2>/dev/nullover a pathspec that included the already-git rm'dcmd/fix— the stale path made git reject the whole invocation,git status --shortshowed the rest unstaged in its second column, and that went unread. So the commit landed withcmd/fix/gone andcmd/root.gostill importing it, which does not build. Amended, and then verified green in isolation (git stash+make check) rather than assumed. Recorded in the plan's "As built" section.Verification
make checkgreen onf043d0din isolation and on the branch tip.markfluence --helpno longer listsfix;markfluence fix docs/a.mdexits 2 with cobra'sunknown command "fix" for "markfluence", suggestingfind.