Normalize tuple defaults on list fields instead of warning - #242
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
Since #205 (ced5afa, the list-field coercion), every config load logs:
on any config that doesn't mention
langfuse.redact_patterns— i.e. for most installs, on everynervecommand and session.Root cause
_DEFAULT_LANGFUSE_REDACT_PATTERNSis a module-level tuple, andLangfuseConfig.from_dicthands it through as the field default._str_listdeliberately passes non-lists untouched (socoerce_scalarscan wrap a${VAR}string as one element), but_scalar_to_listonly knewNone/str/scalars — a tuple fell through to the warn-and-skip branch. Harmless in effect (the consumerlist()s the field, all four patterns still compiled), but pure noise on every load.Fix
config.py: materialize the default aslist(...)infrom_dict, so the field arrives as its declared type. SameNone-vs-[]semantics as before (a bareredact_patterns:keeps the defaults, an explicit[]turns redaction off).coerce.py: teach_scalar_to_listto copy tuples element-wise. YAML never parses to a tuple, so one can only be a code-side default — the elements are already the intended entries, only the container is wrong.Tests
from_dict({})must yield an actuallistfor every declaredlist[X]field across all config dataclasses — the existing sweeps only fed values in and never checked what a builder produces when the section is absent, which is exactly how this slipped through. (DEFAULT_CLAUDE_MODELSis the only other tuple constant near a list field and is alreadylist()-wrapped at its use site; the sweep now pins that class of bug.)langfuseblock keeps the default redact set with zeronerve.coercewarnings.Both fail on
main, pass here. Full suite: 2940 passed.