feat(threads): observable priority chain + verifier script by CGMossa · Pull Request #183 · A2-ai/dvs2

CGMossa · 2026-05-01T09:49:22Z

AI-written details

Summary

Closes the original investigation: verify that set_num_threads's priority chain resolves identically when called from dvs-rpkg as from dvs-cli.

dvs/src/utils.rs (+74/−28): get_threadpool now resolves the priority chain (override → environment → default) and emits a log::debug! naming the resolved count and the winning tier via a private ThreadSource enum (Override / Environment / Default):

thread pool: <N> threads (source: override|environment|default, work_items=<W>)

Each tier clamps to its own cap (ENVIRONMENT_MAX_THREADS / DEFAULT_MAX_THREADS) and to work_items, co-located inside the match arm so a future edit can't drop a clamp.

ui/main_threads.sh (new, +229): exercises the chain end-to-end on both interfaces.

CLI section — 4 scenarios: no-flag/no-env default; env-only; --threads N beats env; --threads 0 clears the override.
R section — 5 phases. A–D drive dvs_add → dvs_status → dvs_get under one config each (default / env / option-overrides-env / withr-scoped override). Phase E holds DVS_NUM_THREADS=3 constant and toggles set_dvs_threads() between four ops in one repo, so the log line visibly flips environment → override → override → environment.

R-side logs render via set_dvs_log_level("debug") (possible since #182 routed the cross-thread log queue to R's console).

justfile (+14/−3):

ui_names adds threads so just ui-run / ui-render / ui-publish cover the new script.
ui-publish-only becomes variadic (*names): no args publish all; args restrict to those names.

Local verification (from previous run)

bash ui/main_threads.sh produces 20 thread pool: log lines; every line's source tier matches its configured input. CLI and R sections drive the same dvs::utils::get_threadpool code path, so identical (N, source) pairs across the two interfaces confirm the priority chain resolves the same through both bindings.

Test plan

Verifying this PR (CLI section)

cargo test -p dvs --lib passes
just install-cli rebuilds the dvs binary with the new log::debug!
CLI section of ui/main_threads.sh produces the expected 4 source-tier outcomes

Env prep needed before the R section of the script runs

The R section needs dvs-rpkg rebuilt against this branch's dvs. Because dvs-rpkg/src/rust/Cargo.toml pins dvs = { branch = "main" }, the rpkg won't pick up these changes until this PR lands on main or you temporarily re-pin:

(temporary) point dvs-rpkg/src/rust/Cargo.toml's dvs dep at this branch, or use a local [patch] block
cargo update -p dvs --manifest-path dvs-rpkg/src/rust/Cargo.toml
just rpkg-install
R section of ui/main_threads.sh produces 16 log lines with the expected tier transitions
grep -c 'thread pool:' /tmp/ui-threads.log returns 20

After merge, the temporary pin can be reverted and the rpkg picked up via a normal lockfile bump.

Drafted by Claude (claude-opus-4-7). Reviewed by the author.

Keats · 2026-05-01T17:17:10Z

 const DEFAULT_MAX_THREADS: usize = 16;
 /// Maximum thread count threshold if `DVS_NUM_THREADS` is set
-const ENV_MAX_THREADS: usize = 32;
+const ENVIRONMENT_MAX_THREADS: usize = 32;


Do we still need that variable? What's the reason to have special max value if an env var is set vs via set_num_threads?

Keats · 2026-05-01T17:19:35Z

+const ENVIRONMENT_MAX_THREADS: usize = 32;

-/// Global thread count override. 0 = unset (use env var or default).
+/// Global thread count override. 0 = unset (use environment variable or default).


Why expand env var to environment variable? I think everyone understands env var

I.. have no defence 🤣

Adds a debug log line in dvs::utils::get_threadpool that names the resolved thread count *and* the source (override / env / default), and a ui/main_threads.sh script that exercises the chain end-to-end through both dvs-cli and dvs-rpkg. The script's CLI section runs four scenarios — no flag/no env, env only, flag-overrides-env, and --threads 0-clears-override — each rendering one \"thread pool: N threads (source: ..., work_items=W)\" line that proves which tier won. The R section runs four phases (default / env / option-overrides-env / withr-scoped-override) with dvs_add → dvs_status → dvs_get in each, after opting in via set_dvs_log_level(\"debug\"). Every operation's resolution prints the same line through R's console (now possible after dvs-rpkg landed the miniextendr log routing in #182). Also adds 'threads' to ui_names in the justfile so 'just ui-run' / 'just ui-render' / 'just ui-publish' include the new script. Local end-to-end run produced 16 thread-pool lines: 4 from CLI, 12 from R (4 phases × 3 ops), every line matching the expected source tier.

Relabel the existing precedence scenarios (CLI 3, Phase C, Phase D) to call out PRECEDENCE in the section headers, and add Phase E: a single phase that holds DVS_NUM_THREADS=3 constant and toggles set_dvs_threads() between four ops. The dvs::utils::get_threadpool log line then prints, in order: env=3 → override=7 → override=5 → env=3 making it directly observable that set_dvs_threads(N) takes precedence over DVS_NUM_THREADS, and that clearing the override (NULL) falls back to env. The CLI section already demonstrates the analogous --threads N > env case; labels are updated for symmetry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

dvs/src/utils.rs — fold the priority chain into a single if/else-if/else inside get_threadpool. The previous (count, source) tuple match and the intermediate ThreadCount enum both added indirection without a second call site to justify it; the inline form reads top-down as the priority order with caps and labels adjacent to the branch that selects them. Log message format and source labels (override/env/default) are unchanged. justfile — ui-publish-only now takes a variadic *names. No args = publish all ui_names (current behavior); args = publish only those names. Replaces the separate ui-publish-one recipe. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…ronment dvs/src/utils.rs — declare `let num_threads; let source;` up front and assign both in each branch of an `if`/`else if`/`else`. Drops the tuple returned from the expression. Also rename the env-tier source label from "env" to "environment" for parity with the other labels (override/default). Update the matching references in ui/main_threads.sh. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Replace the &'static str source label with a private ThreadSource enum {Override, Environment, Default} and a Display impl that emits the same lowercase labels (preserving the contract with ui/main_threads.sh). Move .min(work_limit) inside each match arm so all bounds for a branch are at the point of computation; the trailing .min(work_limit) chained after the match was easy to miss when reading. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Spell out the env-tier in full: ENV_MAX_THREADS → ENVIRONMENT_MAX_THREADS, env_threads → environment_threads, and the matching doc/comment text. The only remaining `env` is `std::env::var` (stdlib path, not renamable). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Keats reviewed May 1, 2026

View reviewed changes

Keats reviewed May 4, 2026

View reviewed changes

Comment thread ui/main_threads.sh

CGMossa force-pushed the feat/verify-thread-priority-chain branch from f67d894 to c371dfc Compare May 4, 2026 08:47

CGMossa force-pushed the feat/verify-thread-priority-chain branch from c371dfc to ac3331f Compare May 16, 2026 09:30

CGMossa and others added 7 commits May 17, 2026 11:10

style: cargo fmt

300badf

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

CGMossa force-pushed the feat/verify-thread-priority-chain branch from ac3331f to 300badf Compare May 17, 2026 09:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(threads): observable priority chain + verifier script#183

feat(threads): observable priority chain + verifier script#183
CGMossa wants to merge 7 commits into
mainfrom
feat/verify-thread-priority-chain

CGMossa commented May 1, 2026 •

edited

Loading

Uh oh!

Keats May 1, 2026

Uh oh!

Keats May 1, 2026

Uh oh!

CGMossa May 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

CGMossa commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI-written details

Summary

Local verification (from previous run)

Test plan

Verifying this PR (CLI section)

Env prep needed before the R section of the script runs

Uh oh!

Keats May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Keats May 1, 2026

Choose a reason for hiding this comment

Uh oh!

CGMossa May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

CGMossa commented May 1, 2026 •

edited

Loading