[PR #148] [MERGED] feat: pre-compile name-collision preview in profile composer #148

Closed
opened 2026-09-02 07:34:53 +02:00 by zaph0d · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/niels-emmer/myace/pull/148
Author: @niels-emmer
Created: 9/1/2026
Status: Merged
Merged: 9/1/2026
Merged by: @niels-emmer

Base: mainHead: feat/pre-compile-collision-preview


📝 Commits (1)

  • 390f105 feat: pre-compile name-collision preview in profile composer

📊 Changes

5 files changed (+475 additions, -3 deletions)

View changed files

📝 frontend/src/components/ProfileForm.tsx (+55 -1)
frontend/src/lib/collisions.test.ts (+117 -0)
frontend/src/lib/collisions.ts (+81 -0)
frontend/src/pages/ProfileComposer.test.tsx (+173 -0)
📝 frontend/src/pages/ProfileComposer.tsx (+49 -2)

📄 Description

What does this change?

Adds a pre-compile name-collision preview to the Profile Composer create form. When a user composes a profile (one base + several additional collections), the form now fetches the selected collections' artifacts and runs a client-side mirror of compile_profile()'s rule-29 dedup step, rendering an advisory amber panel that names each colliding artifact, both collections, and the winner — before the profile is saved. Each collision has a "Disable in this profile" action that adds the losing artifact's id to disabled_artifact_ids (a field compile_profile() already respects), so the user can resolve the collision at edit time instead of discovering it on /build/compile.

Pure frontend change: no backend, no new endpoint, no migration. The preview replicates the backend's exact semantics so it stays consistent with what compilation actually does.

N/A — follow-up feature to the rule-29 collision work (see docs/plans/pre-compile-collision-preview.md).

Type of change

  • Bug fix
  • New feature
  • Breaking change (fix or feature that would cause existing behavior to change)
  • Documentation only
  • Chore / refactor

How was this tested?

  • cd frontend && npm run test — 106 tests pass (21 files), including 8 new unit tests for detectNameCollisions() and 3 new component tests for the panel + disable action.
  • cd frontend && npm run lint — 0 errors (8 pre-existing warnings in untouched files).
  • cd frontend && npm run buildtsc -b and vite build both clean.

Open questions — decisions made

Per the plan's "Open questions" section, here are the decisions made during implementation:

  1. Panel placement — The useQueries fan-out lives in ProfileComposer.tsx (it already owns data-fetching); the collision list and a disable handler are passed down to ProfileForm.tsx, where the panel renders colocated with the collection pickers. This matches the plan's leaning toward ProfileForm.tsx for colocation while keeping the data-fetching where the component already has it.
  2. Edit path — Out of scope for v1. The create form is the only composition surface; profile editing happens via PATCH on the detail page. Noted as future work.
  3. Disable vs. drop — Implemented "Disable in this profile" as the surgical action. No "remove collection" hint was added: the pickers are immediately adjacent to the panel, so dropping a collection is already one click away, and a hint would add noise for the common single-collision case.

Checklist

  • I read CONTRIBUTING.md
  • Tests pass locally (pytest / npm run test / npx tsc -b as applicable)
  • I added tests for new behavior, or explained why none are needed
  • Schema changes include an Alembic migration with a working downgrade() (N/A — no schema change)
  • I updated README.md, AGENTS.md/CLAUDE.md, and/or docs/ if this change affects what they document (N/A — pure UX addition; compile-time warning behavior unchanged)

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/niels-emmer/myace/pull/148 **Author:** [@niels-emmer](https://github.com/niels-emmer) **Created:** 9/1/2026 **Status:** ✅ Merged **Merged:** 9/1/2026 **Merged by:** [@niels-emmer](https://github.com/niels-emmer) **Base:** `main` ← **Head:** `feat/pre-compile-collision-preview` --- ### 📝 Commits (1) - [`390f105`](https://github.com/niels-emmer/myace/commit/390f105ccf639e0070059effcdd6d1ca778e9466) feat: pre-compile name-collision preview in profile composer ### 📊 Changes **5 files changed** (+475 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `frontend/src/components/ProfileForm.tsx` (+55 -1) ➕ `frontend/src/lib/collisions.test.ts` (+117 -0) ➕ `frontend/src/lib/collisions.ts` (+81 -0) ➕ `frontend/src/pages/ProfileComposer.test.tsx` (+173 -0) 📝 `frontend/src/pages/ProfileComposer.tsx` (+49 -2) </details> ### 📄 Description ## What does this change? Adds a pre-compile name-collision preview to the Profile Composer create form. When a user composes a profile (one base + several additional collections), the form now fetches the selected collections' artifacts and runs a client-side mirror of `compile_profile()`'s rule-29 dedup step, rendering an advisory amber panel that names each colliding artifact, both collections, and the winner — before the profile is saved. Each collision has a **"Disable in this profile"** action that adds the losing artifact's id to `disabled_artifact_ids` (a field `compile_profile()` already respects), so the user can resolve the collision at edit time instead of discovering it on `/build/compile`. Pure frontend change: no backend, no new endpoint, no migration. The preview replicates the backend's exact semantics so it stays consistent with what compilation actually does. ## Related issue N/A — follow-up feature to the rule-29 collision work (see `docs/plans/pre-compile-collision-preview.md`). ## Type of change - [ ] Bug fix - [x] New feature - [ ] Breaking change (fix or feature that would cause existing behavior to change) - [ ] Documentation only - [ ] Chore / refactor ## How was this tested? - `cd frontend && npm run test` — 106 tests pass (21 files), including 8 new unit tests for `detectNameCollisions()` and 3 new component tests for the panel + disable action. - `cd frontend && npm run lint` — 0 errors (8 pre-existing warnings in untouched files). - `cd frontend && npm run build` — `tsc -b` and `vite build` both clean. ## Open questions — decisions made Per the plan's "Open questions" section, here are the decisions made during implementation: 1. **Panel placement** — The `useQueries` fan-out lives in `ProfileComposer.tsx` (it already owns data-fetching); the collision list and a disable handler are passed down to `ProfileForm.tsx`, where the panel renders colocated with the collection pickers. This matches the plan's leaning toward `ProfileForm.tsx` for colocation while keeping the data-fetching where the component already has it. 2. **Edit path** — Out of scope for v1. The create form is the only composition surface; profile editing happens via PATCH on the detail page. Noted as future work. 3. **Disable vs. drop** — Implemented "Disable in this profile" as the surgical action. No "remove collection" hint was added: the pickers are immediately adjacent to the panel, so dropping a collection is already one click away, and a hint would add noise for the common single-collision case. ## Checklist - [x] I read [`CONTRIBUTING.md`](../CONTRIBUTING.md) - [x] Tests pass locally (`pytest` / `npm run test` / `npx tsc -b` as applicable) - [x] I added tests for new behavior, or explained why none are needed - [ ] Schema changes include an Alembic migration with a working `downgrade()` (N/A — no schema change) - [ ] I updated `README.md`, `AGENTS.md`/`CLAUDE.md`, and/or `docs/` if this change affects what they document (N/A — pure UX addition; compile-time warning behavior unchanged) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
zaph0d 2026-09-02 07:34:53 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
github-mirrors/myace#148
No description provided.