Skip to content

fix: downgrade empty-fetch log from ERROR to INFO (#2873)#2874

Open
chubes4 wants to merge 1 commit into
mainfrom
fix-2873-fetch-no-content-severity
Open

fix: downgrade empty-fetch log from ERROR to INFO (#2873)#2874
chubes4 wants to merge 1 commit into
mainfrom
fix-2873-fetch-no-content-severity

Conversation

@chubes4

@chubes4 chubes4 commented Jul 12, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #2873. FetchStep::execute() logged 'Fetch handler returned no content' at ERROR level for every empty fetch result — a routine, expected outcome (filtered sources, quiet windows, small subreddits with no qualifying items today), not a handler failure. On wire.extrachill.com's ~27 daily reddit flows this produced ~16 ERROR rows/day of pure noise, diluting the error channel and triggering false-positive investigation via the agent wake briefing.

Severity rationale

  • The job already proceeds to the success-family terminal status completed_no_items.
  • RunMetrics::recordStepResult() already records result: no_content — the machine-readable outcome was never wrong, only the log severity.
  • The is_wp_error( $packets ) branch immediately above ('Fetch handler failed: ...') is the actual handler-failure path and correctly stays at error — untouched by this PR.
  • Downgraded to info (not debug) because it's a meaningful per-flow-run event worth keeping visible in normal log output, just not error-worthy. The existing downstream 'Step completed with no new items to process' INFO log already covers the human-readable summary; this is the step-level detail one layer below it.

Paired WARNING decision

The issue also flagged the paired 'Step execution did not produce a successful result' WARNING fired from ExecuteStepAbility::logStepExecutionResult(). I traced this to a single call site that already receives the classified $execution_result['status'] (via StepExecutionResult::classify()/fromStepOutput()). Gating on 'completed_no_items' === $status there is a clean, one-line, well-bounded change — it is not tangled into generic step-failure handling, since completed_no_items is already a distinct, explicitly-modeled status value (not inferred from packet count at that call site). Skipped it for that one case; every other non-success status (failed, blocked, etc.) still warns exactly as before.

Net effect for a routine empty-fetch day: one INFO log from FetchStep + one INFO log from the engine ("no new items to process"). Previously it was ERROR + WARNING + INFO for the same event.

Changes

  • inc/Core/Steps/Fetch/FetchStep.php — empty-packets branch logs at info instead of error; is_wp_error() failure branch unchanged.
  • inc/Abilities/Engine/ExecuteStepAbility.phplogStepExecutionResult() early-returns (skips the WARNING) when $execution_result['status'] === 'completed_no_items'.
  • tests/fetch-no-content-log-severity-smoke.php — new pure-PHP smoke test (follows the existing tests/*-smoke.php convention) asserting: FetchStep logs at info not error, the handler-failure error log is untouched, RunMetrics no_content recording is untouched, the paired WARNING is skipped for completed_no_items, and the WARNING still fires for genuine failed classifications.

Verification

  • homeboy review data-machine --changed-since origin/main — audit: 0 findings/passed, lint: 0 findings/passed. The test stage failed in this environment due to a WP Codebox PHPUnit recipe-builder module being unavailable (buildWordPressPhpunitRecipe missing) — a sandbox/harness infra gap, not a code failure; confirmed by running the repo's actual portable test pattern directly instead:
  • Ran the new smoke test plus all directly-related existing smoke tests (fetch-handler-failure-status-smoke.php, job-status-accounting-smoke.php, job-outcome-metrics-smoke.php, step-execution-result-contract-smoke.php, fetch-item-dispositions-smoke.php, run-metrics-smoke.php) via php tests/<name>.php. New test: 9/9 pass. Pre-existing failures in job-status-accounting-smoke.php, job-outcome-metrics-smoke.php, and fetch-item-dispositions-smoke.php (missing wp_json_encode() stub for a newer StepResult/RunResult codepath, and an unrelated normal completion still marks processed assertion) reproduce identically on origin/main before this change — confirmed via git stash. Not introduced by this PR.

Not released, not deployed — PR only per task constraints.

Empty fetch results are routine, expected behavior (filtered sources,
quiet windows, small subreddits with no qualifying items), not handler
failures. The job already terminates in the success-family
completed_no_items status and RunMetrics already records
result: no_content.

- FetchStep::execute() now logs the empty-packets branch at info
  instead of error. The is_wp_error() handler-failure branch above it
  is untouched and still logs at error, correctly.
- ExecuteStepAbility::logStepExecutionResult() now skips the paired
  "Step execution did not produce a successful result" WARNING when
  the classified status is completed_no_items. That WARNING already
  gets its own downstream INFO log ("Step completed with no new items
  to process"); firing a WARNING on top of that for a successful empty
  window was pure noise. This is a clean, bounded gate on one
  well-defined status value inside a single call site (not tangled
  into generic step-failure handling), so it's a safe accompanying fix
  rather than a separate, deferred change.
- Added tests/fetch-no-content-log-severity-smoke.php following the
  existing pure-PHP smoke test pattern, asserting both the log-level
  change and the WARNING-skip behavior (while confirming genuine
  failures still warn).

Fixes #2873
@homeboy-ci

homeboy-ci Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Homeboy Results — data-machine

Lint

review lint — passed

ℹ️ Full options: homeboy self docs commands/lint
Deep dive: homeboy review lint data-machine --changed-since 0b764b2

Artifacts and drill-down
  • CI results artifact: homeboy-ci-results-data-machine-review-lint-quality-Linux-node24 contains immediate command JSON for this action invocation.
  • Observation artifact: homeboy-observations-data-machine-review-lint-quality-Linux-node24 contains exported Homeboy run history for deeper queries.
  • Drill-down: download the observation artifact, then run homeboy runs import <dir>, homeboy runs list, and homeboy runs findings <run-id>.
  • Artifacts are attached to the workflow run: https://github.com/Extra-Chill/data-machine/actions/runs/29176191913

Test

review test — failed

ℹ️ No tests ran — the runner failed before producing results. See raw_output.stderr_tail / raw_output.stdout_tail for the underlying error (bootstrap failure, missing deps, DB connection, etc.).
ℹ️ To run specific tests: homeboy test data-machine -- --filter=TestName
ℹ️ Auto-fix lint issues: homeboy refactor data-machine --from lint --write
ℹ️ Collect coverage: homeboy test data-machine --coverage
ℹ️ Analyze failures: homeboy test data-machine --analyze
ℹ️ Pass args to test runner: homeboy test -- [args]
ℹ️ Full options: homeboy self docs commands/test
Deep dive: homeboy review test data-machine --changed-since 0b764b2

Artifacts and drill-down
  • CI results artifact: homeboy-ci-results-data-machine-review-test-quality-Linux-node24 contains immediate command JSON for this action invocation.
  • Observation artifact: homeboy-observations-data-machine-review-test-quality-Linux-node24 contains exported Homeboy run history for deeper queries.
  • Drill-down: download the observation artifact, then run homeboy runs import <dir>, homeboy runs list, and homeboy runs findings <run-id>.
  • Artifacts are attached to the workflow run: https://github.com/Extra-Chill/data-machine/actions/runs/29176191913

Audit

review audit — passed

  • audit — 37 finding(s)
  • Total: 37 finding(s)

Deep dive: homeboy review audit data-machine --changed-since 0b764b2

Artifacts and drill-down
  • CI results artifact: homeboy-ci-results-data-machine-review-audit-quality-Linux-node24 contains immediate command JSON for this action invocation.
  • Observation artifact: homeboy-observations-data-machine-review-audit-quality-Linux-node24 contains exported Homeboy run history for deeper queries.
  • Drill-down: download the observation artifact, then run homeboy runs import <dir>, homeboy runs list, and homeboy runs findings <run-id>.
  • Artifacts are attached to the workflow run: https://github.com/Extra-Chill/data-machine/actions/runs/29176191913
Tooling versions
  • Homeboy CLI: homeboy 0.281.20+123145ee082b+0f6169fb
  • Extension: wordpress from https://github.com/Extra-Chill/homeboy-extensions
  • Extension revision: 98a1bf10
  • Action: unknown@unknown

@chubes4

chubes4 commented Jul 12, 2026

Copy link
Copy Markdown
Member Author

homeboy / Test CI failure: proven unrelated infrastructure flake (not a regression from this PR's diff)

Root cause identified and filed upstream: Extra-Chill/homeboy-extensions#2216

What's failing

The WordPress test runner's NO_TEST_FILES fallback path (test-runner-wp-codebox.shrun_composer_test_script()) shells out to composer test, which for this repo is literally:

"scripts": { "test": "homeboy review test data-machine" }

That inner call is a plain, unflagged recursive invocation of homeboy review test — it does not inherit the --force-hot --allow-local-hot flags the outer CI wrapper (homeboy-action) adds. On a GitHub-hosted runner with load average just above the warm-machine threshold, that inner call trips Homeboy's resource-policy guard and exits 2. The WP-Codebox → composer-fallback sequence then retries 8-16 times over 6-9 minutes before the harness gives up, each retry hitting the same rejection.

Why this isn't a regression from this diff

Verification performed

  • homeboy review data-machine --changed-since origin/main: audit 0 findings/passed, lint 0 findings/passed.
  • Direct php tests/*.php smoke test execution (the repo's actual portable test pattern) for the new test and all fetch/step-execution-result related tests.

No code changes made in response to this CI failure — the diff itself is unaffected. Filed the upstream infra bug with full evidence in the issue above. Flagging for maintainer awareness; happy to rerun CI again if useful once #2216 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FetchStep logs 'Fetch handler returned no content' at ERROR level for routine no-new-items outcomes

1 participant