Skip to content

feat(scheduler): per-job max_iterations override from jobs.json - #33323

Open
inquistiff wants to merge 2 commits into
NousResearch:mainfrom
inquistiff:pr/0017-per-job-max-iterations
Open

feat(scheduler): per-job max_iterations override from jobs.json#33323
inquistiff wants to merge 2 commits into
NousResearch:mainfrom
inquistiff:pr/0017-per-job-max-iterations

Conversation

@inquistiff

Copy link
Copy Markdown

What does this PR do?

Hermes scheduler has a single global max_iterations cap applied to every cron job. In multi-tenant cron fleets, this is too coarse — some jobs (deep-research / multi-step debugging) need 40+ iterations; others (single-shot watchdogs / digest builders) should cap at 3 to fail-fast. A global cap forces a worst-case ceiling that wastes tokens on jobs that should never need that many turns.

This patch reads max_iterations from the per-job jobs.json entry if present, falling back to the global cap if not specified.

{
  "id": "deep-research-cron",
  "prompt": "...",
  "enabled_toolsets": ["agent", "browser"],
  "max_iterations": 40
}

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • cron/scheduler.py line 1156 (approx): per-job max_iterations key checked before global config fallback. One-line change, fully backward-compatible.

How to Test

  1. Add "max_iterations": 5 to a job entry in jobs.json.
  2. Fire the job and observe it caps at 5 iterations rather than the global default.
  3. Remove the field — job falls back to global cap (or 90 default).
  4. Unit test asserts: per-job override wins; missing field falls through to global; invalid type (string, negative) logs warning and uses global.

Checklist

  • My commit messages follow Conventional Commits
  • My PR contains only changes related to this fix/feature
  • I've run pytest tests/ -q and all tests pass

Add job.get(max_iterations) as first priority before config.yaml
fallback. Allows P5 Eval Judge to run at 120 iterations while
global default stays at 90.

Line 1156: was _cfg.get(agent).get(max_turns) or ... or 90
Now: job.get(max_iterations) or _cfg.get(agent).get(max_turns) or ...
@alt-glitch alt-glitch added type/feature New feature or request comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have labels May 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing with open PR #33305 (cron-specific max_iterations cap). Also duplicate of closed #2168 (identical feature). #33305 adds a global cron.max_iterations default; this PR adds per-job override from jobs.json. Could be complementary.

- Extract inline one-liner into validated block with logger.warning on
  invalid type (string, float, negative, zero) — matches spec from PR
  description; closes gap that got NousResearch#2168 closed
- Add tests/cron/test_scheduler_max_iterations.py: 13 tests covering
  fallback chain, type validation, boundary values, and scheduler import
- Fallback chain: job.max_iterations (positive int) > agent.max_turns >
  max_turns > 90 hard default

Production-validated: patch running in live cron fleet 6+ weeks.
@inquistiff

Copy link
Copy Markdown
Author

Thanks for the context on #2168 — appreciated.

To address the gaps flagged there:

Scope: This PR covers the scheduler read path (_build_run_context), which is the primary use case — jobs defined statically in jobs.json. The cronjob_tools.py create/update path is a separate surface that would need a follow-on PR; happy to file one if maintainers want full parity.

Fallback chain (composes with #33305):

job.max_iterations  →  cron.max_iterations (#33305)  →  agent.max_turns  →  90

This PR handles the per-job leaf; #33305 handles the global floor. The two are complementary — if both land, operators get a global cap + per-job escape hatch.

Validation (commit f187497cb): The original one-liner (job.get("max_iterations") or global or 90) had a latent bug — 0, floats, and strings all behave unexpectedly with the or-chain. The updated implementation uses an explicit isinstance(int) + positive-integer guard with logger.warning on invalid values, falling back to the global config gracefully.

Tests (commit f187497cb): 13 unit tests added in tests/cron/test_scheduler_max_iterations.py covering the full fallback chain, type validation (string, float, negative, zero all rejected), boundary values (1 and 500 valid), and import smoke.

Production: This patch has been running in a 50-job jobs.json fleet for 6+ weeks with no issues.

Happy to rebase on top of #33305 once that lands if you'd prefer the integration to be explicit in a single PR.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused scheduler change. The underlying gap is present on current main: cron/scheduler.py:2899 resolves only the global budget, then passes it to AIAgent at cron/scheduler.py:3054.

Problems

  • cron/scheduler.py:1541 accepts True because Python booleans are integers. A JSON true would silently impose a one-iteration cap despite the stated positive-integer validation contract.
  • tests/cron/test_scheduler_max_iterations.py:21 duplicates production logic in _resolve instead of exercising _run_job_impl/run_job; the tests can pass even if the scheduler does not use the field. Existing scheduler tests inspect constructor kwargs at tests/cron/test_scheduler.py:995.
  • The field is not exposed by the normal job-writing surfaces (cron/jobs.py:1033, tools/cronjob_tools.py:659, hermes_cli/subcommands/cron.py:27), so it is limited to manual jobs.json edits.

Suggested changes

  • Reject booleans with type(value) is int, and cover both boolean values.
  • Assert the real AIAgent(..., max_iterations=...) constructor argument through run_job, including fallback and warning cases.
  • If this is intended as a supported per-job configuration, integrate it through the job store and public cron interfaces, preferably using the existing max_turns vocabulary.

Automated hermes-sweeper review.

Comment thread cron/scheduler.py
# Max iterations: per-job override > agent.max_turns > max_turns > 90
_global_max_iter = _cfg.get("agent", {}).get("max_turns") or _cfg.get("max_turns") or 90
_job_max_iter = job.get("max_iterations")
if _job_max_iter is None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

bool is a subclass of int, so a hand-edited JSON true passes this condition and silently sets a one-iteration cap. Use type(_job_max_iter) is int and add boolean cases to the validation coverage.

if job_val is None:
return global_max, warnings
if isinstance(job_val, int) and job_val > 0:
return job_val, warnings

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This duplicates the resolver instead of exercising the scheduler, so it can pass while the production run_job path ignores the field. Mock AIAgent through run_job and assert its max_iterations constructor kwarg, as existing scheduler tests inspect constructor kwargs.

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

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants