[vpetersson-bot]

The bug

Add a web asset, then open Add asset again: the first asset's URL is still sitting in the Asset URL box.

The modal is hidden with x-show, never unmounted, so the plain <input> — and the nested x-data="{ tab: 'uri' }" behind the tab strip — outlive a close. Nothing ever cleared them. Hitting Enter on that pre-filled field is one keystroke away from a duplicate asset.

The fix

The URL input is now bound to homeApp state (addUri), and the tab selection moves out of the modal's nested x-data into the same component, so openAdd() can reset both. Resetting on open rather than on close means the form is fresh however the last one ended — saved, cancelled, Esc, or backdrop click.

Two behaviours are handled deliberately:

• An upload the operator hid mid-batch keeps running, and its progress lives on the file tab, so a reopen during a batch goes there. The tab strip stays clickable during an upload, so whichever tab they happened to be on when they hid the modal says nothing about where the progress is.
• The Apps pane is its own appsTab() component, so it resets itself off an add-modal-open event rather than being left parked on the last app's filled-in config form — the same staleness, one tab over. It exposes reset(), which only unwinds the config phase: an empty or unreachable catalog sets loaded plus its own error message, and bouncing that to ready would show an empty grid that load() then declines to re-fetch.

Also in scope: the rest of the category

A sweep for the same shape — an editable control in a surface that hides rather than unmounts, with nothing to reset it — turned up one more instance. The Apps install form's Name box was bound one-way with :value, so the DOM drifted from assetName as soon as the operator renamed an app. Re-selecting the same app re-seeds the identical string, which is no reactive change and therefore no re-render: the previous install's name was still sitting in the box. x-model keeps them in step.

The rest came back clean, for a reason worth stating: the edit and bulk-edit modals wrap their forms in <template x-if>, which tears the subtree down on close and rebuilds it from current state on reopen. The delete and reboot confirmations hold no input. The import and Screenly-migration wizards rebuild items/selected on every validate. The Add pane was the only form gated by x-show alone — which is why it was the only one that broke.

tests/test_template_state_reset.py keeps it that way: a static guard (no DB, no browser, no build — same model as test_design_tokens.py) that fails any editable control bound one-way with :value outside a <template x-if>. No allowlist. Its docstring records why it deliberately stops short of policing every x-show subtree: /settings holds typed values across a disclosure toggle on purpose, and only what the surface means can tell that apart from the Add modal, where every open is a fresh action. That shape stays pinned behaviourally instead.

Testing

• bun test — 140 pass. New add-modal-reset.test.ts mounts real Alpine against the modal markup rather than calling openAdd() directly, so it pins the parts that aren't observable from the component object: the x-model write-through to the DOM input, the cross-scope tab lookup from inside appsTab's nested x-data, and the @add-modal-open.window listener. Six cases — URL cleared, tab reset, file tab kept during an upload, Apps pane back on the catalog, app-store error left alone, and the Apps Name box re-seeded on re-select.
• Mutation-checked: the suite fails when the listener is removed, when reset() is widened back to an unconditional back(), and when the Name box reverts to :value. The static guard was likewise confirmed to fire on the real pre-fix template.
• Two Playwright regression tests added to tests/test_app.py reproducing the reported flow (add via URL, reopen, assert the field is empty) and the tab reset.
• bunx tsc --noEmit, ruff check, ruff format --check all clean.

The Playwright suite could not be run in this agent runtime (out of disk space, so the Docker test stack wouldn't build) — CI was its first execution, and the integration job is green.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Two moderate issues remain in upload-tab preservation and Apps-pane error handling; one test-coverage nit also remains.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes stale Add asset modal state when reopened by resetting the URL, selected tab, and Apps configuration while preserving active upload progress.

Changes:

• Binds and resets the URL input and shared tab state.
• Resets the Apps pane on modal open.
• Adds unit and Playwright regression tests.

Review findings:

• Moderate: Active uploads may reopen on the wrong tab.
• Moderate: Apps-pane error states can be lost after reset.
• Nit: Apps-pane reset coverage lacks a DOM/integration assertion.

File summaries

File	Summary
tests/test_app.py	Adds modal reopen regression tests.
src/anthias_server/app/templates/_asset_modal.html	Binds URL state, moves tab state, and adds Apps reset handling.
src/anthias_server/app/static/src/home.ts	Implements modal state reset logic.
src/anthias_server/app/static/src/home.test.ts	Adds state-reset and event tests.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Tests do not exercise the production template wiring, and the PR description reports an inaccurate test count.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/anthias_server/app/static/src/add-modal-reset.test.ts:20

• This test mounts MODAL, a hand-written fragment, rather than the production _asset_modal.html, so it cannot catch a mismatch in the actual template. For example, removing or misspelling @add-modal-open.window="reset()" or x-model="addUri" in _asset_modal.html leaves these tests green because the copied attributes remain. Please add coverage that renders/drives the real Django template (or an end-to-end regression for the Apps pane) instead of relying solely on this duplicate markup.

// The panes the reset touches, trimmed to the attributes under test.
// Kept in sync with _asset_modal.html by hand — a bun test can't render
// a Django template.

• Files reviewed: 5/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The modal reset tests do not verify the rendered template or Apps reset wiring.

Review details

Suppressed comments (1)

src/anthias_server/app/static/src/add-modal-reset.test.ts:21

• MODAL is a hand-maintained subset rather than the Django template, so these tests can pass even if _asset_modal.html loses x-model, changes the scope, or removes @add-modal-open.window: the copied string still contains the expected attributes. The Playwright additions do not cover the Apps reset, leaving that production wiring unverified. Exercise the rendered template for this path (for example, add an integration case that selects an app, closes/reopens, and asserts the catalog), or render the Django template in the test setup.

// The panes the reset touches, trimmed to the attributes under test.
// Kept in sync with _asset_modal.html by hand — a bun test can't render
// a Django template.
const MODAL = `

• Files reviewed: 5/6 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Expand template scanning recursively and strengthen DOM coverage for the production listener and Apps name binding.

Review details

Suppressed comments (2)

src/anthias_server/app/static/src/add-modal-reset.test.ts:20

• This DOM suite mounts a hand-copied subset rather than rendering _asset_modal.html, so it cannot catch regressions in the production @add-modal-open.window listener or the Apps Name x-model binding; changing either attribute in the Django template leaves these tests green. The Playwright additions cover the URL and tab flows, but not these Apps-pane behaviors. Add an assertion against the rendered template (for example, an integration test) or otherwise generate this fixture from the template.

// The panes the reset touches, trimmed to the attributes under test.
// Kept in sync with _asset_modal.html by hand — a bun test can't render
// a Django template.

tests/test_template_state_reset.py:161

• The guard only scans HTML files directly under templates/, so any editable control added in a template subdirectory will be silently omitted from this repository-wide check. Other template guards in tests/test_template_views.py and tests/test_design_tokens.py use recursive scans; use the same here so the test cannot pass without checking all templates.

TEMPLATE_FILES = sorted(TEMPLATES.glob('*.html'))

• Files reviewed: 6/7 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The modal regression test uses a reduced fixture instead of the rendered template, so it may miss wiring regressions.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/anthias_server/app/static/src/add-modal-reset.test.ts:20

• This suite mounts a hand-maintained reduced MODAL string rather than the rendered _asset_modal.html, so it will still pass if the production template drops or misspells x-model="addUri" or @add-modal-open.window="reset()". Please add an assertion against the rendered Django/Playwright markup (or generate this fixture from the template) so the regression covers the actual wiring.

• Files reviewed: 6/7 changed files
• Comments generated: 0 new
• Review effort level: Lite

[vpetersson-bot]

Both points from the last pass are addressed in 71ea200.

Fixture drift: you're right that the reduced MODAL string made the suite evidence about itself rather than about production. Four cases now read _asset_modal.html and assert the exact attributes the fixture leans on — x-model="addUri", x-model="assetName", @add-modal-open.window="reset()", and that the Add pane hasn't re-grown a nested x-data declaring tab (which is what put it out of openAdd()'s reach in the first place). Verified by mutation: dropping the URL binding or renaming the listener in the Django markup now fails the suite where it previously stayed green.

I kept the reduced fixture rather than driving this through Playwright — the Apps pane needs a live app-store index to reach the config phase, so a browser test would need a stubbed store to cover what these cases cover directly. The template assertions close the wiring gap that made the fixture untrustworthy.

Non-recursive glob: fixed, now rglob, matching test_design_tokens.py. No template subdirectories exist today, so this is purely so a future move can't silently shrink a repo-wide guard.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

🟢 Approval recommended

The reviewed changes have regression coverage and no unresolved blocking issues.

Review details

• Files reviewed: 6/7 changed files
• Comments generated: 0 new
• Review effort level: Lite