Skip to content

fix(build): land the batching commit's unpushed consumers - #681

Closed
ScriptedAlchemy wants to merge 3 commits into
cursor/simplify-pr421-hot-pathsfrom
claude/base-build-fixes
Closed

fix(build): land the batching commit's unpushed consumers#681
ScriptedAlchemy wants to merge 3 commits into
cursor/simplify-pr421-hot-pathsfrom
claude/base-build-fixes

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

Merge this first — it supersedes #676

cursor/simplify-pr421-hot-paths does not compile. perf(observation): batch admission and preparation landed changes whose consumers and one whole file were never git added, so the work exists only in one local working tree. Every branch cut from the pushed head fails before its own code is built — that is currently every open PR and every agent working against this branch.

Four separate breakages, all the same root cause:

Missing Referenced by
crates/tracedecay-runtime-core/src/background_cpu.rs (whole file + mod line) observation.rs import
ObservationApplicationError::BatchWorkerStopped arm runtime/ingest/failure.rs:517
HostAdmissionOutcome.recovery field kiro.rs, cline_like.rs, ingest/tests.rs ×2
CodeIndexWorkerPlanInstallErrorV1, worker_reservation_bytes, installed_worker_status, with_background_cpu_permit src/daemon/code_index_scheduler.rs

The last one is a ~694-added/170-removed-line delta in crates/tracedecay-code-index/src/parallelism.rs — the in-flight worker-planning work. It is carried here as its original commit rather than reconstructed.

One judgement call, flagged for the author

BatchWorkerStopped needed a classification and none was written. It is classified retryable / Unavailable with reason code observation_batch_worker_stopped: a worker that went away before reaching a verdict decided nothing about the payload, so re-running the same input can succeed. Classifying it permanent would silently drop observations on a transient worker fault. If the author intended different semantics, this is the line to change.

Verification

cargo check -p tracedecay-sessions --all-targets clean. Cargo.lock is updated because the carried commit adds dependencies.

Two agents independently hit these and reported rather than repairing, which is how the scope got established rather than guessed.

🤖 Generated with Claude Code

ScriptedAlchemy and others added 3 commits August 24, 2026 02:08
Sizes the code-index worker pool from a per-worker resident budget and
host headroom instead of raw core count, and reports the resulting plan
(configured selection, effective workers, limiting reason) so callers
can see why a width was chosen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
perf(observation) landed an import of
tracedecay_runtime_core::background_cpu, but neither the module file nor
its declaration was added, so the pushed integration branch does not
compile: every branch cut from it fails on an unresolved import.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
perf(observation) added an ObservationApplicationError variant and a
HostAdmissionOutcome field without updating four consumers, and
code_index_scheduler references four parallelism items whose file was
never git added. The pushed branch therefore does not compile, so every
branch cut from it fails before its own work is even built.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 537a041

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 537a041e5a

ℹ️ 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".

Comment on lines +300 to +302
pub fn install_process_background_cpu(
width: NonZeroUsize,
) -> Result<Arc<ProcessBackgroundCpuV1>, BackgroundCpuInstallErrorV1> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wire the CPU authority into daemon startup

A repo-wide search of this commit finds no production caller of either install_worker_plan or install_process_background_cpu, so this process-wide singleton remains unset. HostAdmissionFacade::application requires process_background_cpu() and otherwise returns background_cpu_unavailable, meaning every otherwise-supported production observation capture is rejected; migrate daemon startup to the new installer rather than leaving the authority unmounted.

AGENTS.md reference: AGENTS.md:L82-L84

Useful? React with 👍 / 👎.

Comment on lines +574 to +575
let pool = rayon::ThreadPoolBuilder::new()
.num_threads(indexing_worker_target(detected_cores()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Honor worker limits in the standalone pool

When a standalone or one-shot caller has no installed runtime, this fallback ignores both TRACEDECAY_INDEX_WORKERS and the new memory-safe plan and instead uses half the detected CPUs. On a 96-core host, even an explicit one-worker override therefore creates 48 parser/artifact workers—about 6 GiB under this module's own 128 MiB-per-worker model—reintroducing the swap/OOM risk the previous max-eight ceiling avoided; malformed overrides also bypass the new typed refusal.

AGENTS.md reference: AGENTS.md:L102-L104

Useful? React with 👍 / 👎.

@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Closing in favour of #688, which is the same fix minus one commit that cannot land on this base.

#681 also carried crates/tracedecay-code-index/src/parallelism.rs. That commit changes parallelism::install to return a Result and removes install_daemon_worker_ceiling, which breaks tracedecay-semantic (embedding_parallelism.rs) and tracedecay-cli (main.rs:207) on this base — it needs its own unpushed follow-up to be coherent, and supplying available_memory_bytes to the new plan API is a design call belonging to its author, not something a build fix should invent.

#688 covers background_cpu.rs, the BatchWorkerStopped arm, all five HostAdmissionOutcome.recovery sites, and the readiness accessors, and passes cargo check --workspace --all-targets --locked clean.

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.

1 participant