chenwei791129 opened a new issue, #9187: URL: https://github.com/apache/devlake/issues/9187
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/devlake/issues?q=is%3Aissue) and found no similar issues. I searched open and closed issues and PRs, including related comments and patches. Similar symptoms have been reported, but I did not find a report or PR covering this specific refdiff cache behavior after missing parent history is collected. Related fixes and their different scope are listed below. ### What happened `calculateDeploymentCommitsDiff` can save an incorrect result when DevLake's stored commit history is incomplete. After the missing parent relationships are collected, the old result is still treated as finished, including during **Collect Data in Full Refresh Mode**. Here is a small example. Letters are placeholders, not real commit SHAs. Lines show commits leading to newer commits, from left to right. ```text Complete Git history: R --- H --- A -------- B \ / ----- F ------ A = previous successful production deployment B = next successful production deployment Already in A: R, H, A New in B: F, B ``` Suppose DevLake has not collected the edge between A and its parent H: ```text DevLake's incomplete stored graph: R --- H [missing edge] A --- B \ / ------------ F ---------- From A, DevLake can only find A. From B, it can still find F, H, and R through the other branch. Expected new commits: B, F Calculated commits: B, F, H, R ``` Refdiff saves the incorrect result and writes a completion marker for `(new=B, old=A)`. When the missing edge is later collected, the marker causes the pair to be skipped. The incorrect rows remain in `commits_diffs`. This can link old PRs/MRs to a much later deployment and greatly inflate Lead Time for Changes. In an affected instance, Full Refresh collected the missing parent data but did not repair the cached diff. Removing **both** the affected pair's diff rows and completion marker, then rerunning refdiff and DORA, corrected the metric. A later normal incremental run preserved the correction. This report focuses on refdiff's handling of incomplete data and its cache. The exact reason the collector initially missed the parent data is not established here. ### What do you expect to happen - If required commit history is missing, collect it or report an incomplete result. Do not mark an unverified diff as finished. - When the parent graph is corrected, invalidate or revalidate affected cached pairs. Full Refresh should not silently preserve a known incorrect result. - Recalculation should replace the pair's old result, so commits that no longer belong in the diff are removed. ### How to reproduce **Graph calculation:** use the example above with `CommitNodeGraph`. 1. Add the parent edges `H -> R`, `F -> H`, `B -> A`, and `B -> F`. Here, `child -> parent` means a call to `AddParent(child, parent)`. 2. Do not add `A -> H` yet. 3. Call `CalculateLostSha("A", "B")`. It returns `B, F, H, R` without reporting incomplete history. 4. Add `A -> H` and call it again. It now returns the correct result, `B, F`. **Cache behavior:** in a disposable test project, create two successful production deployment records for A and B, with B's previous successful deployment pointing to A. Make the example commits and parent edges available through that project's repository mapping. 1. Run `calculateDeploymentCommitsDiff` while `A -> H` is missing. The incorrect diff is stored and the pair is marked finished. 2. Add the missing parent edge. 3. Rerun the task for the same pair, including with `fullSync=true`. The completion-marker check skips the pair, so the old diff remains. The graph calculation was reproduced locally using the upstream code. The stale-cache behavior was observed on the affected instance; the small example is a synthetic illustration, not its private data. <details> <summary>Small graph-only reproducer</summary> Save this as `incomplete_graph_test.go` in `backend/plugins/refdiff/utils`, then run: ```sh go test -v commit_node_graph.go incomplete_graph_test.go ``` ```go package utils import "testing" func TestIncompleteDeploymentHistory(t *testing.T) { g := NewCommitNodeGraph() g.AddParent("H", "R") g.AddParent("F", "H") g.AddParent("B", "A") g.AddParent("B", "F") // A is not a root commit, but its parent edge has not been collected. before, _, _ := g.CalculateLostSha("A", "B") g.AddParent("A", "H") after, _, _ := g.CalculateLostSha("A", "B") t.Logf("missing A -> H: %v", before) t.Logf("after collecting A -> H: %v", after) if len(before) != 4 || len(after) != 2 { t.Fatalf("unexpected result: before=%v after=%v", before, after) } } ``` Output: ```text missing A -> H: [B F H R] after collecting A -> H: [B F] ``` This test documents the current graph behavior. It does not test the database cache or mean that the bug is fixed. </details> ### Anything else Relevant code in `v1.0.3-beta15`: - [`CalculateLostSha`](https://github.com/apache/devlake/blob/v1.0.3-beta15/backend/plugins/refdiff/utils/commit_node_graph.go) treats missing parent information as the end of history and does not return a completeness error. - [`calculateDeploymentCommitsDiff`](https://github.com/apache/devlake/blob/v1.0.3-beta15/backend/plugins/refdiff/tasks/deployment_commit_diff_calculator.go) skips pairs with completion markers, without checking graph changes or `FullSync`. - [`BatchSave`](https://github.com/apache/devlake/blob/v1.0.3-beta15/backend/helpers/pluginhelper/api/batch_save.go) uses `CreateOrUpdate`. Removing only the completion marker is therefore not enough: recalculation does not delete old diff rows that are absent from the new result. The graph algorithm and deployment-diff task have the same logic in `v1.0.3-beta18` and the main revision reviewed. This was a source comparison, not a runtime test on those versions. Related reports: - #8434, #8818, and #8857 concern missing commit data. This report starts after a data gap exists and focuses on refdiff's incorrect result and stale cache. Merged PR #8959 improves GitLab MR commit collection; it does not repair refdiff's cache. - #8188, #8249, and #8790 describe similar symptoms of incorrect PR-to-deployment mapping. **Merged PR #8942** adds a two-phase DORA lookup: direct SHA match, then diff-based fallback. The beta15 DORA source already contains this fix, but the fallback still reads `commits_diffs`. The PR does not invalidate or replace refdiff's cached results. - #9182 / #9183 concern repeated calculation when there is **no** previous successful deployment; PR #9183 is still open. Here, both A and B exist and both SHAs are non-empty; the problem is that an incorrect result is **not** recalculated. All commit labels and diagrams above are synthetic. No internal project names, URLs, commit SHAs, deployment IDs, dates, logs, or credentials are included. ### Version v1.0.3-beta15 (MySQL 8, GitLab integration) ### Are you willing to submit PR? - [ ] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
