Skip to content

Commit a8cc2ea

Browse files
author
Douglas Jones
committed
fix: is_bottom exempt from BottomPropagationError check; add tests; fix quickref
- interpreter.py: is_bottom is the one primitive that must inspect bottom values rather than propagate them. Exempt it from the propagation check. - test_runtime.py: add IsBottomTests (7 tests) covering direct-call pattern, BottomWithReason, bind pattern, and conditional expression usage - test_server.py: add adversarial tests — POST to unknown path, HEAD on malformed identity, GET /imports with corrupt stored object - AGENT_QUICKREF.md: update is_bottom docs — both direct-call and bind patterns now work; remove outdated WRONG/RIGHT framing 461 tests passing.
1 parent fa74972 commit a8cc2ea

4 files changed

Lines changed: 206 additions & 25 deletions

File tree

codifide/runtime/interpreter.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,9 +588,12 @@ def _call_primitive(self, name: str, arg_exprs: tuple, frame: _Frame) -> Any:
588588
# an argument. A program that wants to compute over a possibly-refused
589589
# value must handle it explicitly in a `believe` arm. Without this
590590
# check, arithmetic on ⊥ surfaces as a host TypeError.
591-
for a in args:
592-
if isinstance(a, _BottomType):
593-
raise BottomPropagationError(fn=name)
591+
# Exception: is_bottom is the one primitive whose purpose is to
592+
# inspect a bottom value — it must not propagate.
593+
if name != "is_bottom":
594+
for a in args:
595+
if isinstance(a, _BottomType):
596+
raise BottomPropagationError(fn=name)
594597
try:
595598
result = spec.fn(*args)
596599
except CodifideError:

docs/AGENT_QUICKREF.md

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -324,42 +324,34 @@ cand
324324
Both patterns are valid. `cand` + `when` is preferred for three or more
325325
branches; `if/then/else` is preferred for binary choices inside a body.
326326

327-
**`is_bottom` — value inspector, not propagation catcher.**
327+
**`is_bottom` — value inspector.**
328328

329-
`is_bottom(x)` returns `true` when `x` is a literal `bottom` value.
330-
It **cannot** catch a `bottom` that propagated through a bind:
329+
`is_bottom(x)` returns `true` when `x` is a `bottom` value (including `bottom "reason"`).
330+
Both the direct-call and bind patterns work:
331331

332332
```codifide
333-
# WRONGbottom propagates through the bind before is_bottom sees it
333+
# Direct-callis_bottom sees the value before any bind
334334
cand
335-
r <- function_that_refuses()
336-
if is_bottom(r) then "caught" else r # raises BottomPropagationError
335+
if is_bottom(function_that_refuses()) then "refused" else "ok"
337336
338-
# RIGHT — use a believe arm to handle propagated bottom
337+
# Bind pattern — also works; is_bottom receives the bottom value
339338
cand
340339
r <- function_that_refuses()
341-
believe r
342-
ge(conf(r), 0.70) => r
343-
else => bottom # or handle the refusal here
340+
if is_bottom(r) then "refused" else r
344341
```
345342

346-
`is_bottom` is useful when `bottom` is passed as an explicit argument
347-
or stored in a data structure, not when it arrives via function return.
348-
349-
**Direct-call `is_bottom` works.** If you need to check whether a function
350-
refuses *before* binding its result, call `is_bottom` directly on the call
351-
expression — no bind needed:
343+
For conditional routing on a possibly-refused value, `believe` is usually
344+
cleaner and more idiomatic:
352345

353346
```codifide
354-
# Worksis_bottom sees the value before any bind propagates it
347+
# Idiomaticbelieve handles refusal explicitly
355348
cand
356-
if is_bottom(moderate(message)) then "refused" else moderate(message)
349+
r <- function_that_refuses()
350+
believe r
351+
ge(conf(r), 0.70) => r
352+
else => bottom
357353
```
358354

359-
This short-circuits: if `moderate` refuses, the `else` branch never runs.
360-
Note that `moderate` is called twice — once for the check and once for the
361-
value. For expensive functions, prefer the `believe` pattern instead.
362-
363355
## Content-addressed imports
364356

365357
Individual symbol imports bring one symbol into scope by name:

tests/test_runtime.py

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,5 +318,156 @@ def inner
318318
self.assertIn("called from", msg)
319319

320320

321+
# ---------------------------------------------------------------------------
322+
# is_bottom — direct-call pattern and propagation footgun regression
323+
# (Sable audit note: AUD-OVERNIGHT-02 follow-up)
324+
# ---------------------------------------------------------------------------
325+
326+
class IsBottomTests(unittest.TestCase):
327+
"""Tests for the is_bottom(f()) direct-call pattern.
328+
329+
The quickref documents two idioms:
330+
- WRONG: r <- f(); is_bottom(r) — bottom propagates through bind
331+
- RIGHT: is_bottom(f()) — is_bottom sees the value before bind
332+
333+
These tests confirm both behaviours and cover BottomWithReason.
334+
"""
335+
336+
def test_is_bottom_on_literal_bottom_returns_true(self) -> None:
337+
src = """
338+
def main
339+
intent "is_bottom on literal bottom"
340+
sig () -> Bool
341+
effects {}
342+
cand
343+
is_bottom(bottom)
344+
"""
345+
self.assertTrue(run(parse(src), "main"))
346+
347+
def test_is_bottom_on_normal_value_returns_false(self) -> None:
348+
src = """
349+
def main
350+
intent "is_bottom on a normal value"
351+
sig () -> Bool
352+
effects {}
353+
cand
354+
is_bottom(42)
355+
"""
356+
self.assertFalse(run(parse(src), "main"))
357+
358+
def test_is_bottom_direct_call_on_refusing_function(self) -> None:
359+
# RIGHT pattern: is_bottom(f()) — sees the bottom before any bind.
360+
src = """
361+
def refuses
362+
intent "always refuses"
363+
sig () -> Any
364+
effects {}
365+
cand
366+
bottom
367+
368+
def main
369+
intent "direct-call is_bottom on a refusing function"
370+
sig () -> Bool
371+
effects {}
372+
cand
373+
is_bottom(refuses())
374+
"""
375+
self.assertTrue(run(parse(src), "main"))
376+
377+
def test_is_bottom_direct_call_on_non_refusing_function(self) -> None:
378+
# RIGHT pattern: is_bottom(f()) where f returns a value.
379+
src = """
380+
def gives_value
381+
intent "returns a string"
382+
sig () -> String
383+
effects {}
384+
cand
385+
"hello"
386+
387+
def main
388+
intent "direct-call is_bottom on a non-refusing function"
389+
sig () -> Bool
390+
effects {}
391+
cand
392+
is_bottom(gives_value())
393+
"""
394+
self.assertFalse(run(parse(src), "main"))
395+
396+
def test_is_bottom_on_bottom_with_reason(self) -> None:
397+
# BottomWithReason is a subclass of _BottomType — is_bottom must catch it.
398+
src = """
399+
def refuses_with_reason
400+
intent "refuses with a reason string"
401+
sig () -> Any
402+
effects {}
403+
cand
404+
bottom "not enough confidence"
405+
406+
def main
407+
intent "is_bottom on bottom-with-reason via direct call"
408+
sig () -> Bool
409+
effects {}
410+
cand
411+
is_bottom(refuses_with_reason())
412+
"""
413+
self.assertTrue(run(parse(src), "main"))
414+
415+
def test_is_bottom_bind_propagation_footgun(self) -> None:
416+
# Previously: r <- f(); is_bottom(r) raised BottomPropagationError
417+
# because is_bottom was treated like any other primitive.
418+
# After the interpreter fix (is_bottom exempt from propagation check),
419+
# this pattern now works correctly — is_bottom returns True.
420+
# The quickref has been updated to reflect this.
421+
src = """
422+
def refuses
423+
intent "always refuses"
424+
sig () -> Any
425+
effects {}
426+
cand
427+
bottom
428+
429+
def main
430+
intent "bind-then-is_bottom now works"
431+
sig () -> Bool
432+
effects {}
433+
cand
434+
r <- refuses()
435+
is_bottom(r)
436+
"""
437+
# Both the direct-call and bind patterns now return True.
438+
self.assertTrue(run(parse(src), "main"))
439+
440+
def test_is_bottom_in_conditional_expression(self) -> None:
441+
# is_bottom used in an inline conditional — common real-world pattern.
442+
src = """
443+
def maybe_refuses
444+
intent "refuses when input is zero"
445+
sig (n: Int) -> Any
446+
effects {}
447+
cand
448+
when gt(n, 0)
449+
n
450+
cand
451+
bottom
452+
453+
def main_refuses
454+
intent "test refusing path"
455+
sig () -> String
456+
effects {}
457+
cand
458+
if is_bottom(maybe_refuses(0)) then "refused" else "ok"
459+
460+
def main_ok
461+
intent "test non-refusing path"
462+
sig () -> String
463+
effects {}
464+
cand
465+
if is_bottom(maybe_refuses(5)) then "refused" else "ok"
466+
"""
467+
m = parse(src)
468+
self.assertEqual(run(m, "main_refuses"), "refused")
469+
self.assertEqual(run(m, "main_ok"), "ok")
470+
471+
321472
if __name__ == "__main__":
322473
unittest.main()

tests/test_server.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,41 @@ def test_symbols_root_get_404(self) -> None:
454454
status, resp = self._get_json("/symbols")
455455
self.assertEqual(status, 404)
456456

457+
def test_post_unknown_path_404(self) -> None:
458+
# POST to an unknown path must return 404, not 500.
459+
status, resp = self._post("/unknown/path", b"{}", "application/json")
460+
self.assertEqual(status, 404)
461+
self.assertEqual(resp["error"], "not_found")
462+
463+
def test_head_malformed_identity_404(self) -> None:
464+
# HEAD /symbols/not-a-hash — falls through to bare 404.
465+
# Regression: must not 500 or hang.
466+
status = self._head("/symbols/not-a-hash")
467+
self.assertEqual(status, 404)
468+
469+
def test_head_valid_identity_absent_404(self) -> None:
470+
fake = "sha256:" + "e" * 64
471+
status = self._head(f"/symbols/{fake}")
472+
self.assertEqual(status, 404)
473+
474+
475+
# ---------------------------------------------------------------------------
476+
# Adversarial: corrupt store object in /imports endpoint
477+
# ---------------------------------------------------------------------------
478+
479+
class CorruptStoreTests(_ServerFixture):
480+
def test_imports_corrupt_stored_object_returns_500(self) -> None:
481+
"""GET /symbols/<id>/imports where the stored object is not a valid module."""
482+
import hashlib
483+
# Write raw bytes that are valid CBOR but not a valid canonical module.
484+
garbage = b"\xa1\x63foo\x63bar" # {"foo": "bar"} — valid CBOR, invalid module
485+
identity = f"sha256:{hashlib.sha256(garbage).hexdigest()}"
486+
self.store._write_atomic(identity, garbage, suffix=".cbor")
487+
488+
status, resp = self._get_json(f"/symbols/{identity}/imports")
489+
self.assertEqual(status, 500)
490+
self.assertEqual(resp["error"], "store_error")
491+
457492

458493
if __name__ == "__main__":
459494
unittest.main()

0 commit comments

Comments
 (0)