Skip to content

fix: publish scratch artifacts to content-addressed storage - #574

Open
harshitethic wants to merge 3 commits into
Hebbian-Robotics:mainfrom
harshitethic:fix/durable-enrichment-artifacts
Open

harshitethic wants to merge 3 commits into
Hebbian-Robotics:mainfrom
harshitethic:fix/durable-enrichment-artifacts

Conversation

@harshitethic

Copy link
Copy Markdown
Contributor

Fixes #545.

Summary

  • detect enrichment artifacts that still live under the run scratch directory;
  • publish those artifacts under durable artifacts/ storage instead of exposing scratch URIs;
  • include the artifact SHA-256 in the published key so changed bytes get a different durable URI;
  • tighten camera-video regression coverage to reject scratch-bound URIs.

Why

Scratch is rewritten across reprocessing. A catalog URI that points into scratch can therefore start resolving to different bytes or disappear entirely. Content-addressed durable keys keep prior catalog URIs bound to the bytes that were published.

Testing

  • updated camera-video path assertions to require durable, content-addressed artifact paths;
  • full repository CI can exercise the integration suite.

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix itself is right, and both halves hold under mutation: forcing artifact_is_scratch_bound = False reddens 1, and dropping artifact_content_digest from the key reddens 1. publish does copy when source and destination differ, so the artifact really does leave scratch rather than just getting a nicer-looking URI.

Two things before this can land.

uv run ruff format --check fails on src/hflow/app.py, so this would turn main red:

-   artifact_name_digest = hashlib.sha256(artifact_name.encode()).hexdigest()[:8]
+   artifact_name_digest = hashlib.sha256(artifact_name.encode()).hexdigest()[
+       :8
+   ]

And step_directory and artifact_name_digest are now computed twice, identically, in the two branches. Only the key layout differs between them. Hoist those two out above the if so the next person changing the naming scheme cannot change one copy and miss the other.

Worth a sentence in the code on why the non-scratch fallback stays non-content-addressed, since a reader hitting the second branch will wonder whether that is deliberate.

@greptile-apps

greptile-apps Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Changes how artifacts are stored and addressed in the system.

The PR appears safe to merge based on the reviewed changes.

Findings

  1. P1 Module Is Truncated ▶
Summary

The PR publishes scratch-bound enrichment artifacts to content-addressed durable storage and tightens camera-video path assertions. The latest change shares artifact-key calculations across publication branches without changing their keys.

Reviews (3) · Last reviewed commit: "Address artifact publishing review feedb..."

Comment thread src/hflow/app.py
# must bump the explicit constant above.
version=media_contact_sheet_step_version(),
)
report.enrichments.append(
_execute_enrichment(media_step, canonical_episode, quarantine_skip)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Module Is Truncated

This replacement reduces hflow.app to an indented 73-line fragment that begins without an enclosing block, so importing it raises IndentationError. The CLI imports this module during startup, and public exports such as App are loaded through it, so the package, CLI, and test suite can no longer start.

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/hflow/app.py was truncated by the last push. On c28600a it is 72 lines and does not parse:

IndentationError: unexpected indent
  line 1: _execute_enrichment(media_step, canonical_episode, quarantine_skip)

The file starts mid-function, so the module docstring, every import, and roughly 3,000 lines above that point are gone. That is what the 7 ruff errors and 81 ty diagnostics are; nothing here is about your actual change.

Sizes across the branch, so you can see where it went:

8d6a134  3158 lines
064f69d  3158 lines
c28600a    72 lines   <- the review-feedback commit
main     3131 lines

064f69d is intact, so git checkout 064f69d -- src/hflow/app.py gets the file back, then re-apply the two review points on top: hoist step_directory and artifact_name_digest above the if, and the line-length fix ruff format wants.

The change itself was already right last round. Both halves held under mutation and publish does copy when source and destination differ, so nothing about the approach needs revisiting.

Main has moved since you branched, so merge it in as well once the file is back.

@harshitethic
harshitethic force-pushed the fix/durable-enrichment-artifacts branch from c28600a to 064f69d Compare September 22, 2026 14:00

This branch has not been deployed

No deployments
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.

[Bug]: camera_video publishes scratch-bound artifact URIs as durable, causing silent byte-replacement or hard deletion on sync rewrite

2 participants