thanos / ex_systolic / dbb2e875c777a92572b4d4bae614067eadcaf121-PR-38 / 1
98%
main: 98%

LAST BUILD BRANCH: v0.2.0/PARTITIONED-POOLEX_BACKEND
DEFAULT BRANCH: main
Ran
Files 18
Run time 0s
Badge
Embed ▾
README BADGES
x

If you need to use a raster PNG badge, change the '.svg' to '.png' in the link

Markdown

Textile

RDoc

HTML

Rst
coverage: 98.485% (+4.0%) from 94.468%
dbb2e875c777a92572b4d4bae614067eadcaf121-PR-38.1

Pull #38

github

thanos 
Code Review: ex_systolic
A detailed catalog of issues found, classified by severity. References use file:line format.

Status legend:FIXED |PARTIALLY FIXED | NOT FIXED | ℹ️ ALREADY OK

---
1. Bugs (incorrect behavior — must fix) — 11/11 FIXEDFIXED

1.1 Trace.at/2 and Trace.for_coord/2 apply Enum.reverse before filtering — wrong order semantics         FIXED
Location: lib/ex_systolic/trace.ex:99-101, 116-119
events
|> Enum.reverse()
|> Enum.filter(...)
Enum.reverse |> Enum.filter is operationally identical to Enum.filter |> Enum.reverse (filter is order-preserving). But the reverse here is logically wrong because it returns events in chronological order, while Trace.record/2 documents prepend order — so callers cannot easily compose at/2 with the chronological-order convention used elsewhere. The bigger concern: there is no guarantee for documented ordering, and the existing test trace_test.exs:106-108 only checks list length, not order.
Fix: Decide a single canonical order (chronological) and document it. Apply reverse after filter for performance and clarity:
def at(%__MODULE__{events: events}, tick) do
  events
  |> Enum.filter(&(&1.tick == tick))
  |> Enum.reverse()
end
Add tests that assert chronological ordering of the returned list.
> **Resolution:** Both `at/2` and `for_coord/2` now do `filter |> reverse` (chronological output). The module doc documents reverse-chronological storage and chronological query semantics. Test added in `trace_test.exs`.

---
1.2 Link.read/1 typespec contradicts the actual return shape         FIXED
Location: lib/ex_systolic/link.ex:103
@spec read(t()) :: {term(), t()} | {:empty, t()}
This implies the success case can return {:empty, t()} ambiguously when term() includes the atom :empty. Worse, the doctest at link.ex:75-76 is broken:
iex> ExSystolic.Link.read(link2)
{42, _}
_ is not a valid match expression in iex> doctest output — it is silently parsed as a term. This doctest never actually runs as written; it likel... (continued)
Pull Request #38: - closed #31 CORE CONCEPT

260 of 264 relevant lines covered (98.48%)

2295.87 hits per line

Source Files on job dbb2e875c777a92572b4d4bae614067eadcaf121-PR-38.1