Conversation
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9531879f6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0910ef8ad
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c247cbf45c
ℹ️ 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".
|
Follow-up feedback and thermo-nuclear audit complete. Feedback classification:
Thermo-nuclear full-diff result: no remaining structural blocker. Production is 481 lines and tests 771; neither crosses 1k. The cleanup phases remain intentionally sequential to avoid multiplying Elasticsearch load, public wrappers preserve the existing API, and response validation is centralized in one canonical helper. Verification:
All four review threads are addressed and resolved; the PR is mergeable and current with origin/main. The external Copilot contributor CLA is the sole remaining gate. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9ebc8c1cc
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6136e0d9ec
ℹ️ 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".
|
Final feedback, destructive-path, coverage, and thermo-nuclear audit completed on Feedback classification:
Data-safety proof:
Thermo-nuclear result:
Verification:
The external Copilot contributor CLA remains the only expected non-code gate. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0333421b59
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR updates the backend orphaned-event cleanup job to bound Elasticsearch work to a rolling recent-ingest window (by CreatedUtc), aiming to eliminate periodic CPU spikes while keeping cross-index fan-out behavior intact. It also tightens failure handling for Elasticsearch responses and expands integration test coverage around the new cutoff and health semantics.
Changes:
- Scope orphaned stack/project/organization discovery and delete-by-query operations to events with
CreatedUtc >= (now - 3 days), using a single cutoff captured per job run. - Treat timed-out/invalid/partially failed Elasticsearch search and delete-by-query responses as job failures via centralized validation helpers.
- Update job health reporting to track the last successful completion and add regression tests for cutoff boundaries, missed runs, and failure/health behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Exceptionless.Core/Jobs/CleanupOrphanedDataJob.cs | Adds a rolling ingest cutoff to orphan cleanup queries, centralizes ES response validation, and adjusts health tracking to last successful run. |
| tests/Exceptionless.Tests/Jobs/CleanupOrphanedDataJobTests.cs | Adds/updates integration tests validating cutoff boundary behavior, health window timing, and failure handling. |
Comments suppressed due to low confidence (2)
src/Exceptionless.Core/Jobs/CleanupOrphanedDataJob.cs:210
- This log line reports "orphaned events" but the value being logged (totalOrphanedEventCount) is incremented by the number of missing project IDs, not the number of deleted event documents. This makes the log message/structured field names misleading for operational analysis.
_logger.LogInformation("Found {OrphanedEventCount} orphaned events from missing projects out of {ProjectIdCount} since {OrphanedEventCutoffUtc}", totalOrphanedEventCount, totalProjectIds, orphanedEventCutoffUtc);
src/Exceptionless.Core/Jobs/CleanupOrphanedDataJob.cs:277
- This log line reports "orphaned events" but the value being logged (totalOrphanedEventCount) is incremented by the number of missing organization IDs, not the number of deleted event documents. This makes the log message/structured field names misleading for operational analysis.
_logger.LogInformation("Found {OrphanedEventCount} orphaned events from missing organizations out of {OrganizationIdCount} since {OrphanedEventCutoffUtc}", totalOrphanedEventCount, totalOrganizationIds, orphanedEventCutoffUtc);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4e43994c2
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9610e611cb
ℹ️ 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".
Co-authored-by: niemyjski <1020579+niemyjski@users.noreply.github.com>
Co-authored-by: niemyjski <1020579+niemyjski@users.noreply.github.com>
5d16ad9 to
171b235
Compare
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. |
Orphaned-event cleanup now uses one inclusive
CreatedUtccutoff from three days before each run for stack, project, and organization discovery and deletion. It still searches every versioned event index, including older occurrence dates ingested recently. This reduces historical aggregation and deletion work while preserving public cleanup entry points.Search timeouts, failed shards, and unrecoverable delete failures fail cleanup. Health distinguishes active progress from successful completion and follows the eight-hour schedule. Logs distinguish missing parent IDs from deleted documents. Four response-validation tests now run without the Elasticsearch integration fixture.
Validation: merged latest main (
deffcf185); Release build passed with zero warnings/errors; standalone response tests passed 4/4; local cleanup integration tests passed 23/23 and the adjacent 15,000-event cleanup test passed 1/1 with synthetic data; focused formatting and diff checks passed. Hosted API tests with coverage, Playwright E2E, client tests, version, application images, and the Elasticsearch image build all passed at21b6d2b4a. The Copilot contributor agreement remains pending.The rolling window intentionally leaves older orphans to retention after gaps longer than three days. Concurrent deletion conflicts remain best effort. Shard fan-out is unchanged, and production CPU improvement has not been measured. No API, WebSocket, configuration, serialization, or frontend contracts change.
Verification and implementation details
dotnet build tests/Exceptionless.Tests/Exceptionless.Tests.csproj --configuration Release --no-restore -m:1dotnet test --project tests/Exceptionless.Tests/Exceptionless.Tests.csproj --no-build --no-restore --configuration Release -- --filter-class Exceptionless.Tests.Jobs.CleanupOrphanedDataResponseTests