The Flag You Don't Have Can't Be Wrong

Students have been able to review a Course since the last slice. They rate it, they write something public, and — optionally — they leave a Private Feedback: a candid note the glossary describes as “Instructor-only.”

Which was, until this week, a lie. Nothing anywhere read it. The column was written on every submission and selected by nothing. CONTEXT.md promised a reader who did not exist.

This slice builds the reader: one screen, at /feedback, holding every Review across every Course, with the Private Feedback beside the public text and a triage state to work through them.

The obvious design is the broken one

An inbox needs to know what you’ve already seen. So: add read boolean default false to reviews, flip it when the Instructor opens a row, count the falses for the badge. Twenty minutes.

Now recall one thing about a Review: it is editable, and it overwrites in place. One per Enrollment, forever. A Student who rated a Course 3★ in frustration, re-watched the module that lost them, and came back to make it 5★ does not create a second Review — they rewrite the first one.

So what happens to your boolean?

The Instructor read the 3★ two weeks ago. read is true. The Student rewrites it into something completely different, and the row sits there — read, filed, invisible — saying something it no longer says. The only fix is for submit/2 to remember to set read = false on every write. A flag that a distant piece of code has to remember to reset is a flag that will eventually not be reset.

Two timestamps and a comparison

The row already carries the answer. Every Ecto schema with timestamps() knows when it was last written. So don’t store whether it was read — store when, and compare:

def unread?(%{read_at: nil}), do: true

def unread?(%{read_at: read_at, updated_at: updated_at}) do
  DateTime.compare(read_at, updated_at) == :lt
end

A Review is Unread when it has never been read, or when the Student has touched it since you last looked. That’s it. That’s the whole triage state.

stateDiagram-v2
    direction LR
    [*] --> Unread: Student submits
(read_at IS NULL)
    Unread --> Read: Instructor expands the row
(read_at := now)
    Read --> Unread: Student re-submits
(updated_at moves past read_at)
    Read --> Unread: "Mark as unread"
(read_at := NULL)

    note right of Read
        Nothing resets a flag here.
        The Student's edit moves updated_at,
        and the comparison does the rest.
    end note

Look at the third transition. Nobody wrote it. There is no code in submit/2 that knows the Feedback screen exists. The Student’s edit bumps updated_at, updated_at overtakes read_at, and the row resurfaces in the Instructor’s list with its new 5★ — because that is what the comparison means, not because anything remembered to make it happen.

That is the difference between deriving state and storing it. The stored version needs a correctness argument that spans two modules. The derived version needs no argument at all.

The trap this sets

The rule only holds while updated_at means “the Student last edited this.” The moment anything else writes the row, it stops being true.

And marking a Review read is writing the row. Through a changeset, it would look like this:

# Wrong: bumps updated_at as well as read_at.
review |> Ecto.Changeset.change(read_at: now) |> Repo.update()

Both timestamps land on the same instant. The Review is read — until the next second ticks, at which point it’s… still read, actually. Fine, by luck. But updated_at now means “whenever the Instructor last clicked,” the Student’s edit history is gone, and the rule the whole screen rests on is quietly measuring the wrong thing.

So reading a Review deliberately does not go through a changeset:

def mark_read(review_id) do
  set_read_at(review_id, DateTime.utc_now(:second))
end

defp set_read_at(review_id, value) do
  {count, _} =
    from(r in Review, where: r.id == ^review_id)
    |> Repo.update_all(set: [read_at: value])

  if count == 1, do: :ok, else: {:error, :not_found}
end

update_all writes exactly the column named and touches nothing else. Reading is not editing, and the schema should not pretend otherwise. read_at isn’t even castable — there is no path to it through the changeset at all.

“Mark as unread,” meanwhile, needs no special mechanism. Unread is the absence of a read, so un-reading is read_at = NULL. The undo is the same field going back to where it started.

The strip does not follow the filters

The summary strip reports average Rating, total, and unread. The list below it filters by Course, by Rating, by unread — combining with AND, requeried server-side.

It is extremely tempting to wire the strip to the same query. Same data, same page, one function; a developer “fixing the inconsistency” would do it in a minute and feel tidy.

Then filter to 1★ and watch your average rating become 1.0.

An average over a set you filtered by that average’s own dimension is not a statistic, it’s a tautology. So summary/0 takes no filters at all, on purpose, and says so where someone might helpfully change it:

@doc """
Deliberately takes **no filters**. It reduces over every Review, always: the
strip reports the whole picture while the list below narrows. Filtering to
1★ must not drag the displayed average down to 1.0 — that is the reading of
a broken product, not of a filtered one.
"""
def summary do

There’s a test that fails if anyone tries.

The same instinct governs the empty case: a Course with no Reviews shows no average at all, not 0.0. A zero reads as a terrible score. An absence should read as an absence. And a filter combination that matches nothing gets an empty state rather than a bare, broken-looking list — while the strip above it goes on reporting that there are, in fact, nine Reviews.

The row that must not vanish

One behaviour took a moment’s thought. Expanding a row marks it read — opening it is having seen it, and a read-only expand would cost a second click per item and let the badge drift from what the Instructor has actually looked at.

But now turn on Unread only and open a row. The row is read. It no longer matches the filter. A naive requery deletes it from under the cursor — mid-sentence, while you’re reading it.

So the read patches the row in place and the list is not requeried until the Instructor next changes a filter. The counts, though, are re-read from the database every time:

# The strip and the sidebar badge are re-read from the DB, never decremented by
# hand: a counter you maintain yourself is a counter that eventually lies.
defp load_counts(socket) do
  summary = Reviews.summary()
  ...
end

The row you’re looking at keeps its seat; the numbers stay honest.

Who wrote this, and what you don’t get to know

The Instructor sees a name and an avatar, resolved live from the Student’s Profile. A Profile that was never filled in — or was scrubbed on a GDPR erasure request — reads “Anonymous student.”

To the trainer. On the admin screen. Behind require_admin.

That is not an oversight, and it’s the part most likely to be “fixed” by a future reader: reviews → enrollments → users.email is one join away, and the person looking at this page is already an admin. Nothing technical stops it.

But ADR 0015 says a Review stores no name so that a scrub flows straight through to the public list. If the same scrubbed Profile is still identifiable the moment the trainer opens a screen, the erasure did not erase — it just moved. So the byline is all there is, and the email appears nowhere on the page. The Instructor can still reach any Student through the Course’s Students tab; they simply cannot do it from a Review, which is precisely what keeps the scrub meaningful.

There’s a test that greps every rendered field for the Student’s email and fails if it finds one.

A Review with nothing in it

A Student can rate 4★ and write nothing at all — no public text, no private note. The row still appears. The Rating is the feedback, and a screen that hides it because two text columns are NULL is hiding data the Student deliberately sent.

Both detail panes say “Not provided,” explicitly, rather than collapsing into empty boxes that read like a rendering bug.

The sidebar, six times over

The Studio sidebar was copy-pasted, inline, into all six admin screens. That was survivable right up until a Feedback entry with an unread badge needed to appear in every one of them — six copies of a nav is six chances for one to go stale.

It’s one component now, with the count assigned once in the :require_admin on_mount hook, so the badge follows the Instructor around the Studio rather than only existing on the screen it counts.

The bug the prototype could not show me

The design system prototype renders a clay star in the “Average rating” card. The app rendered a grey square.

The markup was identical. No CSS rule in the stylesheet matched the element — I checked, by walking every stylesheet in the browser and asking each rule whether it matched. None did. Yet the computed style came back with a background-color, an opacity: 0.2, and a 24px box that nothing in my CSS had asked for.

The class was .summary-icon.rating. daisyUI ships a .rating component — it masks its children into star glyphs, which is a delightful thing to discover a hostile version of. My star was being turned into a daisyUI star-shaped mask of a star, which is a grey square.

The prototype is a standalone HTML file. It loads the design tokens and nothing else. It cannot surface a collision with a framework it never loads — which is a good argument for the rule that a screen isn’t done until it has run in the actual app, and a good argument against trusting a class name as generic as rating in a codebase with a utility framework in it.

It’s .summary-icon-rating now, with a comment explaining what will happen to the next person who “simplifies” it back.

What’s here

• /feedback in the admin live_session — a Student is redirected, as with /courses.
• Every Review across every Course; filter by Course, Rating and unread (AND); sort by recency, lowest or highest Rating, with Rating ties broken by recency.
• Public text and Private Feedback side by side; “Not provided” where a Student left one out.
• Unread derived from read_at IS NULL OR read_at < updated_at — pinned by tests that run async: true against no database at all, because a rule made of two timestamps needs no Postgres to prove.

Not here, and each its own Card if it’s ever wanted: notifying the Instructor when a Review lands (nothing in this product pushes anything to a trainer yet, and that has its own delivery questions), a per-Course feedback tab, and a 5→1 star breakdown chart. Still out, as it has been since Reviews shipped: the Instructor cannot hide, approve, or reply to a Review.

There is also no pagination. Every Review renders. That’s fine at this volume and will not be fine forever — but the fix is a Card, not a guess.