test(usecases): hotpath coverage on work_rollup and semantic_vector_commit_scale - #791
Conversation
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75de232724
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let read_model = tracedecay_usecases::observability::observatory_read_model( | ||
| database.as_ref(), | ||
| None, | ||
| 0, | ||
| ) |
There was a problem hiding this comment.
In self-verifying runs, this post-measurement observatory_read_model call is the only source of the required label; the actual run_work_rollup_case and settled workload in observability_runtime_contract.rs use execution_topology_rollup_metrics instead. The check can therefore pass even when the measured work-rollup journey records no Hotpath samples at all. Require a label emitted by the real benchmark path rather than synthesizing an unrelated read solely for coverage.
AGENTS.md reference: AGENTS.md:L152-L160
Useful? React with 👍 / 👎.
| // the macros are no-ops and no report may appear; feature on, the guard | ||
| // lifecycle completes with the metrics server forced off and a parseable | ||
| // exit report. Extend this list when the machine itself gains labels. | ||
| const EXPECTED_HOTPATH_LABELS: &[&str] = &[]; |
There was a problem hiding this comment.
Because this expected-label slice is empty, finish only verifies that dropping the guard produced syntactically valid JSON; it succeeds even if the semantic commit workload records zero spans or gauges. This makes the advertised coverage unable to detect missing or broken instrumentation. Add a bounded Hotpath measurement around the actual commit sweep and require its exact label, or stop presenting this run as semantic workload coverage.
AGENTS.md reference: AGENTS.md:L152-L160
Useful? React with 👍 / 👎.
| let prior = std::fs::read(&path).ok(); | ||
| (path, prior) |
There was a problem hiding this comment.
When a feature-off baseline inherits a HOTPATH_OUTPUT_PATH that already contains a report, this reads the entire file into a Vec<u8> retained by coverage until after the final VmHWM sample. Since semantic_vector_commit_scale measures process-wide peak memory and states that every retained byte belongs to the subject, a large prior profile directly inflates and corrupts the result. Compare the file without retaining its contents for the lifetime of the workload.
AGENTS.md reference: AGENTS.md:L196-L198
Useful? React with 👍 / 👎.
…sor/hotpath-serving-slice-tests-862f
…sor/hotpath-serving-slice-tests-862f
…sor/hotpath-serving-slice-tests-862f
Performance Comparison
|
2 participants
Summary
Coverage-only PR for
crates/tracedecay-usecasesbencheswork_rollupandsemantic_vector_commit_scale. Adds a sharedbenches/hotpath_coverage.rshelper so both binaries self-check:HOTPATH_OUTPUT_PATHis set.HOTPATH_METRICS_SERVER_OFF); guard drop emits a JSON report.work_rollupthen asserts the staticusecases.observability.read_modellabel.semantic_vector_commit_scaledrives the uninstrumented in-memoryVectorGenerationStateMachineV1, so it verifies the guard lifecycle only (labels_verified=0is expected, not a fake assertion).Does not expand
.github/workflows/hotpath-profile.yml. Does not restamp existing#[hotpath::measure]. Does not enable hotpath in default/production. Does not merge #707. Diff is the three usecases bench files only.Verification
cargo build -p tracedecay-usecases --benchesand both binaries run feature-off (no report, no 6770/6771 listener).cargo build -p tracedecay-usecases --benches --features hotpath(cold, ~8 min).semantic_vector_commit_scalecompletes (labels_verified=0, report_bytes=622, scratch report cleaned up).work_rolluphung twice in the cloud agent VM (once ~3.5 min spawn abort, once ~29 min timeout). Not used as a CI gate here. Worth a local rerun withHOTPATH_METRICS_SERVER_OFF=1andTRACEDECAY_DATA_DIRpointed at a fresh dir.Test plan
cargo build -p tracedecay-usecases --benchescargo build -p tracedecay-usecases --benches --features hotpathHOTPATH_METRICS_SERVER_OFF=1.