aclai-lab / aclai-lab/SoleLogics.jl

check(::Atom, ::AbstractDict) returns nothing for an absent atom, but its docstring documents false

Open
#111 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Julia
Stars
20
Forks
7
PR merge metrics
No merged PRs in 30d

Description

## Summary

`check(::Atom, ::AbstractDict)` returns `nothing` for an atom that is absent from the dictionary,
but its own docstring documents that case as returning `false`, and the public wrapper declares a
`Union{Bool, Vector{Bool}}` return. The documented example therefore cannot run.

## Evidence

The docstring in `src/utils/propositional-logic.jl` gives exactly this example:

```julia-repl
julia> check(Atom(3), Dict([1 => ⊤, 2 => ⊥]))
false
```

The implementation a few lines below returns `nothing` in that branch:

```julia
check(::CheckAlgorithm, a::Atom, i::AbstractDict) = haskey(a,i) ? Base.getindex(i, value(a)) : nothing
```

And the public entry point in `src/types/interpretation.jl` annotates the return:

```julia
function check(φ::Formula, args...; kwargs...)::Union{Bool, Vector{Bool}}
```

so the `nothing` cannot be converted. Running the documented example raises:

```
MethodError: Cannot convert an object of type Nothing to Union{Bool, Vector{Bool}}
```

with the stack trace pointing at `interpretation.jl:160`.

## Why it matters

An unassigned atom is the ordinary case of a partial assignment, and the documentation promises it
means `false`. Today the most basic query in the docstring throws instead of returning a usable
answer.

There is also a second, quieter risk in the same shape: where the value *does* survive to a caller,
`nothing` means *undetermined*, and a consumer must never read that as *false*. Whichever way this is
resolved, the two should not be silently interchangeable.

## What needs deciding first

This is an API semantics choice and belongs to the maintainers, not to a patch:

- **Closed-world**: an absent atom is `false`, matching the current docstring. Simple, and the
documented examples become true.
- **Three-valued**: an absent atom is genuinely undetermined, in which case the return type and the
docstring should say so explicitly and callers should be forced to handle it.

Either is defensible. The current state — documented as one, implemented as the other, and typed as
neither — is the only outcome that cannot be right.

## Done looks like

The chosen semantics implemented consistently for direct and compound checks, the docstring and the
declared return type agreeing with it, and a regression test covering the absent-atom case.

Happy to prepare the patch once the semantics are chosen.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.