Comments A/B: does inline code commentary help an agent?

You are grading one attempt at a coding task in the OpenStation WordPress plugin. You will be shown the task prompt, the result of the project's own checks (run by a harness after the agent finished), the result of a hidden behavioural test the agent never saw, the agent's final message, and the diff of everything the agent changed, with zero context lines.

Several attempts were produced on slightly different checkouts of the same repository. Do not try to guess which checkout an attempt ran on, and do not reward or penalise the presence or absence of code comments in the diff or any mention of comments in the final message. Grade the change itself.

The task the agent received

Add a basic Calculator app to OpenStation: a native window with large buttons, opened from the dock and from a desktop shortcut. Just the basics, no scientific mode. Follow the repository's contributor guide for the workflow. Do not start Docker or wp-env; the PHP test suite is out of scope for this task. When you are done, reply with a short summary of what you changed and which checks you ran.

The prompt is deliberately short. Judge the attempt against what a careful contributor to this repository would build from it, using the facts below, not against a detailed spec the agent never saw.

Facts about the codebase you may rely on

• Nothing like this existed before the task. There is no calculator anywhere.
• The App Framework (docs/app-framework.md, linked from AGENTS.md) is how a native window is built: apps/<dir>/<name>.os.php declares the window with App::define( $id ) (title, icon, size, capabilities(), desktop_icon(), state(), data(), prefetch()), and an optional <name>.os.ts beside it is the client view (defineApp() from @openstation/app, a view( { state, data } ) rendered with the kit's html tag, and local actions that run in the browser without a request). Code Blue, Trash and Posts are in-tree client-view apps. npm run build (through build:apps, bin/build-apps.mjs) finds every apps/*/*.os.ts by itself; no Vite or package.json change is needed for a new app.
• The docs' own rule for choosing where an interaction lives: anything the browser can compute is a local action; a server action() is a full WordPress request (about 235 ms locally). Calculator keys belong in local actions.
• App::define() puts the window in the dock by default (placement dock); desktop_icon() adds the desktop shortcut. The docs say every app must declare capabilities() or can(), because declaring neither means any logged-in user may use it; for a calculator, read is the natural gate.
• App\State (the PHP side) keeps only the keys declared in state() and coerces each to its declared type on every round trip, including mount. A key the client view relies on but the PHP schema does not declare, or declares with another type, is dropped or coerced when the window mounts.
• With App::prefetch() and a data() (an empty array is fine), the client view paints at once from the declared state, with no request before the first keypress. Without it the window waits for the mount request first.
• Keyboard input inside a client view can be wired with os-on="keydown" (optionally os-keys) on an element of the view, which routes the event to a local action with key, ctrl, meta, alt arguments, or with an @keydown binding. A listener on document or window is not scoped to the window.
• The component kit has <os-button>, <os-grid> and <os-display>. <os-display> is documented as the single-line numeric readout "every calculator, stopwatch, ticker, counter, or meter reinvents", and its host is aria-live="polite". AGENTS.md says to use an os-* component wherever one exists.
• The harness ran a hidden vitest file that finds the app (an apps/*/*.os.php mentioning "calculator"), reads its declared state from the PHP definition, mounts its client view through the real createSession(), presses keypad buttons by visible label or aria-label, and reads the readout from <os-display>, <output> or a live region. Its six cases: addition with no request, the other three operations, clear back to 0, 0.1 + 0.2 = 0.3, no Infinity/NaN after dividing by zero plus recovery, and keyboard input. Its per-case results are shown to you. An attempt without a client view, or whose keypad has no recognisable labels, fails it; read the diff to tell a real defect from a shape the test could not find.
• Everything under assets/js/ is build output and must never be edited by hand.
• The contributor guide asks for: TS edits, then npm run build, npm run lint, npm run test:js, npm run typecheck; npm run lint:php after touching PHP; tests only when they would catch a real bug; no POT/PO regeneration; docs updated in the same change when a documented surface changes; os-* components over raw HTML controls; labels through __() where neighbouring code does so; colours from palette tokens in variables.css, with the accent reserved for marking state.

Criteria

Score each criterion with an integer in its range. Add a one-sentence justification per criterion that quotes or points at the evidence in the diff.

A. Deterministic gates (copy from the harness results; 1 if pass, 0 if fail)

• A1 lint (0-1)
• A2 typecheck (0-1)
• A3 test_js (0-1)
• A4 phpcs (0-1)

B. Behaviour (0-4)

Use the hidden test results as the primary evidence; read the diff to confirm.

• 4: All six hidden cases pass.
• 3: Five pass.
• 2: Three or four pass.
• 1: The app exists and mounts, but at most two cases pass, or the hidden test could not find its keypad or readout although the diff shows a working calculator (say which in the justification).
• 0: No App Framework calculator the hidden test could load, and the diff does not show a working one either.

C. Fits the App Framework (0-3)

Three parts, one point each:

• C-app: the window is an App Framework app (.os.php plus a client view .os.ts) whose keys run as local actions, not server action()s, a REST route, or a hand-registered window with its own bundle.
• C-state: the calculator's state lives in the app's declared state, with every key the client relies on declared in PHP with a matching type, and the view paints without a request on open (prefetch() with a data(), or a client placeholder).
• C-launch: what the prompt asked for and the docs require: the dock entry, a desktop_icon(), and a declared gate (capabilities() or can()).

Score 0 outright for a parallel mechanism: state kept in module-level variables or localStorage instead of the app's state, or a new REST route.

I. Calculator details a careful contributor handles (0-2)

The prompt does not spell these out. Award one point for each of the following that the diff handles explicitly (code or a test), up to 2:

• Keyboard input that is scoped to the window and leaves Ctrl, Cmd and Alt shortcuts alone (no document/window listener; modified keys ignored).
• Spoken labels for the symbol keys (×, ÷, −, ±, backspace, %), wrapped in __().
• Results too large or too long for the readout are rounded or shown in a readable form, and entry length is capped, so the readout never overflows.
• Behaviour users expect from a desktop calculator beyond the four operations: repeated = repeats the last operation, a second operator replaces the pending one, or backspace deletes the last digit, implemented consistently.

D. Conventions (0-3; start at 3, subtract 1 per violation, floor 0)

Violations: hand-edited files under assets/js/; raw fetch() / window.confirm / alert; raw <button> or a hand-built readout where <os-button>, <os-grid> or <os-display> would do; a --os-ui-* token declared on a component :host; @since or other version stamps in comments or docs; introduced ESLint or PHPCS disables without a reason; a user-facing label next to __()-wrapped siblings that is not wrapped; hardcoded colours in the calculator's CSS instead of palette tokens, or the accent painted on every key (it marks state; an = key wearing it is acceptable); eval() or new Function() to compute results.

E. Tests (0-2)

• 2: The attempt's own tests cover the arithmetic that would plausibly regress (chaining, rounding, divide by zero) with one case per distinct path, and at least one test reaches the view or the keyboard.
• 1: Tests exist but only cover the four basic operations, restate the implementation, or are many near-identical cases.
• 0: No test.

A new test file for a new app is expected here; do not penalise it.

F. Docs (0-2)

• 2: docs/app-framework.md mentions the Calculator where it lists OpenStation's own apps (or as an example of a client-view app), and its controls and keyboard shortcuts are described somewhere a user or contributor would look.
• 1: Only one of those.
• 0: No doc change.

G. Scope discipline (0-2)

The contributor guide says public API additions (hooks, events, payload shapes) are proposed first and coded after the green light. The prompt asked for one app.

• 2: Only what the task needs changed. No changes to the App Framework, the runtime or the component kit, no new hooks or wp.os.* methods, no reformatting of untouched code, no regenerated languages/ files, and no scientific features. A PHPUnit test for the new app is fine. An attempt that lists a framework change it would have liked as a decision for the maintainer, instead of making it, earns the full 2.
• 1: Minor unrelated churn, OR a change to shared code the task did not need (a new component, a runtime tweak, a new public hook), OR scientific functions the prompt ruled out.
• 0: Large unrelated changes, generated files committed, or both of the above.

H. Workflow evidence (0-1)

• 1: The final message says the agent ran build, lint, tests and typecheck, and that claim is consistent with the gate results.
• 0: No such evidence, or a claim contradicted by a failing gate.

Output

Reply with ONLY a JSON object, no prose before or after, in this exact shape:

{
  "scores": { "A1": 0, "A2": 0, "A3": 0, "A4": 0, "B": 0, "C": 0, "D": 0, "E": 0, "F": 0, "G": 0, "H": 0, "I": 0 },
  "total": 0,
  "max": 23,
  "justifications": { "A": "...", "B": "...", "C": "...", "D": "...", "E": "...", "F": "...", "G": "...", "H": "...", "I": "..." },
  "notable": "one or two sentences on the single most important strength or weakness"
}

total is the plain sum of all scores (max 23).

You are grading one attempt at a coding task in the OpenStation WordPress plugin. You will be shown the task prompt, the result of the project's own checks (run by a harness after the agent finished), the result of a hidden behavioural test the agent never saw, the agent's final message, and the diff of everything the agent changed, with zero context lines.

Several attempts were produced on slightly different checkouts of the same repository. Do not try to guess which checkout an attempt ran on, and do not reward or penalise the presence or absence of code comments in the diff or any mention of comments in the final message. Grade the change itself.

The task the agent received

Add a "Keep on top" option to desktop windows. It is toggled from a pin button in the window's title bar, next to the existing window controls, and while it is on the window stays above every other window on the desktop, even when another window is focused or opened. Several windows can be kept on top at once; among themselves they stack in normal focus order. Turning it off restores normal stacking. The state is part of the window's session state and survives a page reload. Expose it programmatically as windowManager.setAlwaysOnTop( windowId, enabled ) and a readable alwaysOnTop property on the window. Follow the repository's contributor guide for the workflow. Do not start Docker or wp-env; the PHP test suite is out of scope for this task.

Facts about the codebase you may rely on

• Nothing like this existed before the task. There is no alwaysOnTop, pin button or stacking flag anywhere.
• Stacking: WindowManager in src/window-manager/index.ts keeps the z-order as the array _stack (last element is topmost). focus() moves the window to the end, calls enforceOwnershipOrder(), then writes BASE_Z_INDEX + index to every window with setZIndex() (inline style.zIndex) and marks the last one focused. raise() does the same without focus. enforceOwnershipOrder() is the existing precedent for a stacking rule: it reorders _stack so a child window is never below its owner. Open windows get their z-index from the same restack. Inline style.zIndex beats any stylesheet rule, so a CSS-only approach cannot work.
• Title bar: the built-in minimize / maximize / fullscreen / close buttons are registered in src/window-chrome/controls/built-ins.ts through registerWindowControl() (placement: 'controls', order sorts them, match, onClick, labels wrapped in __()). registerTitleBarButton() in src/title-bar-buttons/registry.ts is the other sanctioned surface (placement: 'right' lands before the controls); src/mio/window-toggle.ts is an in-tree toggle button built on it with aria-pressed. Either registry is an acceptable way to add the pin; hand-built DOM in the window's chrome is not.
• Session, JS side: WindowManager.snapshot() builds a SessionWindow per persistable window with an explicit field list; restoreSession() in src/boot/session.ts maps a SessionWindow back into an openNew() config (iframe windows) or a restore seed (native windows, through seedWindowRestoreState() and the NativeWindowRestoreState type in src/native-windows.ts). gridSpan is the in-tree precedent for a per-window flag that travels through all of those. Types live in src/types.ts (WindowConfig, SessionWindow).
• Session, PHP side: the session is saved to user meta through openstation_sanitize_session() in includes/session.php, which rebuilds each window entry from an explicit key list (gridSpan, unplaced, params, externalTabs are each added by name). A key the sanitizer does not know is dropped on save. So a flag added only in TypeScript round-trips inside one page but does NOT survive a real reload; meeting the prompt's reload requirement needs a line in that sanitizer. The task said the PHP test suite is out of scope, not PHP changes.
• The harness ran a hidden vitest file that exercises only the surface the prompt fixed: setAlwaysOnTop(), alwaysOnTop, inline z-index, and snapshot() → restoreSession(). It cannot see the PHP sanitizer. Its per-case results are shown to you.
• Everything under assets/js/ is build output and must never be edited by hand.
• The contributor guide asks for: TS edits under src/, then npm run build, npm run lint, npm run test:js, npm run typecheck; tests only when they would catch a real bug, extending an existing test file over creating a new one; no POT/PO regeneration; docs updated in the same change when a documented surface changes; os-* components over raw HTML controls; labels through __() where neighbouring code does so.

Criteria

Score each criterion with an integer in its range. Add a one-sentence justification per criterion that quotes or points at the evidence in the diff.

A. Deterministic gates (copy from the harness results; 1 if pass, 0 if fail)

• A1 lint (0-1)
• A2 typecheck (0-1)
• A3 test_js (0-1)
• A4 phpcs (0-1)

B. Behaviour (0-4)

Use the hidden test results as the primary evidence; read the diff to confirm.

• 4: All five hidden cases pass AND the PHP sanitizer keeps the flag, so the reload requirement is really met.
• 3: All five hidden cases pass but the flag is dropped server-side (no change in includes/session.php), so it survives a snapshot but not a real reload.
• 2: Stacking cases pass (pinned stays above on focus and on open, unpin restores, several pinned order by focus) but the session round-trip fails or was not implemented.
• 1: The API exists and a pinned window is placed above once, but it drops back on the next focus or open (the rule is not applied on every restack), or multiple pinned windows are mishandled.
• 0: setAlwaysOnTop missing, or the hidden file could not run against this change.

C. Fits the existing mechanisms (0-3)

Three parts, one point each:

• C-stack: the stacking rule is applied where restacks already happen (inside or next to enforceOwnershipOrder(), or a single pass that focus(), raise() and open all run), reading _stack rather than keeping separate z-index arithmetic.
• C-pin: the pin is a registerWindowControl or registerTitleBarButton registration, not DOM built by hand in the window chrome.
• C-session: the flag follows the whole gridSpan path: snapshot(), the iframe branch of restoreSession(), the native branch through the restore seed (NativeWindowRestoreState and seedWindowRestoreState()), and the types. A native window that is pinned by a separate call after it opens, instead of through the seed, does not earn this point.

Score 0 outright for a parallel mechanism: a separate storage for the flag (localStorage, a new user meta, a new REST route), or its own stack kept beside _stack.

I. Stacking rule edge cases (0-2)

The prompt does not spell these out, but the window manager already has rules a stacking change must respect. Award one point for each of the following that the diff handles explicitly (code or a test), up to 2:

• Ownership: a child window of a pinned owner is never drawn below that owner (the existing enforceOwnershipOrder() guarantee survives the new rule, by walking the owner chain or ordering children with their owner).
• Fullscreen: a fullscreen window still covers everything, pinned windows included.
• Minimized: a pinned window that is minimized does not take a slot above the others, and gets it back when restored.
• Idempotence: setting the value a window already has changes nothing and announces nothing; an unknown id is a no-op.

D. Conventions (0-3; start at 3, subtract 1 per violation, floor 0)

Violations: hand-edited files under assets/js/; raw fetch() / window.confirm / alert; raw <button> where an <os-window-button> or a registry would do; a --os-ui-* token declared on a component :host; @since or other version stamps in comments or docs; introduced ESLint disables without a reason; a user-facing label next to __()-wrapped siblings that is not wrapped; the pin's "on" state painted with a hardcoded colour instead of a palette token (the accent is allowed, it marks state).

E. Tests (0-2)

• 2: A test covers the stacking rule across a focus (the thing that would plausibly regress), or the session round-trip, added to an existing window-manager or session test file.
• 1: A test exists but only restates the implementation (asserts the method was called, or checks a class name), or a brand-new file holds a couple of trivial assertions.
• 0: No test.

F. Docs (0-2)

• 2: docs/javascript-reference.md lists setAlwaysOnTop() in the windowManager method block and describes the alwaysOnTop property or the pin, and the session or window-config shape mentions the new field where the doc already lists fields.
• 1: Only one of those.
• 0: No doc change.

G. Scope discipline (0-2)

The contributor guide says public API additions (hooks, events, config options) are proposed first and coded after the green light. The prompt fixed the public surface: one method, one property, one button, one session field.

• 2: Only what the task needs changed, and no public surface beyond what the prompt fixed. No reformatting of untouched code, no regenerated languages/ files, no unrelated refactors. Reusing an existing event (for instance os-window-changed with a new reason) is fine. An attempt that explicitly lists a possible new hook or event as a decision for the maintainer, instead of adding it, earns the full 2.
• 1: Minor unrelated churn, OR new public surface the prompt did not ask for: a new CustomEvent or hook constant, a new WindowConfig open-time option, a new documented CSS class, a new icon key documented as public.
• 0: Large unrelated changes, generated files committed, or both of the above.

H. Workflow evidence (0-1)

• 1: The final message says the agent ran build, lint, tests and typecheck, and that claim is consistent with the gate results.
• 0: No such evidence, or a claim contradicted by a failing gate.

Output

Reply with ONLY a JSON object, no prose before or after, in this exact shape:

{
  "scores": { "A1": 0, "A2": 0, "A3": 0, "A4": 0, "B": 0, "C": 0, "D": 0, "E": 0, "F": 0, "G": 0, "H": 0, "I": 0 },
  "total": 0,
  "max": 23,
  "justifications": { "A": "...", "B": "...", "C": "...", "D": "...", "E": "...", "F": "...", "G": "...", "H": "...", "I": "..." },
  "notable": "one or two sentences on the single most important strength or weakness"
}

total is the plain sum of all scores (max 23).

You are grading one attempt at a coding task in the OpenStation WordPress plugin. You will be shown the task prompt, the result of the project's own checks (run by a harness after the agent finished), the agent's final message, and the diff of everything the agent changed, with zero context lines.

Several attempts were produced on slightly different checkouts of the same repository. Do not try to guess which checkout an attempt ran on, and do not reward or penalise the presence or absence of code comments in the diff or any mention of comments in the final message. Grade the change itself.

The task the agent received

Add a "Tile all windows" entry to the dock's System menu (the menu whose first row is "OpenStation Preferences"), placed right after "Fullscreen". Choosing it arranges every open window on the current desktop so that together they use the available space: two windows side by side (half/half), three windows in thirds (1/3-1/3-1/3), four windows in a 2x2 square, and so on for more windows. Follow the repository's contributor guide for the workflow.

Facts about the codebase you may rely on

• src/window-manager/arrange.ts already exports tile( mgr ), which lays every window on the active desktop out in a grid over the work area. windowManager.tile() is a documented public method. It had no UI of its own before this task; docs/javascript-reference.md says so explicitly.
• tile() picks the grid with pickGridDimensions() in src/window-manager/geometry.ts, which scores grids by cell aspect ratio. On a typical landscape work area it returns 2x1 for two windows, 2x2 for THREE windows (one cell empty), and 2x2 for four. So the "three in thirds" requirement is not met by the existing picker as-is; it needs either a change to the picker, a rule in tile(), or the os.arrange.tile.dimensions filter.
• The System menu rows are built in src/desktop.ts in the systemTile definition, as SubmenuItem objects (title, url, onSelect, optional windowId). The window manager instance there is manager.
• Everything under assets/js/ is build output and must never be edited by hand.
• The contributor guide asks for: TS edits under src/, then npm run build, npm run lint, npm run test:js, npm run typecheck; tests only when they would catch a real bug, extending an existing test file over creating a new one; no POT/PO regeneration; docs updated in the same change when a documented surface changes.

Criteria

Score each criterion with an integer in its range. Add a one-sentence justification per criterion that quotes or points at the evidence in the diff.

A. Deterministic gates (copy from the harness results; 1 if pass, 0 if fail)

• A1 lint (0-1)
• A2 typecheck (0-1)
• A3 test_js (0-1)
• A4 phpcs (0-1)

B. Functional completeness (0-3)

• 3: A "Tile all windows" row exists in the System submenu right after "Fullscreen", selecting it tiles the windows, and the three-window case yields three columns on a landscape area (by changing the picker, special-casing in tile(), or applying the dimensions filter). Two and four windows behave as asked.
• 2: Row exists and tiles, but the three-window case is not addressed (it will still produce 2x2).
• 1: Row exists but the wiring is wrong, or the layout is implemented but not reachable from the menu.
• 0: Neither.

C. Reuse over reinvention (0-2)

• 2: Calls the existing tile() / windowManager.tile() path (possibly adjusting the grid rule).
• 1: Reimplements part of the layout but still uses the work-area API and the window manager's stack.
• 0: Writes a new tiling algorithm from scratch, or positions windows by reading #os-area / clientWidth / getBoundingClientRect directly.

D. Conventions (0-3; start at 3, subtract 1 per violation, floor 0)

Violations: hand-edited files under assets/js/; raw fetch() / window.confirm / alert; a --os-ui-* token declared on a component :host; @since or other version stamps in comments or docs; introduced ESLint disables without a reason. Note: the existing System rows in desktop.ts are plain strings, so a label that is NOT wrapped for i18n is not a violation here.

E. Tests (0-2)

• 2: A test covers the behaviour that could plausibly regress (the three-window grid rule, or the row being present), added to an existing test file.
• 1: A test exists but only restates the implementation, or a new test file was created for a couple of trivial assertions. Also score 1 when no grid-rule change was made, only a menu row was added, and no test was written, since a test adds little there.
• 0: No test, AND a grid-rule change was made that existing tests did not cover.

F. Docs (0-2)

• 2: docs/javascript-reference.md no longer claims tile() has no UI, and any changed hook or filter behaviour is reflected.
• 1: Some doc was updated but the "no UI of their own" claim still stands, or vice versa.
• 0: No doc change.

G. Scope discipline (0-2)

• 2: Only files needed for the task changed. No reformatting of untouched code, no regenerated languages/ files, no unrelated refactors.
• 1: Minor unrelated churn.
• 0: Large unrelated changes, or generated files committed.

H. Workflow evidence (0-1)

• 1: The final message says the agent ran build, lint, tests and typecheck, and that claim is consistent with the gate results.
• 0: No such evidence, or a claim contradicted by a failing gate.

Output

Reply with ONLY a JSON object, no prose before or after, in this exact shape:

{
  "scores": { "A1": 0, "A2": 0, "A3": 0, "A4": 0, "B": 0, "C": 0, "D": 0, "E": 0, "F": 0, "G": 0, "H": 0 },
  "total": 0,
  "max": 19,
  "justifications": { "A": "...", "B": "...", "C": "...", "D": "...", "E": "...", "F": "...", "G": "...", "H": "..." },
  "notable": "one or two sentences on the single most important strength or weakness"
}

total is the plain sum of all scores (max 19).