Repository navigation
feat: nested paths in {{input.a.b}} / {{config.a.b}} placeholders - #76
Conversation
Bbenchaya
left a comment
There was a problem hiding this comment.
Nice, this covers everything in #61 including both optional parts, and folding the input/config branches into _resolve_ref is a good cleanup. Verified locally against d671944: 7 of the 8 new tests fail on main, 510 pass with the patch, ruff check/ruff format --check/mypy src/ clean. Also applies cleanly on the current main tip (bf2f877, after #75 and #73). Probed a few extra cases (dotted top-level key precedence, tuple indexing, out-of-range index, path into a scalar, default('x.y') with a dot in the literal, config_data precedence) and they all behave as documented.
One small fix before I approve:
_lookup_path: segment.isdigit() → segment.isdecimal(). isdigit() accepts characters int() rejects, e.g. '²'.isdigit() is True but int('²') raises, so {{input.items.².sku}} raises ValueError out of the resolver, which otherwise never raises on bad input. isdecimal() is the exact predicate for what int() accepts (Arabic-Indic ١ still works, superscripts don't). Contrived input, but it's the only throwing path in the resolver and it's a one-word change.
Non-blocking:
- This commit is authored as
44erxyz <moonxyz@outlook.de>rather than the noreply address on your previous PRs. Just a heads-up in case that email isn't verified on your GitHub account, so the commit gets attributed to you. README.md,docs/guides/langgraph-integration.mdanddocs/architecture/overview.mdalso have placeholder tables with{{input.field}}. #61 only named the two files you updated, so this is fine as is; a row in those would be a nice touch if you're in there anyway.
Separately, while probing I hit a pre-existing bug that is not this PR's: PLACEHOLDER_PATTERN with fullmatch treats "{{input.a}}-{{input.b}}" as a single expression (the lazy .+? is defeated by fullmatch), so it comes back as the literal string. With any leading text it works. I'll file that as its own issue.
🤖 Review assisted by AI (Cursor)
|
Thanks for the thorough review. Pushed
511 tests pass, |
str.isdigit() accepts characters such as '²' that int() raises on, so
{{input.items.².sku}} raised ValueError out of the resolver. isdecimal()
is the predicate that matches what int() accepts.
Also adds the nested-path row to the remaining placeholder tables.
be66928 to
bf11c40
Compare
Checklist
🚨 Please review this repository's contribution guidelines.
Description
{{input.a.b.c}}and{{config.a.b.c}}now read nested values. Integer segments index lists ({{input.items.0.sku}}), so both optional parts of the issue are in.| default(...)if given, otherwise the<customer.id>marker plus the debug log.Noneresolves toNone, same as fix: resolve placeholders whose value is None #72._resolve_ref.Tests:
TestValueResolverNestedPaths(8 tests, 7 fail on main). Docs tables inmatchers-and-resolvers.mdandmock-format.mdupdated, plus a changelog entry under Added.Fixes #61