fix(sync): recompute changed components after reverts - #419
Open
mosheabr wants to merge 1 commit into
Open
Conversation
The rsync loop records a component as changed the moment rsync produces a diff. The drift, compliance and prune steps run after it and revert those files, but nothing ever removed the component from the list, so a component whose skills were all held was still reported as updated. Every sync PR has been affected. #417 listed AIQ, CUDA-Q, DeepStream, Megatron-Core, NeMo MBridge, NeMo Relay, NeMo Retriever, TAO Toolkit and cuDF under "Components updated" while changing exactly 8 files, all in skills/doca-telemetry-exporter. Earlier syncs are the same: #409 listed 29 with 3 directories changed, #403 listed 29 with 1, #396 listed 31 with 2. The body contradicts itself, since those components also appear in the held list directly below. Recompute the list from the working tree after every step that can mutate it. Non-component entries are re-established from their own evidence rather than carried over, and a dirty tree can never recompute to "no changes", so a README-only or generated-file sync still opens a PR. This also shrinks the title that #412 had to cap: the length came from naming components that were never updated. Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
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
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.
The problem
The rsync loop records a component as changed the moment rsync produces a diff (line 129). The drift, compliance and prune steps run after it and revert those files, but nothing ever removed the component from the list — every later write is an append, and line 844 only dedupes.
So a component whose skills were all held for signature drift still comes out labelled "updated", contradicting the held list printed directly below it in the same PR body.
This is not a one-off. Measured across four syncs:
#417 named AIQ, CUDA-Q, DeepStream, Megatron-Core, NeMo MBridge, NeMo Relay, NeMo Retriever, TAO Toolkit and cuDF while changing exactly 8 files, all under
skills/doca-telemetry-exporter. Every other component on that list appears in the held section.It also explains the title length that #412 had to cap — the title is built from this same list, so it was long because it named components that were never updated.
The fix
One new step after
Regenerate README tables, beforeBuild sync summary, that rebuilds the list from the working tree once every mutating step has run.Two guards worth calling out:
orphan pruning,plugin catalog) describe changes outsideskills/, so they are re-established from their own evidence rather than carried over.catalog maintenanceentry.The step also logs what it dropped, so the reverted-vs-updated distinction is visible in the run output.
Verification
Extracted the step and ran it against the real
components.dset (36 components) with a working tree manipulated per case:DOCAonlycatalog maintenance✅orphan pruning✅CUDA-Q, DOCA✅CUDA-Q✅