Skip to content

Detect model name collisions and extend duplicate-name policy#494

Open
Papyszoo wants to merge 13 commits into
mainfrom
copilot/detect-model-name-collisions
Open

Detect model name collisions and extend duplicate-name policy#494
Papyszoo wants to merge 13 commits into
mainfrom
copilot/detect-model-name-collisions

Conversation

@Papyszoo
Copy link
Copy Markdown
Owner

Summary

Adds a configurable model duplicate-name policy (Reject / AutoRename) and extends it to all asset types and WebDAV uploads. Includes the frontend "Model Upload Behavior" settings section, a new Models.Name index, comprehensive E2E coverage, and the post-policy test cleanup.

What's in this PR

  • Backend: ModelDuplicateNamePolicy setting with Reject/AutoRename support; Models.Name index for collision detection; policy enforcement extended to all asset types and WebDAV writes.
  • Frontend: Model Upload Behavior settings section.
  • Tests: E2E coverage for the duplicate-name policy; ~22 pre-existing failures resolved; demo prewarm timeout/retry hardening; HDRI thumbnail transition tracking fix; parallel env-map count tolerance.
  • Cleanup (this commit batch): Removed the stale-texture-set cleanup helper from e2e steps (the policy makes the orphans it was deleting impossible) and made the env map thumbnail wait robust against virtualised list re-renders.

Test plan

  • CI green on the full e2e suite
  • Manual: upload model with existing name in Reject mode → rejected with clear error
  • Manual: upload model with existing name in AutoRename mode → auto-renamed with suffix
  • Manual: try the same flows for texture sets, environment maps, sprites, sounds
  • Manual: WebDAV PUT of a colliding name follows the active policy

Copilot AI and others added 13 commits April 17, 2026 19:09
…port

- Add ModelDuplicateNamePolicy to SettingKeys and SettingValidator
- Add ExistsByNameAsync and GetNamesByPrefixAsync to IModelRepository
- Create ModelNameService for centralized duplicate name resolution
- Update AddModelCommandHandler and CreateModelFromBlendCommandHandler
- Update GetSettingsQueryResponse to include new setting
- Handle 409 Conflict in WebDAV middleware and model REST endpoint
- Update existing tests for new constructor parameter

Agent-Logs-Url: https://github.com/Papyszoo/Modelibr/sessions/48d971e5-9443-4631-9c23-532e5344d6c7

Co-authored-by: Papyszoo <12179062+Papyszoo@users.noreply.github.com>
- Add modelDuplicateNamePolicy to settings API response type
- Add updateSetting helper for individual key-value setting updates
- Add Model Upload Behavior section to Settings page with policy dropdown
- Update demo mock handlers with new field and PUT /settings/:key handler
- Shift accordion section indices for Blender/SSL/WebDAV sections

Agent-Logs-Url: https://github.com/Papyszoo/Modelibr/sessions/48d971e5-9443-4631-9c23-532e5344d6c7

Co-authored-by: Papyszoo <12179062+Papyszoo@users.noreply.github.com>
- Add IX_Models_Name database index with EF Core migration for
  ExistsByNameAsync and GetNamesByPrefixAsync query performance
- Add E2E scenarios: WebDAV PUT reject (409), WebDAV PUT autorename (201),
  REST upload reject (409) for duplicate model names
- Add api-helper methods: uploadModelRaw, updateSetting, getSettings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Set AutoRename as default policy in global-setup.ts before each phase
- Fail fast if policy setting fails (throw instead of warn)
- Fetch actual model name from API in recycle bin steps instead of
  hardcoding 'test-cube' (supports AutoRename suffixes)
- Fetch actual model name in filter steps instead of deriving from
  local filename
- Add After hook to restore AutoRename after duplicate-name scenarios
  to prevent poisoning later tests in the slow phase

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- URL-encode setting key in settingsApi.ts updateSetting()
- Add ok() assertions before parsing model detail JSON in E2E recycle
  bin steps to fail fast on non-2xx responses

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Waveform and environment map thumbnail prewarm runs fire-and-forget
in demo mode. On slow CI runners, the audio decode + canvas render
can exceed 30s. Increase assertions from 30s to 60s to accommodate
CI variability without weakening the test assertions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The demo mode prewarm functions (waveform, env map thumbnails) run
fire-and-forget during initDemoData(). The React components don't
reactively pick up IndexedDB changes, so if the initial sound list
fetch happens before prewarm completes, the waveform img element
never appears in the DOM — even with long timeouts.

Replace static timeout waits with expect().toPass() loops that reload
the page between attempts. This ensures the component re-fetches
fresh data from IndexedDB, making the test resilient to slow CI
runners where prewarm may take 30-90 seconds.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename ModelNameService to AssetNameService with delegate-based signature
- Apply DuplicateNamePolicy to sprites, sounds, texture sets, environment maps
- Fix WebDAV to use asset Name instead of File.OriginalFileName for sprites/sounds
- Add GetVirtualFileName helper in WebDavUtilities
- Rename setting key: ModelDuplicateNamePolicy → DuplicateNamePolicy (with fallback)
- Update frontend labels and API types
- Update E2E setting key references
- All 458 backend tests pass, 244 frontend tests pass

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The transition tracking reads recorded states from an in-browser
interval. A race condition occurs when Playwright's poll confirms
the thumbnail loaded but the 100ms interval hasn't captured the
final state yet. Fix by:

1. Exposing pushState on the tracking object
2. Flushing state in getTrackedCardThumbnailTransitions before reading
3. Using expect.poll for the transitions assertion as a safety net

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'toolbar counter should increase by N' step used strict equality,
which fails when parallel workers create environment maps between the
remember and check steps. Use >= to tolerate parallel interference.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wait for the entry by name and scroll it into view before asserting the
thumbnail is visible. Without this the assertion races with virtualised
list re-renders and intermittently fails after the duplicate-name policy
created denser test inventories.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The duplicate-name policy prevents the orphan rows the helper was
deleting, so the cleanup pass is now dead code that adds startup latency
for every scenario.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

2 participants