Add more aggregate fields to the agent assignment model

[novasam23]

Closes #2339

[Copilot]

Pull request overview

This PR extends the API aggregates needed by the task-details “assigned agents” view by adding per-assignment aggregates for current speed, hashes cracked, and keyspace searched, and centralizes task-speed aggregation logic in TaskUtils.

Changes:

• Extend TaskUtils::getTaskProgress() to optionally scope progress to a specific agent, and add TaskUtils::getCurrentSpeedOfTask() for reusable speed aggregation.
• Add AgentUtils::getAggregateCracked() to sum cracked hashes for an agent (optionally per task).
• Expose new aggregate fields (cracked, currentSpeed, searched) on AgentAssignmentAPI, and refactor TaskAPI to use the new speed helper.

Reviewed changes

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

File	Description
src/inc/utils/TaskUtils.php	Adds reusable “current speed” aggregation and enables agent-scoped task progress.
src/inc/utils/AgentUtils.php	Adds aggregate cracked-hashes helper for agent/task scope.
src/inc/apiv2/model/TaskAPI.php	Refactors current-speed aggregation to call the new TaskUtils helper.
src/inc/apiv2/model/AgentAssignmentAPI.php	Adds new aggregate fields for assignments (cracked, speed, searched).

---

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

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/inc/utils/AgentUtils.php:645

• getAggregateCracked() returns the raw SUM result, which can be NULL when there are no matching chunks. Returning NULL violates the declared return type (int) and can trigger a TypeError. Default the aggregate to 0 (and optionally cast) before returning.

    $qF1 = new QueryFilter(Chunk::AGENT_ID, $agentId, "=");
    $qF2 = $taskId !== null ? new QueryFilter(Chunk::TASK_ID, $taskId, "=") : null;
    $agg1 = new Aggregation(Chunk::CRACKED, Aggregation::SUM);
    $results = Factory::getChunkFactory()->multicolAggregationFilter([Factory::FILTER => array_filter([$qF1, $qF2])], [$agg1]);
    return $results[$agg1->getName()];

src/inc/apiv2/model/AgentAssignmentAPI.php:156

• This aggregate currently does 2 DB fetches per assignment (task + agent). The agent lookup is only needed to get the agent ID for filtering; consider changing TaskUtils::getTaskProgress to accept an optional agentId (instead of an Agent) so this method can avoid fetching the Agent model for every assignment.

    $task = Factory::getTaskFactory()->get($object->getTaskId());
    $agent = Factory::getAgentFactory()->get($object->getAgentId());
    $keyspace = $task->getKeyspace();
    return Util::showperc(TaskUtils::getTaskProgress($task, $agent), $keyspace);

[s3inlc]

In the old frontend, it was displaying the number of the keyspace covered plus the percentage. Ideally, we can provide this for the new frontend as well, but in order to allow this, the 'searched' aggregation should not return the percentage, but just the value of the keyspace, so the frontend can then calculate how many percent this is of the whole task (i.e. just removing the wrapping Util::showperc()).

[novasam23]

I've added the additional aggregate field as you suggested

[s3inlc]

Sorry, maybe I was not clear. I meant to update the 'searched' aggregate. I don't think it makes sense to have both cprogress and searched as they're quite redundant, except just the percentage calculation. But I think in general we should avoid percentage calculations in the backend, as this is a frontend part (displaying) and it's more accurate to deliver the actual number and leave it to the frontend (or a potential API user) to decide what they do with the value.

[novasam23]

Totally agree, I think the API should not be concerned with the formatting of data, that's the UI's concern. But to keep it consistent, I would suggest to get rid of all those aggregate fields searched that currently exist in some API model endpoints. We should just keep the cprogress field, which is the raw value.

[novasam23]

Removing the existing occurrences of searched would also require modifications in the UI though. Still better to get this change in before the 1.0.0 release, because removing fields after they existed in a major release at one point is not ideal.