Skip to content

fix!: reject keyword arguments in @model signatures instead of warning - #326

Open
bvdmitri wants to merge 1 commit into
mainfrom
fix/307-model-kwargs-error
Open

bvdmitri wants to merge 1 commit into
mainfrom
fix/307-model-kwargs-error

Conversation

@bvdmitri

Copy link
Copy Markdown
Member

Closes #307. This is a breaking change — see below.

Problem

src/model_macro.jl:959-961 parsed keyword arguments in a @model signature and then ignored them,
emitting only a warning:

if !isnothing(ms_kwargs) && length(ms_kwargs) > 0
    @warn("Model specification language does not support keyword arguments. Ignoring $(length(ms_kwargs)) keyword arguments.")
end

A user writing @model function m(x; a = 1) gets no a in the body, so the only real signal is a
later UndefVarError — and the warning is easy to lose in REPL output. Unsupported positional
arguments already hard-error at the call site (model_macro.jl:923), so this was inconsistent.

Change

Error at macro expansion, naming the offending keyword arguments and the supported form:

The `m` model macro does not support keyword arguments in the model signature, but got 2: a, b.
Declare all model interfaces as positional arguments, `m(x)`, they are passed by name at the call site.

Breaking-change assessment

Nothing in this repository relied on the old behaviour:

  • the warning string appears in exactly one place and no test or doc asserts it (there is no
    @test_logs / @test_warn anywhere in the suite)
  • ms_kwargs is referenced only at model_macro.jl:952 and :959
  • zero @model function signatures containing ; exist across src/, test/, docs/ and
    README.md — all 39 @model function occurrences in the docs and README are positional-only

The risk is downstream (RxInfer.jl and user code). Any such definition will now fail to expand — but
its keyword arguments were never in effect in the first place, so the fix surfaces a pre-existing
silent bug rather than removing working functionality. Worth a breaking version bump; there is no
CHANGELOG in this repo to update.

Tests

New test item in test/model_macro_tests.jl asserting the error fires for a kwarg-bearing signature,
that it names the offending keywords and the supported form, and that positional-only signatures still
expand.

Full suite on Julia 1.12: 259/259 test items, all passing.

🤖 Generated with Claude Code

Keyword arguments in a `@model` function signature were parsed and then silently
dropped, with only a `@warn`. The body never saw them, so a user writing
`@model function m(x; a = 1)` got no `a` and a later `UndefVarError` as the only
real signal, and the warning is easy to miss in a noisy REPL.

Raise an error at macro expansion instead, consistent with unsupported
positional arguments at the call site, and name the offending keyword arguments
plus the supported form.

BREAKING: `@model` definitions carrying keyword arguments now fail to expand
rather than silently ignoring them. Nothing in this repository relied on the old
behaviour: no test or doc asserted the warning, and no `@model` signature with
keyword arguments exists in `src/`, `test/`, `docs/` or `README.md`. Downstream
packages that defined such signatures will need to drop them, but their keyword
arguments were never in effect to begin with.

Closes #307.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.76%. Comparing base (4cc7c6c) to head (9a24e99).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
+ Coverage   90.72%   90.76%   +0.04%     
==========================================
  Files          16       16              
  Lines        2263     2264       +1     
==========================================
+ Hits         2053     2055       +2     
+ Misses        210      209       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Consider promoting @model keyword-argument drop from @warn to @error

1 participant