Restore recipe hierarchy rows in SourcesFileResults#6764
Conversation
PR #6022 removed the recursive hierarchy recording in RecipeRunCycle to fix non-summable estimatedTimeSaving values. This had the side effect of breaking the Sankey visualization, which needs (parentRecipe, recipe) rows at every level to draw the full drill-down from top-level recipes through to leaf recipes that made changes. Restore the recursive recordSourceFileResult() method but pass 0 for estimatedTimeSaving on hierarchy rows, so that only leaf recipes carry effort values and the column remains summable.
|
Any immediate thoughts here @pstreef given your involvement with |
|
yeah, perhaps we should just have a different data-table if this is just for 1 visualization? The reason for removal was more than just being able to sum. It was also confusing. I would involve @jkschneider and/or @sambsnyd here for opinions |
sambsnyd
left a comment
There was a problem hiding this comment.
I think this is reasonable. Peter's comments about it being a bit confusing are perfectly fair, but recording the hierarchy is a more accurate description of what happens when a recipe makes changes. Maybe mentioning the nature of the 0-savings rows in the data table description would help - although we don't surface those descriptions many places.
|
Thanks both! I'll merge as we're one to one restoring what was removed in #6022, with the exception of the effort which is explicitly set to zero and documented as such. I want to acknowledge that Peter suggested an alternative data table, but seeing how it would have to hook into RecipeRunCycle, and be attached for every run I think it'd be better not to introduce a near duplicate if the zero-time rows here work as well. |
Summary
RecipeRunCyclethat was removed in Align estimated time savings betweenResultandSourcesFileResults#60220forestimatedTimeSavingso the column remains summableProblem
(parentRecipe, recipe)pairs from theSourcesFileResultsdata table. PR Align estimated time savings betweenResultandSourcesFileResults#6022 removed the recursive recording of hierarchy rows to fix non-summableestimatedTimeSavingvalues. This caused the Sankey to only show 1-2 levels instead of the full drill-down (5+ levels for migrations like Java 25).Solution
Restore the
recordSourceFileResult()method but pass0LforestimatedTimeSavingon all non-leaf hierarchy rows. Only the leaf recipe that actually made the change carries its effort value. This gives the Sankey all the edges it needs while keeping the effort column summable.Test plan
SourceFileResultsTest.hierarchicalpasses unchangedRecipeEstimatedEffortTest.estimatedTimeSavingsForMultipleRecipespasses unchanged (effort sums still correct)SourceFileResultsTest.deepHierarchyRecordsAllLevelsverifies hierarchy rows are produced with zero effort