[Bug] Inconsistent Average Seedtime Calculation

[AnabolicsAnonymous]

UNIT3D Version

9.1.7

PHP & Platform

8.4.5 - Ubuntu 24.04

Laravel verion

12.26.4

Have you done this?

• I am willing to share my stack trace and logs
• I can suggest a fix as a Pull Request

Expectation

I expect the average seedtime that is displayed on the site to be the actual seedtime that gets used.

Description

Instead, it seems like there are two different numbers that gets used. For example, the average seedtime that gets shown on the group requirements page seems to be the "history" average seedtime which doesn't include deleted torrents but on auto group it uses the total one that does include deleted torrents. Over time this causes numbers to be off and will show that one doesn't qualify for a group when they really do.

Stack trace & logs

[Roardom]

The bigger inconsistency here is that group min_uploaded requirement includes deleted torrents, but the min_seedtime doesn't

[AnabolicsAnonymous]

At the very least it should use the same number that autogroup uses, and maybe the number should be unified so all occurrences count deleted or don't.

[tdrkmn]

My 2 cents

Possible Root Cause

StatsController line 288 uses DB::table() which bypasses Eloquent's SoftDeletes:
https://github.com/HDInnovations/UNIT3D/blob/master/app/Http/Controllers/StatsController.php#L288

This includes deleted torrents in average seedtime calculation, while AutoGroup correctly uses Eloquent and excludes them.

Possible Fix

- 'user_avg_seedtime' => DB::table('history')->where('user_id', '=', $user->id)->avg('seedtime'),
+ 'user_avg_seedtime' => History::where('user_id', '=', $user->id)->avg('seedtime'),

Testing

Find test user with deleted torrents:

SELECT user_id, 
       COUNT(*) as total,
       COUNT(CASE WHEN deleted_at IS NULL THEN 1 END) as active,
       COUNT(CASE WHEN deleted_at IS NOT NULL THEN 1 END) as deleted,
       AVG(seedtime) as all_avg,
       AVG(CASE WHEN deleted_at IS NULL THEN seedtime END) as active_avg
FROM history 
GROUP BY user_id 
HAVING deleted > 0
ORDER BY deleted DESC
LIMIT 5;

Pick a user_id where deleted > 0 and all_avg differs from active_avg.

Verify fix in tinker:

php artisan tinker

// Replace 123 with your test user_id
echo "Display shows: " . \App\Models\History::where('user_id', 123)->avg('seedtime') . "\n";
echo "AutoGroup uses: " . \App\Models\User::withAvg('history as avg_seedtime', 'seedtime')->find(123)->avg_seedtime . "\n";

Both values should match after the fix.

[tdrkmn]

If the previous analysis is correct, I'll merge this commit.

FYI: CONTRIBUTING.md says to use composer check-style / composer fix-style but those commands don't exist.

I used Pint instead:

~/UNIT3D$ ./vendor/bin/pint --test

  .........

  ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── PSR 12  
    PASS   .......................................................................................................................................................................... 1525 files

Have I understood correctly? Might want to update the docs

[AnabolicsAnonymous]

@tdrkmn, I mean I haven't exactly tested History::where('user_id', '=', $user->id)->avg('seedtime'), but what I do know is that the AutoGroup uses $user->avg_seedtime for its requirements while the requirements page uses DB::table('history')->where('user_id', '=', $user->id)->avg('seedtime'),. My testing does show that these two numbers start growing further apart the longer someone uses a site which eventually makes the requirements page become inaccurate. A true fix would require changing all instances of either uses with the other depending on whether or not we want to include deleted torrents in the seedtime number.

UNIT3D/app/Console/Commands/AutoGroup.php

Line 81 in f3fc849

&& ($group->min_avg_seedtime === null || $user->avg_seedtime >= $group->min_avg_seedtime)

UNIT3D/app/Http/Controllers/StatsController.php

Lines 285 to 293 in f3fc849

	return view('stats.groups.groups-requirements', [
	'current' => Carbon::now(),
	'user' => $user,
	'user_avg_seedtime' => DB::table('history')->where('user_id', '=', $user->id)->avg('seedtime'),
	'user_account_age' => (int) Carbon::now()->diffInSeconds($user->created_at, true),
	'user_seed_size' => $user->seedingTorrents()->sum('size'),
	'user_uploads' => $user->torrents()->count(),
	'groups' => Group::orderBy('position')->where('is_modo', '=', 0)->get(),
	]);

[Roardom]

If the previous analysis is correct, I'll merge this commit.

FYI: CONTRIBUTING.md says to use composer check-style / composer fix-style but those commands don't exist.

I used Pint instead:

~/UNIT3D$ ./vendor/bin/pint --test

.........

─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── PSR 12
PASS .......................................................................................................................................................................... 1525 files

Have I understood correctly? Might want to update the docs

Thanks. However it's actually the other way around: the raw query is used specifically so that deleted torrents are included. This was implemented in various places before UNIT3D had soft deletion functionality included but sites were already manually using soft deletion (otherwise ->withTrashed() would've been a better fit). Every other place except for autogroup uses the DB::table syntax as a result. It looks to be a regression from 3723b58.

Thanks for the note on the contributing guide as well, it'll be updated shortly: #5120.

[tdrkmn]

@Roardom thanks for the clarification! You're absolutely right, I had it backwards.

I've fixed both locations where the regression from 3723b58 occurred:

• AutoGroup.php line 66
• InviteTreeController.php lines 105-106

Testing confirms the fix:

// User with n > 0 deleted torrents as explained before
DB::table('history')->where('user_id', 123)->avg('seedtime')  // CERTAIN_VALUE (includes deleted)
AutoGroup now sees: CERTAIN_VALUE
InviteTree now sees: CERTAIN_VALUE

All three now correctly include deleted torrents.

I checked for other seedtime calculations:

grep -rn "avg.*seedtime\|seedtime.*avg" app/ --include="*.php"

• StatsController.php: uses DB::table() (correct)
• HistorySearch.php: uses DB::raw() (correct)

Are there other places I should check for similar issues with withAvg()/withSum() on the history relation?

I reverted the previous commit and updated: tdrkmn@cc02154

[Roardom]

I think there's one last spot in StatsController.php:

UNIT3D/app/Http/Controllers/StatsController.php

Lines 159 to 169 in f3fc849

	public function seedtime(): \Illuminate\Contracts\View\Factory|\Illuminate\View\View
	{
	return view('stats.users.seedtime', [
	'users' => User::query()
	->with('group')
	->withSum('history as seedtime', 'seedtime')
	->orderByDesc('seedtime')
	->take(100)
	->get(),
	]);
	}

[HDVinnie]

@tdrkmn any update on submitting a pull request?