submitting the same digest twice is a no-op (#14)
Browse files- submitting the same digest twice is a no-op (7c28fb0c8619dcc32d19a13aa817b948e1ea0dbf)
- submit_test.py +68 -0
- tests/test_submit_test.py +135 -0
submit_test.py
CHANGED
|
@@ -147,6 +147,55 @@ def _split_image_ref(ref: str) -> tuple[str, str, str]:
|
|
| 147 |
return ref, "", ""
|
| 148 |
|
| 149 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 150 |
def count_test_submissions(
|
| 151 |
api: HfApi, hf_user: str, track: str, division: str
|
| 152 |
) -> int:
|
|
@@ -280,6 +329,25 @@ def validate_and_submit(
|
|
| 280 |
|
| 281 |
api = HfApi(token=token)
|
| 282 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 283 |
held = count_test_submissions(api, hf_user, track, division)
|
| 284 |
if held >= config.MAX_TEST_SUBMISSIONS_PER_TRACK:
|
| 285 |
return False, (
|
|
|
|
| 147 |
return ref, "", ""
|
| 148 |
|
| 149 |
|
| 150 |
+
def find_existing_submission(
|
| 151 |
+
api: HfApi, hf_user: str, track: str, division: str, image_digest: str
|
| 152 |
+
) -> dict | None:
|
| 153 |
+
"""This team's existing submission of this exact image, if there is one.
|
| 154 |
+
|
| 155 |
+
Makes submitting idempotent. The same digest is the same image by
|
| 156 |
+
definition, so registering it twice cannot produce a different score; it
|
| 157 |
+
would only spend a second slot out of three. A double-click or a re-run of
|
| 158 |
+
the instructions did exactly that in the rehearsal, producing two identical
|
| 159 |
+
PENDING rows.
|
| 160 |
+
|
| 161 |
+
Keyed on the digest, not the tag or the model name, because the digest is
|
| 162 |
+
what is actually scored. Same reasoning as everywhere else in the pipeline.
|
| 163 |
+
|
| 164 |
+
Returns the request record, or None. An unreadable request is skipped: the
|
| 165 |
+
lenient direction, since the cost of missing one is a duplicate rather than
|
| 166 |
+
a refusal.
|
| 167 |
+
"""
|
| 168 |
+
digest = (image_digest or "").strip()
|
| 169 |
+
if not digest:
|
| 170 |
+
return None
|
| 171 |
+
prefix = f"requests/{config.TEST_PHASE}/{track}/{division}/"
|
| 172 |
+
try:
|
| 173 |
+
files = api.list_repo_files(config.REQUESTS_REPO, repo_type="dataset")
|
| 174 |
+
except Exception:
|
| 175 |
+
return None
|
| 176 |
+
for f in files:
|
| 177 |
+
if not (f.startswith(prefix) and f.endswith("/request.json")):
|
| 178 |
+
continue
|
| 179 |
+
try:
|
| 180 |
+
local = hf_hub_download(
|
| 181 |
+
config.REQUESTS_REPO, f, repo_type="dataset", token=api.token
|
| 182 |
+
)
|
| 183 |
+
with open(local, encoding="utf-8") as fh:
|
| 184 |
+
obj = json.load(fh)
|
| 185 |
+
except Exception:
|
| 186 |
+
continue
|
| 187 |
+
if obj.get("hf_user") != hf_user:
|
| 188 |
+
continue
|
| 189 |
+
# The digest is NESTED under "image", which is the contract with the
|
| 190 |
+
# worker's parse_request(). A flat obj["image_digest"] reads None on
|
| 191 |
+
# every record, so the comparison would never match and this whole
|
| 192 |
+
# function would silently do nothing.
|
| 193 |
+
recorded = ((obj.get("image") or {}).get("digest") or "").strip()
|
| 194 |
+
if recorded == digest:
|
| 195 |
+
return obj
|
| 196 |
+
return None
|
| 197 |
+
|
| 198 |
+
|
| 199 |
def count_test_submissions(
|
| 200 |
api: HfApi, hf_user: str, track: str, division: str
|
| 201 |
) -> int:
|
|
|
|
| 329 |
|
| 330 |
api = HfApi(token=token)
|
| 331 |
|
| 332 |
+
# Idempotent: the same digest is the same image, so registering it twice
|
| 333 |
+
# cannot produce a different score. Checked BEFORE the cap, because a
|
| 334 |
+
# re-submission is not a new submission and a team already at the limit
|
| 335 |
+
# must not be told it has run out for something it already sent.
|
| 336 |
+
existing = find_existing_submission(api, hf_user, track, division, image_digest)
|
| 337 |
+
if existing is not None:
|
| 338 |
+
status = existing.get("status", "PENDING")
|
| 339 |
+
when = (existing.get("created_at") or "")[:19].replace("T", " ")
|
| 340 |
+
return True, (
|
| 341 |
+
f"✅ Already submitted. This exact image is registered for "
|
| 342 |
+
f"{config.TRACK_LABELS[track]} / {division} as "
|
| 343 |
+
f"`{existing.get('submission_id', '')}`"
|
| 344 |
+
+ (f", submitted {when} UTC" if when else "")
|
| 345 |
+
+ f", currently **{status}**. Nothing was registered again and no "
|
| 346 |
+
"submission was spent, because the digest is the same image. Track "
|
| 347 |
+
"it in **My Submissions**; push a new image and submit that digest "
|
| 348 |
+
"if you want a different one scored."
|
| 349 |
+
)
|
| 350 |
+
|
| 351 |
held = count_test_submissions(api, hf_user, track, division)
|
| 352 |
if held >= config.MAX_TEST_SUBMISSIONS_PER_TRACK:
|
| 353 |
return False, (
|
tests/test_submit_test.py
CHANGED
|
@@ -196,6 +196,141 @@ class ValidateOnlyTest(unittest.TestCase):
|
|
| 196 |
self.assertFalse(self._call(image_ref="docker.io/x/y")[0])
|
| 197 |
|
| 198 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 199 |
class CountTestSubmissionsTest(unittest.TestCase):
|
| 200 |
"""Which request states hold a slot, and whose."""
|
| 201 |
|
|
|
|
| 196 |
self.assertFalse(self._call(image_ref="docker.io/x/y")[0])
|
| 197 |
|
| 198 |
|
| 199 |
+
class SubmitIsIdempotentTest(unittest.TestCase):
|
| 200 |
+
"""The end-to-end property, across the seam that matters.
|
| 201 |
+
|
| 202 |
+
Testing the lookup alone would not have caught the real bug: the lookup read
|
| 203 |
+
a flat field while the writer nests it. This drives validate_and_submit and
|
| 204 |
+
asserts nothing is written.
|
| 205 |
+
"""
|
| 206 |
+
|
| 207 |
+
def setUp(self):
|
| 208 |
+
self._tmp = tempfile.TemporaryDirectory()
|
| 209 |
+
self.addCleanup(self._tmp.cleanup)
|
| 210 |
+
p = mock.patch.object(config, "TEST_PHASE_OPEN", True); p.start()
|
| 211 |
+
self.addCleanup(p.stop)
|
| 212 |
+
self.records = {}
|
| 213 |
+
d = mock.patch.object(st, "hf_hub_download", self._download); d.start()
|
| 214 |
+
self.addCleanup(d.stop)
|
| 215 |
+
|
| 216 |
+
def _download(self, repo_id, path, repo_type=None, token=None):
|
| 217 |
+
local = os.path.join(self._tmp.name, path.replace("/", "_"))
|
| 218 |
+
with open(local, "w", encoding="utf-8") as fh:
|
| 219 |
+
json.dump(self.records[path], fh)
|
| 220 |
+
return local
|
| 221 |
+
|
| 222 |
+
def _submit(self, api, digest=DIGEST):
|
| 223 |
+
with mock.patch.object(st, "HfApi", return_value=api):
|
| 224 |
+
return st.validate_and_submit(
|
| 225 |
+
hf_user="team_a", track="convqa", division="large",
|
| 226 |
+
team_name="Team A", model_name="m", license_str="mit",
|
| 227 |
+
open_weight=True, total_params=8_000_000_000,
|
| 228 |
+
active_params=8_000_000_000, image_ref=REF,
|
| 229 |
+
image_digest=digest, token="t",
|
| 230 |
+
)
|
| 231 |
+
|
| 232 |
+
def _existing(self, sid, digest, status="PENDING"):
|
| 233 |
+
path = f"requests/test/convqa/large/{sid}/request.json"
|
| 234 |
+
self.records[path] = {
|
| 235 |
+
"submission_id": sid, "hf_user": "team_a", "status": status,
|
| 236 |
+
"created_at": "2026-08-08T19:56:00Z",
|
| 237 |
+
"image": {"ref": REF, "digest": digest},
|
| 238 |
+
}
|
| 239 |
+
return path
|
| 240 |
+
|
| 241 |
+
def test_resubmitting_the_same_image_writes_nothing(self):
|
| 242 |
+
self._existing("s1", DIGEST)
|
| 243 |
+
api = FakeApi(list(self.records))
|
| 244 |
+
ok, msg = self._submit(api)
|
| 245 |
+
self.assertTrue(ok, msg)
|
| 246 |
+
self.assertEqual([], api.uploads, "a duplicate must not be written")
|
| 247 |
+
self.assertIn("Already submitted", msg)
|
| 248 |
+
self.assertIn("s1", msg)
|
| 249 |
+
|
| 250 |
+
def test_a_team_at_the_cap_can_still_resubmit_what_it_already_sent(self):
|
| 251 |
+
# The idempotency check must run BEFORE the cap, or a team at 3/3 is
|
| 252 |
+
# told it is out of submissions for an image it already sent.
|
| 253 |
+
for i in range(config.MAX_TEST_SUBMISSIONS_PER_TRACK):
|
| 254 |
+
self._existing(f"s{i}", f"sha256:{i}" + "a" * 63)
|
| 255 |
+
self._existing("mine", DIGEST)
|
| 256 |
+
api = FakeApi(list(self.records))
|
| 257 |
+
ok, msg = self._submit(api)
|
| 258 |
+
self.assertTrue(ok, msg)
|
| 259 |
+
self.assertIn("Already submitted", msg)
|
| 260 |
+
self.assertNotIn("limit reached", msg)
|
| 261 |
+
self.assertEqual([], api.uploads)
|
| 262 |
+
|
| 263 |
+
def test_a_genuinely_new_image_is_still_written(self):
|
| 264 |
+
# The other half: idempotency must not swallow a real submission.
|
| 265 |
+
self._existing("s1", DIGEST)
|
| 266 |
+
api = FakeApi(list(self.records))
|
| 267 |
+
ok, msg = self._submit(api, digest="sha256:" + "c" * 64)
|
| 268 |
+
self.assertTrue(ok, msg)
|
| 269 |
+
self.assertTrue(api.uploads, "a new digest must still be registered")
|
| 270 |
+
|
| 271 |
+
|
| 272 |
+
class FindExistingSubmissionTest(unittest.TestCase):
|
| 273 |
+
"""Submitting the same digest twice must not spend a second slot.
|
| 274 |
+
|
| 275 |
+
The digest is the image, so registering it again cannot change the score.
|
| 276 |
+
Two identical PENDING rows appeared in the rehearsal from one repeated
|
| 277 |
+
submit, each holding a slot out of three.
|
| 278 |
+
"""
|
| 279 |
+
|
| 280 |
+
def setUp(self):
|
| 281 |
+
self._tmp = tempfile.TemporaryDirectory()
|
| 282 |
+
self.addCleanup(self._tmp.cleanup)
|
| 283 |
+
self.records = {}
|
| 284 |
+
patch = mock.patch.object(st, "hf_hub_download", self._download)
|
| 285 |
+
patch.start(); self.addCleanup(patch.stop)
|
| 286 |
+
|
| 287 |
+
def _download(self, repo_id, path, repo_type=None, token=None):
|
| 288 |
+
local = os.path.join(self._tmp.name, path.replace("/", "_"))
|
| 289 |
+
with open(local, "w", encoding="utf-8") as fh:
|
| 290 |
+
json.dump(self.records[path], fh)
|
| 291 |
+
return local
|
| 292 |
+
|
| 293 |
+
def _add(self, sid, digest, hf_user="team_a", track="convqa", division="large"):
|
| 294 |
+
path = f"requests/test/{track}/{division}/{sid}/request.json"
|
| 295 |
+
# Shaped like the real record: the digest is NESTED under "image".
|
| 296 |
+
self.records[path] = {
|
| 297 |
+
"submission_id": sid, "hf_user": hf_user, "status": "PENDING",
|
| 298 |
+
"created_at": "2026-08-08T19:56:00Z",
|
| 299 |
+
"image": {"ref": REF, "digest": digest},
|
| 300 |
+
}
|
| 301 |
+
|
| 302 |
+
def _find(self, digest, hf_user="team_a"):
|
| 303 |
+
return st.find_existing_submission(
|
| 304 |
+
FakeApi(list(self.records)), hf_user, "convqa", "large", digest
|
| 305 |
+
)
|
| 306 |
+
|
| 307 |
+
def test_the_same_digest_is_found(self):
|
| 308 |
+
self._add("s1", DIGEST)
|
| 309 |
+
found = self._find(DIGEST)
|
| 310 |
+
self.assertIsNotNone(found, "an identical digest must be recognised")
|
| 311 |
+
self.assertEqual("s1", found["submission_id"])
|
| 312 |
+
|
| 313 |
+
def test_it_reads_the_nested_digest_not_a_flat_field(self):
|
| 314 |
+
# The record stores image.digest. A flat obj["image_digest"] reads None
|
| 315 |
+
# on every record, so the lookup would match nothing and silently do
|
| 316 |
+
# nothing. That bug was written and caught here.
|
| 317 |
+
self._add("s1", DIGEST)
|
| 318 |
+
self.assertIn("image", self.records[list(self.records)[0]])
|
| 319 |
+
self.assertIsNotNone(self._find(DIGEST))
|
| 320 |
+
|
| 321 |
+
def test_a_different_digest_is_not_a_duplicate(self):
|
| 322 |
+
self._add("s1", DIGEST)
|
| 323 |
+
self.assertIsNone(self._find("sha256:" + "b" * 64))
|
| 324 |
+
|
| 325 |
+
def test_another_teams_identical_image_is_not_mine(self):
|
| 326 |
+
self._add("s1", DIGEST, hf_user="someone_else")
|
| 327 |
+
self.assertIsNone(self._find(DIGEST))
|
| 328 |
+
|
| 329 |
+
def test_an_empty_digest_matches_nothing(self):
|
| 330 |
+
self._add("s1", DIGEST)
|
| 331 |
+
self.assertIsNone(self._find(""))
|
| 332 |
+
|
| 333 |
+
|
| 334 |
class CountTestSubmissionsTest(unittest.TestCase):
|
| 335 |
"""Which request states hold a slot, and whose."""
|
| 336 |
|