Skip to content

fix(config): fall back to defaults on malformed numeric config values#488

Open
lourincedaging0-commits wants to merge 1 commit into
vouchdev:testfrom
lourincedaging0-commits:fix/config-numeric-coercion-fallback
Open

fix(config): fall back to defaults on malformed numeric config values#488
lourincedaging0-commits wants to merge 1 commit into
vouchdev:testfrom
lourincedaging0-commits:fix/config-numeric-coercion-fallback

Conversation

@lourincedaging0-commits

Copy link
Copy Markdown

what changed: recall.load_config and capture.load_config now route their numeric config values (max_chars, min_observations, dedup_window_seconds) through a small _coerce(value, default, cast) helper instead of a bare int()/float() call, so a malformed value falls back to the default instead of raising.

why: both docstrings already promise "read the <namespace>: stanza; fall back to defaults," and compile.load_config already implements exactly that contract with a _coerce helper (see compile.py:64-70, added for the same reason — a config typo must degrade, not take down every caller). recall.py and capture.py only guarded the yaml read/parse, not the per-field coercion, so a config typo like max_chars: a lot raised an uncaught ValueError straight out of the SessionStart recall hook, and the equivalent typo under capture: crashed any hook-driven observe()/finalize() call. this mirrors the existing, tested pattern rather than introducing a new one.

nothing on disk changes shape — no migration impact, no effect on an existing .vouch/ directory. behavior only changes for the malformed-value case, which previously crashed.

validation commands run:

  • .venv/bin/python -m pytest tests/ -q --ignore=tests/embeddings (full suite, all green)
  • .venv/bin/python -m mypy src (clean)
  • .venv/bin/python -m ruff check src tests (clean)
  • confirmed both new regression tests fail with ValueError on the pre-fix code (stashed the src changes, reran) and pass with the fix restored

recall.load_config and capture.load_config both document "read the
namespace; fall back to defaults" but only guarded the read of
config.yaml itself, not the int()/float() coercion of individual
values. a typo like `max_chars: a lot` raised an uncaught ValueError
straight out of the SessionStart recall hook; the same shape of typo
in capture: crashed observe()/finalize().

compile.load_config already solved this with a small _coerce(value,
default, cast) helper (see the comment at compile.py:64-70) and has a
test proving it. mirror that pattern in recall.py and capture.py so
all three load_config functions honor the same fallback contract, and
add the matching regression tests.

validated: pytest tests/ -q --ignore=tests/embeddings, mypy src,
ruff check src tests all pass.
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 54859caa-8682-4069-9e74-e9ceb181d7bd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance tests tests and fixtures size: S 50-199 changed non-doc lines labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs documentation, specs, examples, and repo guidance size: S 50-199 changed non-doc lines tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant