Skip to content

Commit 93ee129

Browse files
Explain hygiene cleanup revalidation blockers (#852)
* Explain hygiene cleanup revalidation blockers * Fix hygiene report lint formatting * Resolve hygiene report PHPCS alignment --------- Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 8d15e8d commit 93ee129

7 files changed

Lines changed: 185 additions & 35 deletions

inc/Cli/Commands/WorkspaceCommand.php

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5109,14 +5109,18 @@ private function render_workspace_hygiene_report( array $report, array $assoc_ar
51095109
$this->format_items(
51105110
array_map(
51115111
fn( $row ) => array(
5112-
'handle' => $row['handle'] ?? '',
5113-
'branch' => $row['branch'] ?? '',
5114-
'signal' => $row['signal'] ?? '',
5115-
'size' => $row['size_human'] ?? '',
5112+
'handle' => $row['handle'] ?? '',
5113+
'branch' => $row['branch'] ?? '',
5114+
'signal' => $row['signal'] ?? '',
5115+
'size' => $row['size_human'] ?? '',
5116+
'dirty' => array_key_exists('dirty', (array) $row) ? ( null === $row['dirty'] ? 'unknown' : $row['dirty'] ) : '',
5117+
'unpushed' => array_key_exists('unpushed', (array) $row) ? ( null === $row['unpushed'] ? 'unknown' : $row['unpushed'] ) : '',
5118+
'fresh_status' => $row['fresh_revalidation_status'] ?? '',
5119+
'fresh_blockers' => implode(',', array_map('strval', (array) ( $row['fresh_revalidation_blockers'] ?? array() ))),
51165120
),
51175121
$candidates
51185122
),
5119-
array( 'handle', 'branch', 'signal', 'size' ),
5123+
array( 'handle', 'branch', 'signal', 'size', 'dirty', 'unpushed', 'fresh_status', 'fresh_blockers' ),
51205124
array( 'format' => 'table' ),
51215125
'handle'
51225126
);
@@ -5126,6 +5130,9 @@ private function render_workspace_hygiene_report( array $report, array $assoc_ar
51265130
WP_CLI::log('');
51275131
WP_CLI::log('Suggested cleanup review:');
51285132
WP_CLI::log( (string) $report['suggested_cleanup_command']);
5133+
if ( ! empty($cleanup['expected_outcome']) ) {
5134+
WP_CLI::log( (string) $cleanup['expected_outcome']);
5135+
}
51295136
}
51305137

51315138
if ( ! empty($report['suggested_size_command']) ) {

inc/Cli/WorkspaceCompactOutput.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ public static function hygiene_report( array $report ): array {
9595
'cleanup' => array(
9696
'blocker_probe_source' => $cleanup['blocker_probe_source'] ?? null,
9797
'blocker_counts' => $cleanup['blocker_counts'] ?? null,
98+
'expected_outcome' => $cleanup['expected_outcome'] ?? null,
9899
'summary' => (array) ( $cleanup['summary'] ?? array() ),
99100
'biggest_candidates' => self::compact_rows( (array) ( $cleanup['biggest_candidates'] ?? array() ) ),
100101
),
@@ -311,12 +312,12 @@ private static function compact_row( array $row ): array {
311312
'path' => $row['path'] ?? null,
312313
'pr_url' => $row['pr_url'] ?? null,
313314
);
314-
foreach ( array( 'size_bytes', 'artifact_size_bytes', 'bytes_reclaimed', 'dirty', 'unpushed', 'age_days', 'created_at', 'liveness' ) as $field ) {
315+
foreach ( array( 'size_bytes', 'artifact_size_bytes', 'bytes_reclaimed', 'dirty', 'unpushed', 'age_days', 'created_at', 'liveness', 'fresh_revalidation_status' ) as $field ) {
315316
if ( array_key_exists( $field, $row ) ) {
316317
$compact[ $field ] = $row[ $field ];
317318
}
318319
}
319-
foreach ( array( 'size_status', 'size_accounting', 'fields_skipped' ) as $field ) {
320+
foreach ( array( 'size_status', 'size_accounting', 'fields_skipped', 'fresh_revalidation_blockers', 'fresh_revalidation_checks' ) as $field ) {
320321
if ( array_key_exists( $field, $row ) ) {
321322
$compact[ $field ] = $row[ $field ];
322323
}

inc/Workspace/WorkspaceHygieneReport.php

Lines changed: 80 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -664,13 +664,13 @@ private function summarize_workspace_worktrees( array $worktrees, ?array $cleanu
664664
$summary['duplicates'] = $duplicates;
665665

666666
if ( null !== $cleanup ) {
667-
$by_reason = (array) ( $cleanup['summary']['skipped_by_reason'] ?? array() );
668-
$dirty_blockers = (int) ( $by_reason['dirty_worktree'] ?? 0 );
669-
$unpushed_blockers = (int) ( $by_reason['unpushed_commits'] ?? 0 );
670-
$summary['protected_dirty'] = $dirty_blockers;
671-
$summary['protected_unpushed'] = $unpushed_blockers;
672-
$summary['missing_metadata'] = (int) ( $by_reason['missing_metadata'] ?? 0 );
673-
$summary['external'] = max($summary['external'], (int) ( $by_reason['external_worktree'] ?? 0 ));
667+
$by_reason = (array) ( $cleanup['summary']['skipped_by_reason'] ?? array() );
668+
$dirty_blockers = (int) ( $by_reason['dirty_worktree'] ?? 0 );
669+
$unpushed_blockers = (int) ( $by_reason['unpushed_commits'] ?? 0 );
670+
$summary['protected_dirty'] = $dirty_blockers;
671+
$summary['protected_unpushed'] = $unpushed_blockers;
672+
$summary['missing_metadata'] = (int) ( $by_reason['missing_metadata'] ?? 0 );
673+
$summary['external'] = max($summary['external'], (int) ( $by_reason['external_worktree'] ?? 0 ));
674674

675675
$probe_source = ! empty($cleanup['inventory_only']) ? 'inventory_known' : 'fresh_probe';
676676
$summary['protected_count_probe_source'] = $probe_source;
@@ -721,22 +721,23 @@ private function summarize_workspace_cleanup( ?array $cleanup, ?array $error, ar
721721
}
722722
unset($candidate);
723723
usort($candidates, fn( $a, $b ) => (int) ( $b['size_bytes'] ?? 0 ) <=> (int) ( $a['size_bytes'] ?? 0 ));
724-
$summary = (array) ( $cleanup['summary'] ?? array() );
725-
$skipped_reason = (array) ( $summary['skipped_by_reason'] ?? array() );
726-
$probe_source = ! empty($cleanup['inventory_only']) ? 'inventory_known' : 'fresh_probe';
727-
$empty_blockers = array(
724+
$summary = (array) ( $cleanup['summary'] ?? array() );
725+
$skipped_reason = (array) ( $summary['skipped_by_reason'] ?? array() );
726+
$probe_source = ! empty($cleanup['inventory_only']) ? 'inventory_known' : 'fresh_probe';
727+
$candidates = $this->annotate_workspace_cleanup_candidates_for_hygiene($candidates, $probe_source);
728+
$empty_blockers = array(
728729
'dirty_worktree' => 0,
729730
'unpushed_commits' => 0,
730731
);
731-
$blocker_counts = array(
732+
$blocker_counts = array(
732733
'inventory_known' => $empty_blockers,
733734
'fresh_probe' => $empty_blockers,
734735
);
735736
$blocker_counts[ $probe_source ] = array(
736737
'dirty_worktree' => (int) ( $skipped_reason['dirty_worktree'] ?? 0 ),
737738
'unpushed_commits' => (int) ( $skipped_reason['unpushed_commits'] ?? 0 ),
738739
);
739-
return array(
740+
$result = array(
740741
'included' => true,
741742
'dry_run' => true,
742743
'skip_github' => true,
@@ -748,6 +749,58 @@ private function summarize_workspace_cleanup( ?array $cleanup, ?array $error, ar
748749
'skipped_by_reason' => $skipped_reason,
749750
'candidates_by_signal' => $summary['candidates_by_signal'] ?? array(),
750751
);
752+
753+
$expected_outcome = $this->workspace_cleanup_expected_outcome($summary, ! empty($cleanup['inventory_only']));
754+
if ( '' !== $expected_outcome ) {
755+
$result['expected_outcome'] = $expected_outcome;
756+
}
757+
758+
return $result;
759+
}
760+
761+
/**
762+
* Add concise fresh-revalidation context to cleanup candidate examples.
763+
*
764+
* @param array<int,array<string,mixed>> $candidates Candidate rows.
765+
* @param string $probe_source inventory_known|fresh_probe.
766+
* @return array<int,array<string,mixed>>
767+
*/
768+
private function annotate_workspace_cleanup_candidates_for_hygiene( array $candidates, string $probe_source ): array {
769+
foreach ( $candidates as &$candidate ) {
770+
if ( 'inventory_known' === $probe_source ) {
771+
$candidate['fresh_revalidation_status'] = (string) ( $candidate['safety_probe_status'] ?? 'not_run_inventory_only' );
772+
$candidate['fresh_revalidation_blockers'] = array( 'pending_fresh_revalidation' );
773+
$candidate['fresh_revalidation_checks'] = array( 'dirty_worktree', 'unpushed_commits', 'primary_or_protected_worktree', 'containment_failure' );
774+
continue;
775+
}
776+
777+
$candidate['fresh_revalidation_status'] = 'passed';
778+
if ( ! array_key_exists('unpushed', $candidate) ) {
779+
$candidate['unpushed'] = 0;
780+
}
781+
}
782+
unset($candidate);
783+
784+
return $candidates;
785+
}
786+
787+
/**
788+
* Explain what the suggested cleanup/apply path can do when hygiene has no fresh-safe rows.
789+
*
790+
* @param array<string,mixed> $summary Cleanup summary.
791+
* @param bool $inventory_only Whether hygiene used inventory-only cleanup.
792+
*/
793+
private function workspace_cleanup_expected_outcome( array $summary, bool $inventory_only ): string {
794+
$fresh_safe = (int) ( $summary['fresh_safe_removable_count'] ?? 0 );
795+
$pending = (int) ( $summary['cleanup_buckets'][ WorktreeCleanupClassifier::BUCKET_CLEANUP_ELIGIBLE_UNPROBED ] ?? 0 );
796+
if ( ! $inventory_only || $fresh_safe > 0 || $pending <= 0 ) {
797+
return '';
798+
}
799+
800+
return sprintf(
801+
'Expected outcome: hygiene found %d cleanup-eligible row(s) pending fresh revalidation and 0 fresh-safe removals. The bounded apply path will remove only rows that still pass dirty, unpushed, primary/protected, and containment checks; blockers are skipped with fresh reason codes.',
802+
$pending
803+
);
751804
}
752805

753806
/**
@@ -764,18 +817,18 @@ private function build_workspace_fast_stats( array $worktrees, ?array $cleanup,
764817
$cleanup_summary = (array) ( $cleanup['summary'] ?? array() );
765818

766819
$counts = array(
767-
'total_candidates' => count($worktrees),
768-
'cleanup_eligible_unprobed_count' => count($cleanup_candidates),
769-
'valid_clean_count' => 0,
770-
'valid_dirty_count' => 0,
771-
'inventory_known_dirty_count' => 0,
772-
'inventory_known_blocker_count' => 0,
773-
'fresh_probed_blocker_count' => 0,
774-
'invalid_broken_orphan_count' => 0,
775-
'unmanaged_skipped_count' => 0,
776-
'dirty_probe_skipped_count' => 0,
777-
'known_worktree_count' => 0,
778-
'known_primary_count' => 0,
820+
'total_candidates' => count($worktrees),
821+
'cleanup_eligible_unprobed_count' => count($cleanup_candidates),
822+
'valid_clean_count' => 0,
823+
'valid_dirty_count' => 0,
824+
'inventory_known_dirty_count' => 0,
825+
'inventory_known_blocker_count' => 0,
826+
'fresh_probed_blocker_count' => 0,
827+
'invalid_broken_orphan_count' => 0,
828+
'unmanaged_skipped_count' => 0,
829+
'dirty_probe_skipped_count' => 0,
830+
'known_worktree_count' => 0,
831+
'known_primary_count' => 0,
779832
);
780833

781834
foreach ( $worktrees as $row ) {
@@ -843,8 +896,8 @@ private function build_workspace_fast_stats( array $worktrees, ?array $cleanup,
843896
return array(
844897
'mode' => $include_worktree_status ? 'full_git_status' : 'cheap_metadata_first',
845898
'partial' => ! $include_worktree_status || empty($size_report['scan_complete']),
846-
'safety_probe_status' => $safety_probe_status,
847-
'safety_probe_note' => $safety_probe_note,
899+
'safety_probe_status' => $safety_probe_status,
900+
'safety_probe_note' => $safety_probe_note,
848901
'status_probe_required_for_summary' => ! $include_worktree_status,
849902
'counts' => $counts,
850903
'estimated_reclaimable_bytes' => $estimated_reclaimable,

inc/Workspace/WorktreeCleanupCandidateClassifier.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ public static function classify_merge_signal_path( array $context, ?array $signa
9292
$base,
9393
array(
9494
'dirty' => (int) ( $context['dirty_count'] ?? 0 ),
95+
'unpushed' => 0,
9596
'cleanup_reasons' => array_values(array_filter(array( $signal['signal'] ?? '', $signal['reason'] ?? '' ))),
9697
'liveness' => (string) ( $context['liveness'] ?? '' ),
9798
),

tests/workspace-compact-output.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ function compact_output_large_rows( int $count ): array {
3838
}
3939

4040
$large_rows = compact_output_large_rows(40);
41+
$hygiene_candidate_rows = $large_rows;
42+
$hygiene_candidate_rows[0]['dirty'] = null;
43+
$hygiene_candidate_rows[0]['fresh_revalidation_status'] = 'not_run_inventory_only';
44+
$hygiene_candidate_rows[0]['fresh_revalidation_blockers'] = array( 'pending_fresh_revalidation' );
45+
$hygiene_candidate_rows[0]['fresh_revalidation_checks'] = array( 'dirty_worktree', 'unpushed_commits', 'primary_or_protected_worktree', 'containment_failure' );
4146
$cleanup = WorkspaceCompactOutput::cleanup_result(
4247
array(
4348
'success' => true,
@@ -218,6 +223,7 @@ function compact_output_large_rows( int $count ): array {
218223
'locks' => array( 'active' => 2, 'stale' => 40, 'database' => array( 'locks' => $large_rows ) ),
219224
'cleanup' => array(
220225
'blocker_probe_source' => 'inventory_known',
226+
'expected_outcome' => 'Expected outcome: hygiene found 40 cleanup-eligible row(s) pending fresh revalidation and 0 fresh-safe removals.',
221227
'blocker_counts' => array(
222228
'inventory_known' => array(
223229
'dirty_worktree' => 3,
@@ -238,7 +244,7 @@ function compact_output_large_rows( int $count ): array {
238244
'safe_to_remove_now' => 0,
239245
),
240246
),
241-
'biggest_candidates' => $large_rows,
247+
'biggest_candidates' => $hygiene_candidate_rows,
242248
),
243249
'size' => array(
244250
'total_bytes' => 123456,
@@ -263,10 +269,14 @@ function compact_output_large_rows( int $count ): array {
263269
compact_output_assert('inventory_known' === ( $hygiene['cleanup']['blocker_probe_source'] ?? null ), 'Compact hygiene output must expose blocker probe source.');
264270
compact_output_assert(3 === ( $hygiene['cleanup']['blocker_counts']['inventory_known']['dirty_worktree'] ?? null ), 'Compact hygiene output must preserve inventory-known dirty blocker bucket.');
265271
compact_output_assert(0 === ( $hygiene['cleanup']['blocker_counts']['fresh_probe']['dirty_worktree'] ?? null ), 'Compact hygiene output must preserve fresh-probed dirty blocker bucket.');
272+
compact_output_assert(isset($hygiene['cleanup']['expected_outcome']), 'Compact hygiene output must preserve expected cleanup outcome.');
266273
compact_output_assert(40 === ( $hygiene['cleanup']['summary']['cleanup_buckets']['cleanup_eligible_pending_revalidation'] ?? null ), 'Compact cleanup summary must preserve pending-revalidation bucket.');
267274
compact_output_assert(0 === ( $hygiene['cleanup']['summary']['cleanup_buckets']['safe_to_remove_now'] ?? null ), 'Compact cleanup summary must not mark unprobed inventory candidates safe.');
268275
compact_output_assert(123456 === ( $hygiene['size']['total_bytes'] ?? null ), 'Compact hygiene output must preserve size bytes.');
269276
compact_output_assert(40 === ( $hygiene['size']['entry_count'] ?? null ), 'Compact hygiene output must preserve size entry count.');
270277
compact_output_assert(count((array) ( $hygiene['cleanup']['biggest_candidates'] ?? array() )) <= 5, 'Compact hygiene output must sample cleanup candidates.');
278+
compact_output_assert('not_run_inventory_only' === ( $hygiene['cleanup']['biggest_candidates'][0]['fresh_revalidation_status'] ?? null ), 'Compact hygiene candidates must preserve fresh revalidation status.');
279+
compact_output_assert(array( 'pending_fresh_revalidation' ) === ( $hygiene['cleanup']['biggest_candidates'][0]['fresh_revalidation_blockers'] ?? null ), 'Compact hygiene candidates must preserve fresh blocker examples.');
280+
compact_output_assert(in_array('unpushed_commits', (array) ( $hygiene['cleanup']['biggest_candidates'][0]['fresh_revalidation_checks'] ?? array() ), true), 'Compact hygiene candidates must preserve fresh revalidation checks.');
271281

272282
echo "workspace compact output test passed.\n";
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
/**
3+
* Focused coverage for hygiene cleanup candidate fresh-revalidation context.
4+
*/
5+
6+
declare(strict_types=1);
7+
8+
if ( ! defined('ABSPATH') ) {
9+
define('ABSPATH', __DIR__ . '/fixtures/');
10+
}
11+
12+
require_once dirname(__DIR__) . '/inc/Workspace/WorktreeCleanupClassifier.php';
13+
require_once dirname(__DIR__) . '/inc/Workspace/WorkspaceHygieneReport.php';
14+
15+
function hygiene_cleanup_summary_assert( bool $condition, string $message ): void {
16+
if ( ! $condition ) {
17+
throw new RuntimeException($message);
18+
}
19+
}
20+
21+
$hygiene = new class {
22+
use DataMachineCode\Workspace\WorkspaceHygieneReport;
23+
24+
public function annotate( array $candidates, string $source ): array {
25+
$method = new ReflectionMethod($this, 'annotate_workspace_cleanup_candidates_for_hygiene');
26+
return $method->invoke($this, $candidates, $source);
27+
}
28+
29+
public function outcome( array $summary, bool $inventory_only ): string {
30+
$method = new ReflectionMethod($this, 'workspace_cleanup_expected_outcome');
31+
return $method->invoke($this, $summary, $inventory_only);
32+
}
33+
};
34+
35+
$inventory_candidates = $hygiene->annotate(
36+
array(
37+
array(
38+
'handle' => 'repo@cleanup-eligible',
39+
'safety_probe_status' => 'not_run_inventory_only',
40+
'dirty' => null,
41+
),
42+
),
43+
'inventory_known'
44+
);
45+
46+
hygiene_cleanup_summary_assert('not_run_inventory_only' === $inventory_candidates[0]['fresh_revalidation_status'], 'inventory candidates explain that fresh probes have not run');
47+
hygiene_cleanup_summary_assert(array( 'pending_fresh_revalidation' ) === $inventory_candidates[0]['fresh_revalidation_blockers'], 'inventory candidates expose pending fresh blocker');
48+
hygiene_cleanup_summary_assert(in_array('dirty_worktree', $inventory_candidates[0]['fresh_revalidation_checks'], true), 'inventory candidates list dirty revalidation check');
49+
hygiene_cleanup_summary_assert(in_array('unpushed_commits', $inventory_candidates[0]['fresh_revalidation_checks'], true), 'inventory candidates list unpushed revalidation check');
50+
51+
$fresh_candidates = $hygiene->annotate(
52+
array(
53+
array(
54+
'handle' => 'repo@safe',
55+
'dirty' => 0,
56+
),
57+
),
58+
'fresh_probe'
59+
);
60+
61+
hygiene_cleanup_summary_assert('passed' === $fresh_candidates[0]['fresh_revalidation_status'], 'fresh-probed candidates are marked passed');
62+
hygiene_cleanup_summary_assert(0 === $fresh_candidates[0]['unpushed'], 'fresh-probed candidates expose passed unpushed check');
63+
64+
$expected_outcome = $hygiene->outcome(
65+
array(
66+
'fresh_safe_removable_count' => 0,
67+
'cleanup_buckets' => array(
68+
DataMachineCode\Workspace\WorktreeCleanupClassifier::BUCKET_CLEANUP_ELIGIBLE_UNPROBED => 8,
69+
),
70+
),
71+
true
72+
);
73+
74+
hygiene_cleanup_summary_assert(str_contains($expected_outcome, '8 cleanup-eligible row(s) pending fresh revalidation'), 'expected outcome names pending cleanup-eligible rows');
75+
hygiene_cleanup_summary_assert(str_contains($expected_outcome, '0 fresh-safe removals'), 'expected outcome names zero fresh-safe removals');
76+
77+
echo "workspace hygiene cleanup summary test passed.\n";

tests/worktree-cleanup-candidate-classifier.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ function () use ( &$evidence_called ): array {
5454
worktree_cleanup_candidate_assert_same('candidate', $candidate_result['type'], 'merged signal is a candidate');
5555
worktree_cleanup_candidate_assert_same('github-merged-pr', $candidate_result['row']['signal'], 'candidate signal is preserved');
5656
worktree_cleanup_candidate_assert_same('github-merged-pr', $candidate_result['row']['reason_code'], 'candidate reason_code matches signal');
57+
worktree_cleanup_candidate_assert_same(0, $candidate_result['row']['unpushed'], 'fresh safe candidate records the passed unpushed probe');
5758
worktree_cleanup_candidate_assert_same(false, $evidence_called, 'no-signal evidence stays lazy for candidates');
5859

5960
$no_signal_filter = null;

0 commit comments

Comments
 (0)