Skip to content

fix(test): make clone stage dominance independent of real wall-time races - #1659

Merged
DecisionNerd merged 4 commits into
mainfrom
fix/1650-deterministic-clone-stage-clock
Sep 30, 2026
Merged

DecisionNerd merged 4 commits into
mainfrom
fix/1650-deterministic-clone-stage-clock

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1650.

Problem

hub_clone::tests::both_identity_forms_import_and_reopen_the_same_real_project injected a two-second thread::sleep into one stage per case and asserted that stage had the largest real wall time. Real import, filesystem work, and scheduling have no two-second bound, so under load PortableImport could out-dominate the injected NetworkTransport stage. That's an invalid timing assumption, not a clone defect. The failure was observed while validating #1623.

Change

  • Add a CloneClock (crates/graphforge-cli/src/hub_clone.rs). Production code always uses Monotonic, which keeps the unchanged Instant::now()/thread::sleep behavior. The Manual variant is #[cfg(test)] only.
  • The five injected-work attribution cases run on an isolated manual clock that only the injected work advances. They still perform real clones, imports, receipts, handoffs and facade reopens.
  • Assertions are tightened, not weakened:
    • the expected dominant component matrix is unchanged;
    • the dominant stage must equal exactly 2_000_000_000 ns;
    • stages must not overlap or reverse;
    • finished_ns must equal the end of the last stage.
  • No sleeps were increased, no retries or serialization were added, and no assertions were removed.

Validation

  • cargo fmt --all -- --check: clean.
  • cargo clippy -p graphforge-cli -- -D warnings, matching the CI Rust Quality gate command: clean. With --all-targets there are pre-existing lint errors in unrelated test files (maintenance_cli/tests.rs, research_cli/interchange.rs, tests/{branches,research_journey,slices}.rs) that also fail on main. None are in hub_clone.rs.
  • python3 scripts/test_environment.py -- cargo test -p graphforge-cli --lib hub_clone: 21 passed, 0 failed, 1 ignored, in 0.36 s. The attribution cases no longer spend about 10 s of real sleeps.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: CurateLabs/graphforge/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: b099d45b-7ba3-4221-ab70-721cadd7a73d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added core Core source code changes documentation Improvements or additions to documentation ci-cd CI/CD configuration changes tooling Developer tooling and automation labels Sep 30, 2026
@DecisionNerd
DecisionNerd added this pull request to the merge queue Sep 30, 2026
@DecisionNerd

Copy link
Copy Markdown
Contributor Author

Independent review and local validation for the #1623 blocker:

  • make pre-push passed at 6231fff43faeb3a1987841583d971d611ac1a880 after refreshing onto the then-current main. Native Rust coverage, rebuilt Python/Node bindings, wrapper coverage, and final thresholds passed (/tmp/gf-1650-prepush-refreshed.log). Rust quality reused compatible validated evidence.
  • The current PR code diff at a6631ab12abe126c4f232d76a8dd6164d5005967 is identical to that independently reviewed change; only its main integration advanced. It still changes only hub_clone.rs.
  • The expected dominant-component matrix, real clone/import/reopen proof, and fail-open equivalence remain. Manual attribution time is isolated; production monotonic timing is unchanged.
  • Current exact-head CI Gate passed; mergeStateStatus is CLEAN, there are no review threads, and the sole closing reference is fix(test): make clone stage dominance independent of real wall-time races #1650.

The earlier concurrency lane failed during dependency-index transport before tests ran; its HTTP/2 framing error remains recorded in run 36659106567. The current refreshed-tree run completed the concurrency lane successfully.

Merged via the queue into main with commit 78de8ce Sep 30, 2026
22 checks passed
@DecisionNerd
DecisionNerd deleted the fix/1650-deterministic-clone-stage-clock branch September 30, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-cd CI/CD configuration changes core Core source code changes documentation Improvements or additions to documentation tooling Developer tooling and automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(test): make clone stage dominance independent of real wall-time races

1 participant