Fix tag game logic: time counting, penalties, and holder updates

[magi-9]

This PR fixes several critical bugs in the game logic:

1. Time Counting: The initial tagger's time was not being counted. Fixed by using GameSettings.tag_holder_since and ensuring it's updated correctly.
2. Penalty Application: Penalties were being applied to the receiver of the tag instead of the holder (giver). Fixed by calculating penalty for the tagger based on time held.
3. Holder Updates: The tag holder was not changing after a tag event. Fixed by explicitly updating GameSettings.current_tag_holder in process_new_tag.
4. Leaderboard: Updated calculation to reflect the new penalty logic and show real-time pending penalties for the current holder.
5. Admin: Exposed holder state in Admin for better management.

---

PR created automatically by Jules for task 8585411131460583284 started by @magi-9

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[Copilot]

Pull request overview

This PR addresses critical bugs in the tag game's scoring and state management logic. The changes ensure that time penalties are correctly applied to taggers (not receivers), that the initial tag holder's time is properly tracked, and that the current tag holder state is accurately maintained.

Changes:

• Added automatic tag_holder_since timestamp tracking when current_tag_holder changes in GameSettings.save()
• Fixed penalty application to charge taggers for time held before tagging (instead of charging the tagged player)
• Updated leaderboard calculation to include pending penalties for the current holder
• Exposed tag holder state fields in Django admin for better game management

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
backend/game/models.py	Added logic to automatically set tag_holder_since when current_tag_holder changes
backend/game/game_engine.py	Fixed penalty logic to charge tagger instead of tagged player; added pending penalty calculation for current holder in leaderboard; updated holder state after tag creation
backend/game/admin.py	Added Current State fieldset to expose current_tag_holder and tag_holder_since in admin interface

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The automatic update of tag_holder_since in the save() method can lead to a race condition. When process_new_tag() calls settings.save() (line 193 in game_engine.py), there's a window between fetching the old instance (line 64) and saving the new one (line 72) where another concurrent tag operation could interfere. This could result in incorrect tag_holder_since timestamps or the current_tag_holder being out of sync.

Consider using select_for_update() to lock the row during the update, or handle the tag_holder_since update explicitly in process_new_tag() before calling save() to avoid relying on implicit behavior that's vulnerable to race conditions.

[Copilot]

The pending penalty calculation uses int(hours_held) which truncates the decimal part. This means if someone holds the tag for 1.99 hours, they only get penalized for 1 hour. While this might be intentional, it's inconsistent with the calculation at line 160 which uses the same pattern. Consider documenting this truncation behavior or using round() or math.floor() for clarity, or even better, use math.ceil() if partial hours should count as full hours for penalty purposes.

[Copilot]

Same issue as above - the pending penalty calculation uses int(hours_held) which truncates decimal hours. This means a player holding for 1.99 hours only gets penalized for 1 hour. For consistency and clarity, consider using the same rounding function throughout (e.g., math.floor(), round(), or math.ceil()) and documenting the intended behavior.

[Copilot]

The comment states "No penalty for tagged user (they just got it)" and "No time held for tagged user (they just got it)" but these comments are misleading. The tagged user (receiver) never had a penalty or time_held to apply in the first place - those belong to the tagger. These comments make it sound like we're intentionally skipping something that should be done. Consider rewording to: "Tagged user only increments received count (penalty and time tracking apply to tagger)" or simply removing these comments as the code is self-explanatory.

[Copilot]

There's a subtle timing issue here. The code calculates time_held using timezone.now() at line 149, then later creates the Tag with tagged_at=timezone.now() at line 180. If there's any delay between these two calls (database queries, leaderboard calculation at line 163), the time_held stored in the Tag record won't exactly match the difference between the new Tag's tagged_at and the previous tag_holder_since. Consider capturing timezone.now() once at the start of the function and using it consistently for both time_held calculation and the Tag's tagged_at field.

[Copilot]

The fallback logic when settings.tag_holder_since is missing queries for tags received by the tagger (line 152). However, if the tagger is the initial holder and has never received a tag, this will correctly return None and time_held will be timedelta() (line 156). But if the tagger received tags in the past and then became holder through admin assignment (not through a tag event), this will calculate time from their last received tag instead of when they became holder, which is incorrect. The comment on line 151 acknowledges this shouldn't happen, but it's worth validating that tag_holder_since is always set when current_tag_holder changes, or raising an error here instead of using a potentially incorrect fallback.

Suggested change

	# Fallback if settings.tag_holder_since is missing (should not happen if configured correctly)
	last_received_tag = Tag.objects.filter(tagged=tagger, verified=True).order_by('-tagged_at').first()
	if last_received_tag:
	time_held = timezone.now() - last_received_tag.tagged_at
	else:
	time_held = timedelta()
	# If this happens, the game settings are in an inconsistent state and we cannot
	# reliably determine how long the current holder has held the tag.
	raise ValueError("Tag holder start time (tag_holder_since) is not set; cannot calculate time held")

[Copilot]

The time_held calculation logic has an issue. At lines 50-65, when calculating time_held from tags_received, the code uses tag.tagged_at (when the user received the tag). However, for the current holder case at line 57, it should use settings.tag_holder_since instead of tag.tagged_at to be consistent with how time is tracked in process_new_tag. If tag.tagged_at and settings.tag_holder_since differ (which they shouldn't but could due to timing or the race condition in the save() method), this will produce inconsistent results.

Suggested change

	time_diff = timezone.now() - tag.tagged_at
	# Use tag_holder_since as the canonical start time when available
	start_time = settings.tag_holder_since or tag.tagged_at
	time_diff = timezone.now() - start_time

[Copilot]

The critical bug fixes in this PR (penalty application to tagger instead of tagged, time counting for initial holder, tag_holder_since updates) lack test coverage. Consider adding tests that verify:

1. When a tag is created, the tagger's total_points is decreased by time_penalty (not the tagged user's)
2. The tag_holder_since is updated when current_tag_holder changes
3. The initial holder's time is correctly counted when they have no received tags
4. Pending penalties are correctly calculated in the leaderboard for the current holder

This is especially important given these are bug fixes for critical game logic.

[magi-9]

@jules check copilot review above

[google-labs-jules]

@jules check copilot review above

Thank you for the heads-up. I've reviewed the code and identified a critical flaw in the leaderboard calculation (it was incorrectly summing intervals between received tags). I've refactored calculate_leaderboard to properly sum time_held from completed tags and added a fallback mechanism to ensure correct time calculation for running games where tag_holder_since might be missing.