Skip to content

Add drawn reference notes and a debt lens - #620

Open
jserv wants to merge 5 commits into
mainfrom
visual-code-review
Open

jserv wants to merge 5 commits into
mainfrom
visual-code-review

Conversation

@jserv

@jserv jserv commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Interactive review returns a reference note after the contributor submits, and it was plain text: a call path or a design tradeoff had to be read out of prose, and nothing asked who would know the reasons later. The note can now carry a flow diagram, a comparison table and quoted code, and the review gains a step that looks at the change as three kinds of debt.

Every citation in a drawn note must be a line of the pinned diff, and quoted code is taken from the diff, never typed. A solid arrow needs a citation; a link the diff cannot support is drawn dashed and labeled as an inference. When a drawn note cites something the diff does not hold, the reviewer is sent back once with the problem named; if it still does not read, the note is stored as written and shown as plain text.

The debt step gives observations and questions, never a score, and writes nothing for the contributor. A removed check opens at its old line, and the contributor can write a short intent record in their own words and copy it into the PR body. The record is kept for one snapshot, is not part of the answers or the signed record, and is never given to the reviewing agent.

The feedback schema, digest, head binding and completion record are unchanged. The runtime is still the Python standard library and Git, and the new Python modules parse on Python 3.8.

Verified with the review unit suites, the browser suites for the workbench and the flow layout (including a randomized layout property test), and scripts/test-interactive-review.sh.

Demo video:

codetrial-interactive-review-50s.mp4

Summary by cubic

The reference note shown after a contributor submits was plain prose, so a call path or design tradeoff had to be read out of text. This adds drawn reference notes — flow diagrams, comparison tables, and quoted code — plus a debt lens step that reads the change as technical, cognitive, and intent debt.

Draw the reference note

  • Every citation must be a line of the pinned diff; quoted code is taken from the diff, never typed.
  • Solid arrows need a citation; inferred arrows are dashed and labeled.
  • Clicking a citation opens its line in the diff.
  • The managed worker draws a flow by default; a note the diff doesn't hold is sent back once, then stored and shown as text.

Debt lens and intent record

  • Reads the change as technical, cognitive, and intent debt and shows observations and questions only, never a score.
  • Removed and added assertions are counted on the same basis, so a moved assertion no longer hides a lost one.
  • A removed check opens at its old line, and the contributor can write a short intent record to copy into the PR body.
  • The record is kept for one snapshot, stays out of the answers and signed record, and is never given to the reviewing agent; finishing refuses unsaved edits to the record.

Written for commit 5b19800. Summary will update on new commits.

View guided diff Turn on auto-fix

jserv added 3 commits November 8, 2026 00:12
The reference note shown after a contributor submits was plain text, so
a call path or a design tradeoff had to be read out of prose. It may now
carry a flow diagram, a comparison table and quoted code, written in a
small draft format.

Every citation must be a line of the pinned diff, code comes from the
diff and is never typed, and a solid arrow needs a citation while an
inference is dashed and labeled, so a picture cannot state an unverified
claim as fact. Clicking a citation opens its line in the diff. The page
draws what the parser returns, with no new dependency, and a note that
does not read stays valid as text, which the managed worker relies on.

Change-Id: I5887e4eadfadc661446a191d91f472963f21d962
A review can pass every test and still leave debt that no test shows:
removed checks nobody can explain, understanding that stayed with one
person, and a rationale that was never written down. The workbench now
ends with a step that reads the pinned diff and the session as
technical, cognitive and intent debt (Storey, arXiv 2603.22106) and asks
one question for each.

It scores nothing and writes nothing for the contributor. A removed
check opens at its old line, and the contributor can write a short
intent record in their own words and copy it into the pull request body.
The record is kept for one snapshot, is not part of the answers or the
signed record, and is never given to the reviewing agent.

Change-Id: I65ce7a7bce5e6fc4b9b4976e95037c137c41a99a
Managed mode, the default review, wrote plain prose, so the diagram,
table and quoted code were never drawn unless an agent wrote the note by
hand. The worker is now taught the draft format, shown the asked file's
lines with the numbers a citation uses, and asked for one small flow of
the key change unless the change has none.

A note the diff does not hold is sent back once with the problem named,
then stored as written and shown as text, so a wrong citation never
reaches the page as a fact and the worker never loops. Run against the
installed Codex CLI it returns a paragraph and a flow whose citations
the diff holds.

Change-Id: Iaa38ff7f0227a9b6b11e6535e4ee8169f79137b8
@jserv
jserv requested a review from ColtenOuO as a code owner November 7, 2026 23:24
cubic-dev-ai[bot]

This comment was marked as resolved.

Count removed and added assertions on the same basis, so a moved
assertion no longer hides a lost one. Finishing now refuses unsaved
intent edits as it does unsaved answers. A truncation notice starts
its own paragraph so it cannot swallow a closing fence. Load sibling
modules once, which makes the note-cache test patch the module the
workbench really calls, and run the explainer suite serially because
it patches a global.

Change-Id: I418aca425f9413edbe50320eda60e9edce5b47f4
cubic-dev-ai[bot]

This comment was marked as resolved.

The previous commit stacked a second lru_cache on a function that was
already cached, which left two caches and made cache_clear reach only
the outer one. Its message was also wrong that siblings were loaded
on every call; they were already kept.

Change-Id: I3ff2b4ae4ea4d6b60fb367eeabe931ffc8ed06d4
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.

1 participant