Skip to content

docs: two bugs from the phase 1b review, and the measured 1b verdicts - #370

Merged
David-Noble-at-work merged 1 commit into
developfrom
docs/audit-bugs-and-1b-verdicts
Aug 11, 2026
Merged

David-Noble-at-work merged 1 commit into
developfrom
docs/audit-bugs-and-1b-verdicts

Conversation

@David-Noble-at-work

Copy link
Copy Markdown
Contributor

Records two correctness bugs found while reviewing cmd/lore and cmd/writ for the issue #365 phase 1b decomposition, plus the measured verdicts for all nine phase 1b targets.

Both bugs are their own issues and plans rather than folded into phase 1b — a semantic fix hidden inside an extract-method diff is exactly what the #312 discipline forbids.

Issue #368 — lore deploy silently skips an unloadable manifest

parseLoreDeployConfig logs the manifest.Load failure and continues, which is why its error return is dead and why the //nolint:unparam on it documents a consequence as though it were an intention.

  • lore deploy @typo.yaml some-package — deploys some-package, exits 0, having fulfilled half the request. CI wrapping this goes green.
  • lore deploy @typo.yaml — reports "no packages to deploy", which is the wrong cause.

Parsing completes before any deployment executes, so aborting costs nothing already done.

Issue #369 — writ segment matching ignores its own suffix grammar

The grammar is [<project>|common][.<os>][.<distro>][.<arch>][.<custom>...] and segment.go:18 states it as the contract. ParseDirName discards position; Match tests each suffix against one flat set of every dimension's values.

  • Order is never validated — common.arm64.Darwin is accepted.
  • Position never selects a dimension — common.desktop.Darwin matches with ROLE=desktop.
  • Specificity is a suffix count, so it cannot rank DISTRO above OS. common.Unix and common.Darwin both measure 1 on a Mac and tie.
  • Ties resolve via sort.Slice, which is unstable — the winner is undefined by contract.

Phase 1b verdicts

All nine read in full and measured. Ruling 5 is added — treatment decided per function by whether it reads as steps — which produced five treatments rather than one:

Verdict Functions
Extract (steps) promptForProvider, main, runOnboard, generateManifest
Extract (named operation) extractFromFunction, buildMultiSource
Flatten findPackages
Argue the suppression buildTree
Delete the directive applyGraphModifications — gocyclo 7, gocognit 8, under both thresholds

buildMultiSource is deferred to #369: its collision predicate at builder.go:280 has zero coverage (make cover reports builder.go:280.45,282.7 1 0) and is reachable only through a tie whose winner is undefined.

Phase 1a is marked completed — PR #367, bare nolint directives 18 → 9.

Documentation only; no code changes.

…verdicts

Reviewing cmd/lore and cmd/writ for the issue #365 phase 1b decomposition
surfaced two correctness defects. Both are recorded as their own issues and
plans rather than folded into behavior-preserving refactoring work, where a
semantic change would hide inside an extract-method diff.

Issue #368 — lore deploy silently skips a manifest it cannot load.
parseLoreDeployConfig logs the failure and continues, so the error return is
dead and the //nolint:unparam on it documents a consequence as an intention.
"lore deploy @typo.yaml some-package" deploys some-package and exits 0, having
fulfilled half the request; "lore deploy @typo.yaml" reports "no packages to
deploy", which is the wrong cause. Parsing precedes all deployment, so aborting
costs nothing already done.

Issue #369 — writ segment matching ignores its own suffix grammar. The grammar
is [<project>|common][.<os>][.<distro>][.<arch>][.<custom>...] and segment.go:18
states it, but ParseDirName discards position and Match tests each suffix against
one flat set of every dimension's values. So order is never validated
(common.arm64.Darwin is accepted), position never selects a dimension, and
specificity is a suffix count that cannot rank DISTRO above OS. common.Unix and
common.Darwin both measure 1 on a Mac and tie; the tie is then resolved by
sort.Slice, which is unstable, so the winner is undefined by contract.

The phase 1b verdicts are recorded with measured gocyclo and gocognit for all
nine targets. Ruling 5 is added: treatment is decided per function by whether it
reads as steps. That produced five treatments rather than one — six extract, one
flattens, one keeps an argued suppression, and applyGraphModifications needs no
work at all (gocyclo 7, gocognit 8, under both thresholds; its directive
suppresses nothing).

buildMultiSource is deferred to #369. Its collision predicate at builder.go:280
has zero coverage, confirmed by make cover, and is reachable only through a tie
whose winner is undefined. A behavior-preserving refactor of a predicate with no
defined behavior is not possible.

Phase 1a is marked completed — PR #367, bare nolint directives 18 to 9.

Documentation only; no code changes.
Issues: #365, #368, #369
@David-Noble-at-work David-Noble-at-work self-assigned this Aug 11, 2026
@David-Noble-at-work
David-Noble-at-work merged commit 089289e into develop Aug 11, 2026
5 checks passed
@David-Noble-at-work
David-Noble-at-work deleted the docs/audit-bugs-and-1b-verdicts branch August 11, 2026 22:44
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