Share duplicate attempt response

[catcherintheroad-hub]

Bounty #846

Summary

• Extract the duplicated duplicate_active_attempt 409 JSON response into a private helper in app/bounty_attempts.py.
• Reuse that helper for both the normal active-attempt duplicate path and the IntegrityError fallback path.
• Preserve the existing response shape, warning calculation, and attempt serialization behavior.

Why this fits #846

This is a focused maintainability cleanup: the route currently builds the same duplicate-attempt response in two places. Sharing the response construction keeps the two duplicate paths in sync without changing the public API contract.

Duplicate / scope check

• MRWK bounty: 100 MRWK - code cleanup and maintainability improvements #846 public bounty row is open with effective award capacity.
• gh pr list --repo ramimbo/mergework --state open --search "duplicate_active_attempt OR active attempt duplicate OR IntegrityError bounty_attempts" did not show a same-scope open PR.
• gh search prs "duplicate_active_attempt bounty_attempts repo:ramimbo/mergework" --state open returned no same-scope PR.
• This does not duplicate Share bounty attempt control character validation #912, which is about source URL control-character validation, or Accept id alias for MCP attempt lookup #916, which is MCP attempt selector usability.

Validation

• uv run --python 3.12 --extra dev python -m pytest tests/test_bounty_attempts.py::test_bounty_attempts_register_list_duplicate_and_release -q
• uv run --python 3.12 --extra dev python -m pytest tests/test_bounty_attempts.py tests/test_bounty_api_routes.py -q
• uv run --python 3.12 --extra dev ruff check app/bounty_attempts.py
• uv run --python 3.12 --extra dev ruff format --check app/bounty_attempts.py
• uv run --python 3.12 --extra dev mypy app/bounty_attempts.py
• uv run --python 3.12 --extra dev python scripts/docs_smoke.py
• git diff --check origin/main...HEAD
• git merge-tree --write-tree origin/main HEAD -> clean tree abda6f4c7b8cc6d19c1ef4bc0fd2711b121cfce1

Scope

No public API contract, attempt lifecycle state, bounty availability logic, ledger behavior, wallet behavior, treasury behavior, payout behavior, proposal execution, admin-token behavior, private data, credentials, secrets, exchange, bridge, cash-out, or MRWK price behavior changed.

Summary by CodeRabbit

• Refactor
  • Improved internal code organization for bounty attempt error handling. No user-facing changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0b1d2b77-8cad-41ce-bac7-a4c2d2b29532

📥 Commits

Reviewing files that changed from the base of the PR and between d4d0e48 and 05f3a15.

📒 Files selected for processing (1)

• app/bounty_attempts.py

---

📝 Walkthrough

Walkthrough

This PR refactors app/bounty_attempts.py to eliminate duplicate HTTP 409 response construction for "duplicate active bounty attempt" scenarios. A new helper function _duplicate_active_attempt_response() is introduced and called from two separate code paths in api_create_bounty_attempt that previously contained identical response-building logic.

Changes

Duplicate active attempt response centralization

Layer / File(s)	Summary
Duplicate response helper function app/bounty_attempts.py	New _duplicate_active_attempt_response() helper accepts a session, bounty, attempt, and timestamp, then returns a 409 JSONResponse with status, attempt details, and computed warnings.
Update duplicate attempt detection call sites app/bounty_attempts.py	Both the "existing active attempt" branch and the IntegrityError handler in api_create_bounty_attempt now call the new helper instead of constructing identical 409 responses inline.

Possibly related issues

• #677: Active-attempt handling and response logic are related; this change centralizes the 409 duplicate-response construction that issue #677 also touches.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Bounty Pr Focus	⚠️ Warning	Commit includes all 163 repository files, but summary claims only +18/-16 changes to app/bounty_attempts.py. Violates scope requirement by including entire repo instead of focused bounty changes.	Rebase to include only changes to app/bounty_attempts.py and related test files (tests/test_bounty_attempts.py, tests/test_bounty_api_routes.py), removing repository-wide config and initialization files.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title "Share duplicate attempt response" directly names the changed surface and accurately reflects the main refactoring: extracting and sharing the duplicate attempt response construction.
Description check	✅ Passed	The description includes all required template sections: Summary, Evidence (with bounty/issue reference, scope checks, and affected surfaces), comprehensive validation steps, and detailed scope statement.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Mergework Public Artifact Hygiene	✅ Passed	PR contains no investment claims, price claims, cash-out claims, payout claims, or security details. MRWK mentioned appropriately as unaffected by changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[catcherintheroad-hub]

Follow-up on the CodeRabbit pre-merge warning: I rechecked the PR file list and local diff after the review completed.\n\n- GitHub PR file list: only app/bounty_attempts.py, +18/-16.\n- Commit count: one commit, 05f3a15.\n- Local git diff --name-status origin/main...HEAD: only M app/bounty_attempts.py.\n- uv.lock is only an untracked local file in my checkout and is not part of this PR.\n\nSo the warning that the commit includes all repository files appears to be a pre-merge-check false positive. The submitted scope remains the single-file duplicate response cleanup.

[Zejia]

Reviewed current head 05f3a15c16844db4bf56fae1e9c73c8137f7f3a2 as a non-author.

Scope checked:

• app/bounty_attempts.py: verified the duplicated active-attempt 409 response is extracted into _duplicate_active_attempt_response() and reused by both the normal duplicate path and the IntegrityError fallback path.
• Confirmed the public response shape is preserved: status remains duplicate_active_attempt, the serialized attempt still uses bounty_attempt_to_dict(existing, now), and warnings are still computed with bounty_attempt_warnings(session, bounty, now).
• Checked GitHub state before review: PR open, mergeStateStatus=CLEAN, mergeable=MERGEABLE, hosted Quality/readiness/docs/image check successful, CodeRabbit reported no actionable comments, and no prior human reviews or #838 claims for this head were visible.

Validation run from a clean PR checkout:

• /Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/python -m pytest tests/test_bounty_attempts.py -q -> 12 passed.
• /Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/ruff check app/bounty_attempts.py -> passed.
• /Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/ruff format --check app/bounty_attempts.py -> 1 file already formatted.
• /Users/jimmy/Documents/Playground/bounty-work/mergework/.venv/bin/mypy app/bounty_attempts.py -> success.
• git diff --check origin/main...HEAD -> clean.
• git merge-tree --write-tree origin/main HEAD -> clean tree df4cf11b657d6ce9077bd0897ba0869f39bae189.

No blocker found in this focused maintainability cleanup.

[aunysillyme]

Reviewed current head 05f3a15c16844db4bf56fae1e9c73c8137f7f3a2.

Refs #838 review bounty.

What I checked:

• app/bounty_attempts.py: _duplicate_active_attempt_response() preserves the existing 409 JSON shape for duplicate active attempts, including status, serialized attempt, and current warnings.
• The normal duplicate path still returns the existing active attempt before creating a new row.
• The IntegrityError fallback still rolls back, reloads the bounty/attempt state, raises the previous hard 409 only when no matching active attempt exists, and otherwise returns the same duplicate response helper.
• Scope stays limited to response construction reuse; it does not change attempt lifecycle semantics, warnings calculation, bounty availability, ledger, wallet, treasury, payout, admin-token, private data, exchange, bridge, cash-out, or MRWK price behavior.

Validation I ran on this exact head:

• uv run --python 3.12 --extra dev python -m pytest tests/test_bounty_attempts.py::test_bounty_attempts_register_list_duplicate_and_release -q -> 1 passed, 1 existing Starlette/httpx warning.
• uv run --python 3.12 --extra dev python -m pytest tests/test_bounty_attempts.py tests/test_bounty_api_routes.py -q -> 32 passed, 1 existing Starlette/httpx warning.
• uv run --python 3.12 --extra dev ruff check app/bounty_attempts.py -> passed.
• uv run --python 3.12 --extra dev ruff format --check app/bounty_attempts.py -> 1 file already formatted.
• uv run --python 3.12 --extra dev mypy app/bounty_attempts.py -> success.
• uv run --python 3.12 --extra dev python scripts/docs_smoke.py -> docs smoke ok.
• git diff --check origin/main...HEAD -> clean.
• git merge-tree --write-tree origin/main HEAD -> clean tree df4cf11b657d6ce9077bd0897ba0869f39bae189.

GitHub state checked before review: PR open, mergeable=MERGEABLE, hosted Quality, readiness, docs, and image checks successful, CodeRabbit successful, and no human reviews on current head visible before this review.

Duplicate/scope check: #838 bounty row 109 is open with 30 effective awards remaining; #922 was the only same-scope open PR surfaced by the duplicate-active-attempt search, with #920/#916 covering different surfaces.

No blocker found.