fix(db): use engine.begin() to commit cntrb_id updates in update_issue_closed_cntrbs_by_repo_id

[mn-ram]

Description

• Replace engine.connect() with engine.begin() in update_issue_closed_cntrbs_by_repo_id so the UPDATE statement that sets cntrb_id on the issues table is actually committed.

In SQLAlchemy 2.0, engine.connect() uses autobegin but does not auto-commit on exit — the transaction is silently rolled back, so cntrb_id was never persisted. engine.begin() commits on a clean exit and rolls back on exception. Every other write in lib.py already uses engine.begin() (lines 78, 88, 384, 426) — this was the only outlier.

Notes for Reviewers

Standalone correctness fix — aligns this function with the existing engine.begin() pattern used throughout lib.py.

Signed commits

• Yes, I signed my commits.

[MoralCode]

im not sure I see the connection between this engine.begin() fix and the issue this PR solves

can you go into more detail about how data from two different collection runs ends up in maybe the same transaction so that it can cause a violation of a uniqueness constraint?

Also id be curious how its possible to violate a uniqueness constraint without calling commit()

[mn-ram]

Thanks for the review — you're right on both counts.

On the primary fix:
In SQLAlchemy 2.0, engine.connect() uses autobegin — it starts a transaction on the first statement but does not auto-commit when the with block exits cleanly. The connection is returned to the pool with a pending rollback, so the UPDATE at line 570 is always silently discarded. engine.begin() commits on a clean exit and rolls back on exception, which is the correct behaviour for a write operation. Every other write in lib.py already uses engine.begin() — this was the only outlier.

On the UniqueViolation:
You're right to question it. update_issue_closed_cntrbs_by_repo_id only does an UPDATE, not an INSERT, so it cannot directly cause a UniqueViolation on issue-insert-unique. That symptom likely originates from the event insertion phase upstream of this function, not from this line. I overstated the connection in the description — I've updated it to remove that claim and keep the scope accurate.

Is there anything else I should address or adjust to get this merged?

[MoralCode]

update_issue_closed_cntrbs_by_repo_id only does an UPDATE, not an INSERT, so it cannot directly cause a UniqueViolation on issue-insert-unique.

That symptom likely originates from the event insertion phase upstream of this function, not from this line. I overstated the connection in the description — I've updated it to remove that claim and keep the scope accurate.

These two statements sound like they contradict each other.

---

I looked at the query immediately below the change you made and it doesnt seem like the update even touches the issue url column at issue here.

I will elaborate more in the comments on the issue itself.

Can you edit your PR description so it is not linked to the underlying issue? I think this is still a valid change, just not the solution to the underlying issue

[mn-ram]

Understood — updated the PR description to remove the link to #3457. This is now a standalone correctness fix to align with the existing engine.begin() pattern in lib.py.