Skip to content

fix(query): validate required arguments before positional type checks - #189

Open
TimeToBuildBob wants to merge 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/query-required-argument-validation
Open

TimeToBuildBob wants to merge 2 commits into
ActivityWatch:masterfrom
TimeToBuildBob:fix/query-required-argument-validation

Conversation

@TimeToBuildBob

@TimeToBuildBob TimeToBuildBob commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

AQL calls with missing required arguments currently reach positional type inspection before Python validates the call, raising IndexError and producing HTTP 500. Bind the supplied arguments to the already-inspected function signature before inspecting values. Missing arguments then follow the existing TypeError → QueryInterpretException path.

Fixes #188.

Validation:

  • Four missing-argument cases failed on master c7821899; all ten new regression/control cases pass after the one-line repair. Controls preserve valid calls, omitted/explicit defaults, extra-argument errors and wrong-type errors.
  • Query regression slice: 66 passed. Full repository make typecheck passed (37 source files); pinned Ruff check/format and hook checks passed. No make test-fast target exists here.
  • Actual offline Flask query-route replay with synthetic MemoryStorage and isolated XDG directories: filter_keyvals([], "title") and query_bucket() returned HTTP 500 before / 400 after. Valid/default controls stayed 200; extra-argument and wrong-type controls stayed 400. Replay used aw-server master 994ea988 and its supported Flask 2.2/Werkzeug 2.3 dependency range, with no listener started.

The initial hosted run failed before tests because Ubuntu 26.04 has no setup-python 3.8 binary. Carry the same Ubuntu 22.04 pin already validated in #187; the native Windows/macOS matrix and Python floor stay intact. All Linux/Windows/macOS Python 3.8 checks now pass at 7feb4ca: Linux/macOS 372 passed and 5 skipped; Windows 369 passed and 8 skipped. All typechecks, lint and CodeQL passed. Greptile 5/5 covers the unchanged product commit cb43196; the later commit changes only the runner pin.

No parser change, blanket IndexError catch, new syntax, released-package claim, or change to optional-argument type policy.

Git-Session-Id: 99c22c1d-2c81-44ce-b296-b51905eb0004
Git-Session-Id: 99c22c1d-2c81-44ce-b296-b51905eb0004
@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

The PR checks required arguments before inspecting positional values.

  • AQL functions check argument counts before checking argument types.

No actionable issues were found. Tests were inspected but not run during this review.

Reviews (1) · Last reviewed commit: "fix(query): validate required arguments ..." · Reviewed by Greptile

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

CI-green and mergeable — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

@TimeToBuildBob

TimeToBuildBob commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 AI code review

Adds sig.bind(*args, **kwargs) to q2_typecheck before positional type inspection so invalid argument counts raise TypeError first. Adds query tests for missing arguments, valid calls, defaults, extra arguments, and wrong types, and changes the Linux CI runner from ubuntu-latest to ubuntu-22.04.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings. The diff looks correct to me on this pass.

Files changed (3) — the diff as I read it
  • .github/workflows/build.yml — Pins the Linux matrix entry to ubuntu-22.04 and adds a comment about Python 3.8 availability.
  • aw_query/functions.py — Calls sig.bind(*args, **kwargs) before q2_typecheck inspects positional argument types.
  • tests/test_query_function_arity.py — Adds an evaluate helper and ten cases covering missing arguments, valid calls, omitted and explicit defaults, extra arguments, and wrong argument types.

Reviewed 7feb4ca7e2fe · panel: codex:gpt-6.1-sol · subscription engine · 13s · about this reviewer

Maintainer commands

@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.

Outage coverage: 1-pass, not merge evidence. Subscription review supplied findings while full consensus capacity was unavailable. The consensus merge gate remains unsatisfied.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing required AQL arguments raise IndexError instead of a query error

1 participant