Skip to content

fix(sync): stop a squashed sync from undoing itself, and unduplicate the corpus control - #107

Merged
rodrigorjsf merged 2 commits into
mainfrom
fix/sync-followups
Aug 5, 2026
Merged

rodrigorjsf merged 2 commits into
mainfrom
fix/sync-followups

Conversation

@rodrigorjsf

Copy link
Copy Markdown
Owner

Follow-up to #104, the 2.5.3 upstream sync. Three things that could not ride
that PR: two findings from its review passes, which finished after the merge
landed, and one lesson from how the merge itself was performed.

The sync PR was squashed, and that undid the sync

#104 merged as a squash, so main kept the merged tree and lost the record that
upstream/main had ever been merged:

$ git log -1 --format='%h parents: %p' 9426796
9426796 parents: c990bbc          ← one parent

$ git merge-base upstream/main 9426796
a818f50...                        ← did not advance

$ git rev-list --count a818f50..upstream/main
11                                ← still "11 behind", on a tree containing all 11

Nothing was lost from the tree — git diff 8516ecf 9426796 was empty, python_ast.py
was present, version = "2.5.3". Only the lineage went.

The cost lands on the next sync. Step 1 of /upstream-sync measures drift from the
merge-base, so it would have re-merged those eleven commits into a tree that already
contains them: conflicts on identical content, and a step-5 ledger with no attributable
delta left in it — which is the one signal that tells you a resolution went wrong.
#105, the drift-measurement issue, would have reported a permanent non-zero count.

Repaired on main in 208cfee with git merge -s ours upstream/main, which records
the second parent and changes no file. Verified before pushing: tree byte-identical to
9426796's, two parents, git rev-list --count now 0, merge-base now 0562b96
(upstream's tip), full suite green.

This PR adds step 7 to the skill so the next sync does not repeat it. The exception is
worth stating explicitly, because the default is right everywhere else: Ticket PRs are
squashed on purpose and CLAUDE.md says so — a sync PR is the one that must not be.

Two review findings from #104

The corpus control restated the Framework manifest vocabulary.
test_every_non_target_directory_is_excluded_and_is_really_not_a_target carried a
literal tuple of "pom.xml", "build.gradle", "build.gradle.kts", "pyproject.toml"
while src/skillspector/framework.py already owns that knowledge. Two vacuous-green
consequences: adding a Framework manifest kind leaves the control passing while it
silently stops discriminating, and the literal list restates as a name what framework
matches by prefix. It now calls _is_jvm_build_file and
_is_python_requirement_file.

Verified non-vacuous — the control rejects all six mutants: a SKILL.md fixture, a
json-only fixture, a build.gradle fixture, a pom.xml fixture, a pyproject.toml
fixture, and a family parent.

A hardcoded corpus count in new prose. The same docstring wrote 35, against the
recorded rule "Snapshot counts rot in prose; write 'every committed snapshot', never a
number."
Now reads "every corpus member".

Not in this PR

The record-only corrections to #104's step-5 ledger — two misattributed test-file
citations, aefa482 unlabelled, and #100's "seven of the eleven" being 4 rather than
7 — are
commented on #104
rather than restated here. The code was unaffected; only the citations were wrong.

/license-audit against main is still outstanding from #104's step 6. It is
user-invoked.

Validations

  • make test-unit — 2831 passed, 13 skipped, 38 deselected, 4 xfailed
  • make lint, make format-check — clean
  • make update-snapshots — tree clean, no snapshot moved

The NON_TARGET_DIRS control restated the Framework manifest vocabulary as a
literal tuple, duplicating knowledge src/skillspector/framework.py already
owns. Two consequences, both the vacuous-green shape this repo guards
against: the day a Framework gains a manifest kind the control keeps passing
while silently ceasing to discriminate, and the literal list missed
build.gradle.kts, which framework matches by prefix rather than by name.

It now calls _is_jvm_build_file and _is_python_requirement_file. Verified
non-vacuous against six mutants -- a SKILL.md fixture, a json-only fixture, a
build.gradle fixture, a pom.xml fixture, a pyproject.toml fixture and a
family parent -- all rejected.

Also drops the hardcoded corpus count from the new docstring, per "Snapshot
counts rot in prose; write 'every committed snapshot', never a number".

Signed-off-by: Rodrigo França <rodrigo_rjsf@hotmail.com>
PR #104 was squash-merged, which kept the merged tree and dropped the record
that upstream/main had been merged at all. The merge-base stayed at a818f50
and step 1 kept measuring 11 commits behind on a tree that contained all
eleven, so the next sync would have re-merged them into a tree that already
had them -- conflicts on identical content, and a step-5 ledger with no
attributable delta left.

Repaired on main in 208cfee with git merge -s ours upstream/main, which
records the second parent and changes no file. Merge-base now advances to
upstream's tip and the count is 0.

New step 7 states the exception -- Ticket PRs are squashed on purpose, a sync
PR is the one that must not be -- gives the two commands that detect a lost
parent, and gives the repair with its three verifications. Applied Learning
carries the one-line form.

Signed-off-by: Rodrigo França <rodrigo_rjsf@hotmail.com>
@rodrigorjsf
rodrigorjsf merged commit 12b6e61 into main Aug 5, 2026
5 checks passed
@rodrigorjsf
rodrigorjsf deleted the fix/sync-followups branch August 5, 2026 20:39
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