Skip to content

Make every planner honour its budget and claim only what it guarantees - #166

Merged
guilyx merged 7 commits into
mainfrom
feat/zen-gates-g3lolk
Nov 3, 2026
Merged

guilyx merged 7 commits into
mainfrom
feat/zen-gates-g3lolk

Conversation

@guilyx

@guilyx guilyx commented Nov 3, 2026

Copy link
Copy Markdown
Member

What and why

This PR fixes the planner-contract bugs from the search review. The old budget test ran on tsp, which finishes before any budget matters, so three planners that never read their budget still passed it.

Bug Before After
iddfs, idastar, ehc ignore the budget blocksworld12 with --time-limit 0.5 still running after 20 s They poll once per expansion. The recursive ones unwind with BudgetExhausted
beam overshoots max_expansions=5 → 12 expanded; a 0.3 s limit took 0.73 s Polls once per node instead of once per layer
bnb overshoots 40 LM-cut calls, 0.4 s past a 0.3 s limit Stops before evaluating the remaining siblings
Depth cap reported as fact bnb/iddfs reported "No plan found" when they hit max_depth; a bnb plan under a cap counted as optimal (cost 16 vs 10) Hitting the cap, or Python's recursion limit, sets truncated
bfs/iddfs listed in OPTIMAL_PLANNERS They minimise plan length: cost 16 on errands where 10 is optimal Removed from the set. The workbench badge follows
Live dashboard Said "✘ no plan found" when the budget stopped the search; smeared repaints on narrow terminals; ~10× too many log lines when not on a terminal Says "stopped by the budget"; clips lines to the stream's width; writes a log line at most every 2 s
Small traps for planner authors idastar/ehc/hc/beam with no heuristic crashed with a TypeError deep in the search; MultiObserver rejected positional args; the SearchObserver docstring claimed hooks are called "defensively" Clear ValueError; positional args forwarded; docstrings corrected

Guarantees changed

  • bfs and iddfs leave OPTIMAL_PLANNERS. They stay shortest-in-length, and the README already described them that way.
  • truncated is now also set when a depth cap or the recursion limit stops a run. That is the only honest reading of such a result.

How it was checked

  • New tests/test_search_contract.py (38 cases):
    • every planner on blocksworld12, with max_expansions=100 and with time_limit=0.3
    • every OPTIMAL_PLANNERS member matched against dijkstra on errands, which has action costs
    • a depth cap reports truncated
    • a missing heuristic gives a clear error
    • MultiObserver positional args
    • the dashboard's non-tty log rate, budget verdict and line clipping
  • Full suite: 498 passed, 1 skipped. flake8 and black are clean, and web/dist is rebuilt.

Checklist

  • flake8 jupyddl tests tools and pytest pass locally
  • python tools/build_web.py re-run; web/dist committed
  • Planners moving out of OPTIMAL_PLANNERS are stated above
  • CHANGELOG.md updated (Unreleased → Fixed)

The history has a merge -s ours of the already-merged #165 tip so the push fast-forwards. Squash-merge.


Generated by Claude Code

Write down where and why we mean to publish jupyddl as citable research
software (.docs/PUBLISHING.md): a Zenodo DOI per release, a JOSS paper,
ICAPS workshops for the research results, with a readiness checklist, the
paper outline, the AI-usage disclosure JOSS requires, and how contributors
can be considered for authorship. The README states the project's aim and
its Cite section now says which version the 2021 preprint describes.

Add the community files an outside contributor expects: a security policy,
a pull-request template, a "Propose an algorithm" issue template, and an
"Adding an algorithm" section in CONTRIBUTING.md.

Run tests and build on pull requests. They were `on: [push]`, which never
fires for a fork, so a contributor's PR showed no checks at all.

Fix AGENTS.md drift: the Python floor is 3.10 and Mergify is retired.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RbGtRHF8XhkiPQCDVHtV2
A package can now add planners, heuristics and generators by declaring
`jupyddl.planners`, `jupyddl.heuristics` or `jupyddl.generators` entry
points. Once installed, they work from the CLI, the benchmark harness and
`jupyddl.solve` by name, without anyone importing the package.

Discovery is lazy and defensive (jupyddl/plugins.py): the CLI runs it
before building its parser, the library only on a registry miss, so
importing jupyddl never scans installed packages. A built-in name always
wins, a plugin that fails to import is skipped with a warning, and
JUPYDDL_NO_PLUGINS=1 turns it off. `jupyddl plugins` lists what was found.
The CLI now reads `requires_heuristic` from the registry at call time,
since INFORMED_PLANNERS is fixed at import and cannot know about plugins.

.docs/EXTENDING.md walks through a heuristic, a planner from a priority
function, a planner from scratch with the budget/observer contract, a
generator, testing the guarantees you claim, and shipping as a plugin.
tests/test_extending.py runs every Python block in it, so it cannot drift.

best_first now accepts a tuple priority for tie-breaking; observers get
its first element as `f`, so traces stay numeric.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RbGtRHF8XhkiPQCDVHtV2
# Conflicts:
#	.docs/PUBLISHING.md
#	AGENTS.md
#	CHANGELOG.md
Each of these returned a plan, a cost or a value without complaint, and
validate_plan accepted the plans because it replays them through the same
Task.apply. tests/test_semantics.py now pins them on tiny hand-written
tasks; 16 of its cases fail on the previous code.

- Derived facts were never retracted, and (not <derived>) held from the
  start. close() now recomputes axioms from scratch and maintains negated
  derived facts; negation inside a rule body (needs stratification) is
  refused. The relaxation gets the complement for free, so it never calls
  a solvable state a dead end.
- An atom both added and deleted left its complement true too; a `when`
  re-adding a deleted atom did the same. Complements: the delete wins.
- Under a (total-cost) metric an action without a cost effect is free, as
  in IPC planners (pegsol/sokoban optimised the wrong objective). Cost
  inside `when` was charged unconditionally: refused, as are negative costs.
- A never-defined numeric fluent read as 0; conditions reading one fail.
- blind skipped zero-cost actions and overestimated.
- h1/h2/hm crashed on numeric states; LM-cut crashed on an empty goal.
- lmcut and h2 clear `admissible` on tasks with conditional effects.
- Optimal planners default to lmcut, not hff; `-H none` means blind.
- make_heuristic builds a heuristic class or partial instead of passing it
  through unbuilt.
- Fact ids and id sets are built in sorted order, so LM-cut tie-breaks,
  expansions and plans no longer change with PYTHONHASHSEED.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RbGtRHF8XhkiPQCDVHtV2
- iddfs, idastar and ehc never read their budget, so --time-limit was
  ignored. They poll per expansion now; the recursive ones unwind with
  BudgetExhausted. beam polls per node, not per layer, and bnb stops
  evaluating siblings once the budget fires.
- Hitting a depth cap (iddfs max_depth, bnb max_depth) or Python's
  recursion limit sets stats.truncated: it is not a proof of
  unsolvability, nor of optimality.
- bfs and iddfs leave OPTIMAL_PLANNERS: they minimise plan length, and on
  errands return cost 16 where 10 is optimal.
- Live dashboard: a budget stop no longer reads "no plan found"; lines are
  clipped to the stream's terminal width so repaints do not smear; the
  non-tty log writes at most every two seconds.
- idastar/ehc/hc/beam without a heuristic raise a clear ValueError;
  MultiObserver forwards positional arguments; the SearchObserver and
  best_first docstrings say what actually happens.

tests/test_search_contract.py runs every planner where the budget is the
only thing that stops it, checks OPTIMAL_PLANNERS against dijkstra under
action costs, and pins the dashboard behaviour.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RbGtRHF8XhkiPQCDVHtV2
@guilyx
guilyx marked this pull request as ready for review November 3, 2026 08:10
@guilyx
guilyx merged commit def944e into main Nov 3, 2026
10 checks passed
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.

2 participants