Skip to content

Spec cleanup: remove unimplemented artifact dedup semantics from External Results v1 - #323

Merged
bg-playground merged 2 commits into
mainfrom
copilot/remove-sha-256-deduplication-content
May 8, 2026
Merged

Spec cleanup: remove unimplemented artifact dedup semantics from External Results v1#323
bg-playground merged 2 commits into
mainfrom
copilot/remove-sha-256-deduplication-content

Conversation

Copilot AI commented May 8, 2026

Copy link
Copy Markdown
Contributor

This PR aligns the External Results v1 spec with current implementation by removing unimplemented SHA-256 artifact dedup behavior and related error-code claims. It keeps idempotency guidance explicit and intentional for v0.1.

  • Scope

    • Docs-only change in docs/specs/external_results_v1.md (no code/test/migration changes).
  • Idempotency rules (§d)

    • Removed the artifact subsection describing SHA-256 deduplication and “return existing artifact” behavior.
    • Kept existing idempotency rules for:
      • case-result dedup by external_id
      • requirement-link insertion no-op
    • Added explicit rationale line that artifact uploads are not deduplicated in v0.1 and retries may create duplicate rows.
  • Error model (§f)

    • Removed artifact.duplicate from the error-code registry.
  • Upload artifact endpoint (§c.7)

    • Verified error-code table does not include 409 artifact.duplicate (no change required there).
Artifact uploads are not deduplicated; reporters that retry an upload (for example on transient network errors) may create duplicate artifact rows, and this is acceptable for v0.1.
Original prompt

Goal

Resolve issue #321 by selecting option B — remove the SHA-256 artifact deduplication content from the spec, since it is not implemented and we are not implementing it now. Keeps the spec honest with current behavior.

Closes: #321

Scope — docs/specs/external_results_v1.md only

This is a documentation-only PR. No code changes, no tests, no migrations.

1. Remove the artifact-dedup paragraph from § d

§ d ("Idempotency rules") currently contains an "Artifacts — deduplication by SHA-256" subsection (around lines 429–431) that says BGSTM dedupes artifacts by SHA-256 hash on the same case_result_id and returns the existing row. This is not implemented in the artifact endpoint shipped in #319.

Remove that subsection entirely. Keep the surrounding § d content (case-result dedup by external_id, requirement-link no-op insertion) intact.

2. Remove artifact.duplicate from § f's error-code registry

§ f ("Error model" → "Error code registry") lists artifact.duplicate as a valid response code. Remove that row from the table — there is no code path that returns it.

3. Cross-check § c.7 ("Upload artifact") error-codes table

Verify the § c.7 error-codes table does not include a 409 artifact.duplicate row. If it does, remove it. (#319 already removed the older 409 row, but double-check during this edit.)

4. Add a one-line rationale comment

In the same § d edit, leave a single explanatory line so future readers understand why dedup isn't there:

Artifact uploads are not deduplicated. Reporters that retry an upload (e.g. on transient network errors) will create duplicate artifact rows; this is acceptable for v0.1.

Phrase it however reads best — the goal is just to make the absence intentional rather than an oversight.

Out of scope

Acceptance criteria

  • § d artifact-dedup-by-SHA-256 subsection is removed.
  • § d retains the case-result external_id dedup and requirement-link no-op subsections.
  • § f error registry no longer lists artifact.duplicate.
  • § c.7 error-codes table verified not to list 409 artifact.duplicate.
  • One-line rationale added in § d explaining that artifact uploads are not deduplicated.
  • No code, test, or migration files modified.
  • Markdown still renders cleanly (no orphan headings, no broken anchors, no leftover references to removed sections).

The following is the prior conversation context from the user's chat exploration (may be truncated):

Assistant: 🎉 Nice. Onward to closing out PR #314.

Recap of what #314 needs now that #319 is on main

Item Why
Rebase onto main Pick up the artifact endpoint, the case-result endpoints from #317, and any other drift since #314 was opened.
Revert the relaxed assertions in scripts/smoke/assert.py The "passed case has requirement link to REQ-CRM-HOMEPAGE" assertion was softened pending #316. Note: #316 isn't merged yet, so this one stays relaxed — but with a comment pointing to #316. Other relaxations should be tightened.
Add artifact assertions Now that the artifact endpoint exists, the smoke can verify upload → audit-log entry with all five details fields (case_result_id, kind, size_bytes, filename, content_type). This was the whole reason the audit-log shape was load-bearing.
Bump the frameworks pin Currently pinned to a SHA that predates the response.id fix. Bump to whatever SHA includes that fix (and reads bgstm:requirement annotations once #316 lands — but for now just the response.id fix).
Verify Alembic migration order external_case_artifacts migration (l1m2n3o4p5q6) needs to be in the smoke bootstrap's migration sequence. Should be automatic via alembic upgrade head but worth a sanity check.

Two questions before I dispatch / draft anything

  1. Frameworks-side response.id fix PR — was that ever dispatched and merged? The earlier log showed it was queued but I don't have confirmation. If it's not merged yet, the pin bump on Add BGSTM external-results smoke workflow pinned to reporter SHA ab5d7c1 with main-branch, audit-log, and artifact-path compatibility hardening #314 has nothing new to point at and we should dispatch that first.

  2. [v0.2] BGSTMReporter should transmit bgstm:requirement annotations and BGSTM should resolve external→UUID for case-result linking #316 status — still open, right? (Reporter bgstm:requirement annotations + BGSTM requirement_external_ids resolution.) If yes, the requirement-link assertion stays relaxed with a # TODO(#316) comment; if it's quietly merged, we tighten it.

Suggested sequence (assuming both above are "still open / not merged")

  1. Pull current state of Add BGSTM external-results smoke workflow pinned to reporter SHA ab5d7c1 with main-branch, audit-log, and artifact-path compatibility hardening #314 — see what's on the branch, what's relaxed, where the frameworks pin is.
  2. Draft a comment on Add BGSTM external-results smoke workflow pinned to reporter SHA ab5d7c1 with main-branch, audit-log, and artifact-path compatibility hardening #314 with a tight checklist for the agent's next round (rebase + artifact assertions + targeted re-tightening, leaving #316-blocked assertions alone).
  3. Then watch for the...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/ae571118-9d38-4718-bf48-9824c50dd222

Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove SHA-256 artifact deduplication content from spec Spec cleanup: remove unimplemented artifact dedup semantics from External Results v1 May 8, 2026
Copilot AI requested a review from bg-playground May 8, 2026 18:53
@bg-playground
bg-playground marked this pull request as ready for review May 8, 2026 19:58
@bg-playground

Copy link
Copy Markdown
Owner

LGTM ✅ — exactly the targeted three-spot edit requested.

Acceptance item Status
§ d SHA-256 dedup subsection removed
§ d retains case-result external_id dedup + requirement-link no-op
§ f error registry no longer lists artifact.duplicate
§ c.7 error-codes table verified clean (no 409 artifact.duplicate)
One-line rationale added explaining absence is intentional
Docs-only — no code/test/migration changes
Markdown clean

The replacement wording is right — calls out that retries can produce duplicates and explicitly bounds the decision to v0.1, so a future reader knows it was deliberate, not an oversight. Closes #321. 🚀

@bg-playground
bg-playground merged commit 01a22fc into main May 8, 2026
7 checks passed
@bg-playground
bg-playground deleted the copilot/remove-sha-256-deduplication-content branch May 8, 2026 20:08
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.

Artifact dedup by SHA-256 is spec'd but not implemented (spec/impl drift in §d)

2 participants