fix(query2): don't drop or mis-split arguments after a nested call - #167
Merged
Merged
Conversation
QFunction/QList/QDict.check consumed one extra character after the closing
bracket (usually the following comma), and QFunction.parse then skipped to
the next comma anywhere in the rest of the argument string. A call like
filter_keyvals(query_bucket("a"), "app", ["Code"]) therefore lost "app",
and merge_events_by_keys(query_bucket("a"), ["app", "title"]) split the
list in half.
check() now returns the exact remainder, and argument parsing requires a
comma between arguments (a missing comma is now a parse error instead of
being silently skipped).
Fixes #164
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Member
Author
|
@greptileai review |
 |
This was referenced Sep 26, 2026
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
1 participant
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.
Fixes #164.
QFunction.check,QList.checkandQDict.checkreturned the remainder one character too late (string[i + 1:]), usually eating the comma after the closing bracket.QFunction.parsethen skipped to the next comma found anywhere in the rest of the argument string. So infilter_keyvals(query_bucket("a"), "app", ["Code"])the"app"argument was dropped, and inmerge_events_by_keys(query_bucket("a"), ["app", "title"])the list was split at its inner comma. The same happened with a list or dict first argument followed by two or more arguments.Changes:
check()returns the exact remainder.QFunction.parserequires a,between arguments. A missing comma is now aQueryParseExceptioninstead of silently continuing.Tests: a parser-level test (nested call, list, dict and deeper nesting, missing separator) and the exact repro from the issue run through
query()on every datastore. Both fail on master and pass here.Parity suite (ActivityWatch/activitywatch#1467)
Ran
scripts/tests/query_parityfrom thetest/query-paritybranch against its pinned aw-server (b19e5b0), aw-client (f80900e) and a release build of the pinned aw-server-rust (c3baa9c), swapping in aw-core:Every failure is a strict xfail flipping to XPASS, so no new divergences. All 35 cases labelled aw-core#164 flip. 6 more flip whose
nested_call_argsquery was labelled by another pattern first (3 × #162, 2 × #163, 1 × ms-resolution).