docs: reconcile milestone acceptance and propose catalog core design
This commit is contained in:
1 parent
c1ef2ee97a
commit
390a7a557a
3 files changed
+199
-8
No files matched your search
@@ -0,0 +1,76 @@
|
||||
# Acceptance reconciliation — 2026-10-12
|
||||
|
||||
Baseline: `c1ef2ee`. This review compares the live Gitea acceptance criteria with
|
||||
source, tests, and recorded runtime evidence. Implementation is not milestone
|
||||
acceptance. The earlier assessment understated remaining M2/M3 work.
|
||||
|
||||
## Evidence
|
||||
|
||||
- Fresh Windows run: `composer test` — 291 tests, 1801 assertions, 22 skips; exit 0.
|
||||
`composer lint` — exit 0. Expected HTTP failure-path logs are not test failures.
|
||||
- Recorded isolated Linux/PostgreSQL 17 run: 291 tests, 1863 assertions, 7 skips;
|
||||
tests and lint exit 0. Not rerun for this review. See `postgres-tests.md`.
|
||||
- M1 runtime evidence is historical: local task reports document HTTP/FPM behavior,
|
||||
database volume survival across down/up, runtime-user permission checks, and asset
|
||||
watch/build delivery. Those ignored reports are not public repository artifacts.
|
||||
- Source-derived recovery risks below need executable regression tests; they are
|
||||
not claims that a new crash experiment was run.
|
||||
|
||||
## Issue disposition
|
||||
|
||||
Tracker reconciliation completed: #6, #7, #8, #9, #11 and #23 were closed with
|
||||
evidence comments. All other reviewed issues remain open with gap comments. The
|
||||
closure-candidate labels below describe the audit recommendation that led to those
|
||||
changes; issue states were rechecked through the live API afterward.
|
||||
|
||||
| Issues | Assessment and outstanding acceptance |
|
||||
| --- | --- |
|
||||
| #6–9, #11 | Closure candidates: Compose, application skeleton, config, assets and logging have implementation, tests and historical runtime evidence. Preserve that evidence in issue comments. |
|
||||
| #10 | Wrappers implemented; complete migration/seed command smoke coverage remains to be reconciled. |
|
||||
| #12 | Phinx runs production migrations through PG fixtures; direct migration rollback/clean-start acceptance remains separate from fixture teardown. |
|
||||
| #13 | Folder schema exists; review root representation, hierarchy invariants and deletion behavior against the issue before closing. |
|
||||
| #14 | Models exist; full specified metadata and timestamp semantics need reconciliation. |
|
||||
| #15 | Multipart files and indexed hashes exist; source mtime and explicit format metadata are missing, and lifecycle naming needs reconciliation. |
|
||||
| #16 | Tags/join table exist; model category assignment is absent and root category name uniqueness is not guaranteed by nullable-parent uniqueness. |
|
||||
| #17 | Thumbnail rows exist; exactly-one-primary invariant and demonstrated separate storage remain missing. |
|
||||
| #18 | Jobs/items/counters/claims exist; confirmed manifest, attempts, resulting-file reference, duplicate decisions and relationships remain incomplete. |
|
||||
| #19 | Backup intent exists; executable consistent backup/restore guidance and clean-environment restore drill are missing. |
|
||||
| #20 | Scanner exists; configured source mounts, destination exclusion, immutable manifest and explicit grouping preview/confirmation are incomplete. |
|
||||
| #21 | Space check exists; required/available byte reporting and production safety-margin configuration remain incomplete. |
|
||||
| #22 | STL validator is covered; one combined extension allowlist does not implement the requested separate supplementary-file policies. |
|
||||
| #23 | Closure candidate: streaming hashing, mutation checks and persisted indexed hashes are implemented and covered. |
|
||||
| #24 | Verified copier exists; nested hierarchy and crash-safe collision publication/reconciliation remain incomplete in worker integration. |
|
||||
| #25 | Queue/worker/locking exist; confirmed grouping and full replay/retry guarantees remain incomplete. |
|
||||
| #26 | Repository progress exists; caller-facing polling, full state distinctions and worker handling of empty jobs remain incomplete. |
|
||||
| #27 | Failures persist; supported retry with attempt history and persisted-message redaction remain incomplete. |
|
||||
| #28 | Importer-facing repository exists; explicit reassignment, catalog-owned transaction contract and broader metadata/lifecycle contracts remain incomplete. |
|
||||
|
||||
## Important implementation findings
|
||||
|
||||
- `ImportService::group()` guesses associations from the first directory component.
|
||||
`ensureModel()` reuses folder/name identities. Neither is user confirmation.
|
||||
The M3 ledger's directory-grouping decision does not supersede live #20 criteria.
|
||||
- The scanner retains relative paths, but job creation discards them. The worker
|
||||
constructs destinations from a model folder and basename, flattening deeper paths.
|
||||
- The worker persists a planned name before copying, but saves a collision suffix
|
||||
only after publication. A crash in between can lead to a second suffixed copy on
|
||||
replay. Existing successful-collision tests do not establish this crash boundary.
|
||||
- A catalog failure after publication can leave a file on disk and a terminal failed
|
||||
item. Source validation also precedes adoption of an already published copy.
|
||||
- Item size can be replaced during planning while scanned job byte totals remain
|
||||
unchanged. Failure text is persisted from exception messages without general
|
||||
redaction. Progress and retry need workflow-level contracts, not just counters.
|
||||
|
||||
## Proposed next delivery order
|
||||
|
||||
1. Complete issue/evidence reconciliation without closing partial milestones.
|
||||
2. Design a bounded #28 catalog contract with explicit grouping/reassignment and
|
||||
stable model/file identities; retain PHP/PDO/Phinx and the existing repositories.
|
||||
3. Extend #18/#20 with immutable, confirmed manifests and safe source selection.
|
||||
4. Resolve #24/#25 publication recovery before calling imports restart-safe.
|
||||
5. Add progress and retry contracts (#26/#27), then M4 CRUD/removal/downloads and
|
||||
serialized folder/model moves. Complete schema prerequisites as their contracts
|
||||
are settled, using new migrations rather than rewriting applied migrations.
|
||||
|
||||
M5 metadata, M6 browse/search, M7 lifecycle/duplicate review and M8 hardening/release
|
||||
remain ahead. No production readiness or percentage-complete claim is implied.
|
||||
@@ -0,0 +1,108 @@
|
||||
# Catalog core foundation — proposed design
|
||||
|
||||
Status: draft for user approval; no product implementation authorized by this document.
|
||||
|
||||
## Intent
|
||||
|
||||
Build the missing #28 catalog contract before extending UI and importer workflows.
|
||||
The existing PHP/PDO repositories are useful foundations, but folder/name-based
|
||||
inference is not explicit grouping. A logical model must have stable identity,
|
||||
independent of its title and physical files.
|
||||
|
||||
## Goals
|
||||
|
||||
- Explicit model creation and file assignment, including multipart STL models with
|
||||
individually selected supplementary images/documents.
|
||||
- Atomic catalog operations with rollback and foreign-key integrity on PostgreSQL.
|
||||
- Preserve file IDs, bytes, paths and timestamps when associations change.
|
||||
- Make future metadata, lifecycle, import and removal operations use one contract.
|
||||
|
||||
## Non-goals
|
||||
|
||||
No browse UI, moves, physical deletion, downloads, duplicate decisions, retry engine
|
||||
or import crash-recovery fix in this first slice. Do not silently relabel unresolved
|
||||
M2/M3 acceptance as complete. No new framework or runtime dependency.
|
||||
|
||||
## Proposed domain rules (require approval)
|
||||
|
||||
1. A physical managed file has one durable catalog identity and belongs to at most
|
||||
one logical model. Unassigned files retain their file record. Shared ownership is
|
||||
outside MVP; separate models cannot silently adopt the same managed path.
|
||||
2. Creating a model is explicit, not an `ensureModel(folder, name)` lookup. Equal
|
||||
titles do not imply equal identity. Empty draft models are allowed so creation
|
||||
and attachment can be separate user actions.
|
||||
3. Assign/reassign/detach is a metadata operation: no copying, renaming or deletion.
|
||||
A model's files may occupy different physical folders; future move operations
|
||||
must preview the actual file set, not assume a single directory. The model retains
|
||||
a required home folder for organization; this does not relocate its files.
|
||||
4. Future import preview defaults to one STL per model. Multipart grouping and
|
||||
supplementary associations must be explicit and confirmed; no automatic grouping
|
||||
based on folder names. This slice supplies the contract, not the preview UI.
|
||||
5. Model removal will detach file records before deleting model-owned metadata.
|
||||
Files and uploaded-media bytes stay on disk. Physical deletion remains outside MVP.
|
||||
|
||||
## Architecture and builder choices
|
||||
|
||||
Keep PHP, PDO, Phinx, Slim and existing repository conventions. Extend
|
||||
`CatalogRepositoryInterface` / `PdoCatalogRepository` rather than introducing a
|
||||
parallel catalog. Add a catalog service boundary for validation and transactions.
|
||||
Its transaction helper must coexist with an outer importer transaction on the same
|
||||
PDO connection without accidentally committing or rolling it back.
|
||||
|
||||
Proposed operations: create model; read aggregate; assign a selected set of existing
|
||||
file IDs; reassign selected files from an expected current owner; detach selected
|
||||
files. Validation occurs before writes; all changes in a multi-file command commit
|
||||
or roll back together. Missing targets, duplicate IDs and stale expected ownership
|
||||
produce explicit domain failures. Lock selected file rows in deterministic ID order
|
||||
on PostgreSQL and recheck ownership under lock. Concurrent reassignment must not
|
||||
silently overwrite another user's decision. SQLite remains the fast default test
|
||||
backend, not proof of PostgreSQL locking.
|
||||
|
||||
Add new migrations, never rewrite applied migrations. Make file ownership nullable
|
||||
and enforce unique managed relative paths across file rows, not just per model.
|
||||
Migration must detect conflicting existing paths and stop with actionable guidance;
|
||||
never merge or delete existing records automatically. Existing rows and IDs survive.
|
||||
Removal service semantics detach first; avoid a model-delete cascade that erases
|
||||
unassigned-file identity. Decide the exact FK action in the implementation plan.
|
||||
|
||||
Remove the current unique model `(folder_id, name)` constraint to permit independent
|
||||
same-title models. Adapt legacy `ensureModel()` so zero matches creates, one match
|
||||
reuses, and multiple matches fail explicitly; never choose an arbitrary model.
|
||||
Its existing ON CONFLICT clause must change with the constraint. Keep this inferred
|
||||
grouping entry point only for importer compatibility, documented as incomplete.
|
||||
Do not claim #20 complete until the importer actually uses a confirmed manifest.
|
||||
|
||||
Builder choices: reuse `PdoRepository` savepoints for nested transactions; require
|
||||
expected ownership for every association command (explicit null means unassigned,
|
||||
not no precondition); preserve file timestamps on association changes. Use RESTRICT
|
||||
on model deletion and explicit detach. Validate canonical managed relative paths
|
||||
with existing path-safety conventions before writes. Migration preflight must also
|
||||
reject noncanonical path aliases rather than silently repair them. Downgrade must
|
||||
fail safely if unassigned files or duplicate same-folder titles prevent restoration
|
||||
of old constraints; no automatic reassignment, renaming or deletion.
|
||||
|
||||
## Acceptance tests
|
||||
|
||||
- Aggregate with two STL parts, selected image and document, with stable identities.
|
||||
- Reassignment/detach changes associations only; files remain byte-for-byte intact.
|
||||
- Invalid target, missing file, stale expected owner or FK failure rolls back all
|
||||
changes; outer transactions remain owned by their caller.
|
||||
- Two PostgreSQL sessions contest the same file; one cannot silently overwrite the
|
||||
other's ownership decision. Test deterministic lock ordering and explicit
|
||||
conflict/deadlock failure; automatic retries are outside this slice.
|
||||
- Migration preserves IDs/data; duplicate managed paths fail without destructive
|
||||
repair. Test forward migration and rollback constraints explicitly.
|
||||
- Default SQLite suite and isolated PostgreSQL suite pass; existing importer tests
|
||||
remain green while its workflow limitations stay documented.
|
||||
|
||||
## Follow-on order
|
||||
|
||||
After this contract, implement confirmed immutable import manifests (#18/#20) and
|
||||
publication recovery (#24/#25), then progress/retry (#26/#27). Catalog CRUD and
|
||||
CSRF/concurrent-edit handling (#29), database-only removal (#30), safe download (#33),
|
||||
and serialized folder/model moves (#31/#32) follow as separately designed slices.
|
||||
|
||||
## Approval boundary
|
||||
|
||||
The ownership/unassigned-file/empty-model rules above are proposals, not existing
|
||||
user decisions. Approve or revise them before creating the implementation plan.
|
||||
Reference in new issue
Block a user