Skip to content

Conversation

@dssecret
Copy link
Collaborator

@dssecret dssecret commented Nov 2, 2025

No description provided.

Signed-off-by: tiksan <webmaster@deek.sh>
@dssecret dssecret added the enhancement New feature or request label Nov 2, 2025
Signed-off-by: tiksan <webmaster@deek.sh>
Signed-off-by: tiksan <webmaster@deek.sh>
Signed-off-by: tiksan <webmaster@deek.sh>
Signed-off-by: tiksan <webmaster@deek.sh>
Signed-off-by: tiksan <webmaster@deek.sh>
Signed-off-by: tiksan <webmaster@deek.sh>
Signed-off-by: tiksan <webmaster@deek.sh>
@dssecret dssecret requested a review from Copilot November 8, 2025 23:59
@dssecret dssecret marked this pull request as ready for review November 8, 2025 23:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds functionality to track armory usage from faction news and improve overdose detection by attempting to determine which drug was used when a member overdoses.

Key changes include:

  • New database table and schema for tracking armory usage (items used, loaned, returned, filled from faction armory)
  • Enhanced overdose event tracking that attempts to identify the specific drug used by checking armory logs and user logs
  • New schedulers and workers to fetch and parse faction armory news
  • User setting to opt-in for sharing overdose drug information from personal logs
  • Item name cache for bidirectional lookup between item IDs and names

Reviewed Changes

Copilot reviewed 23 out of 25 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
worker/priv/repo/migrations/20251104195426_add_armory_usage.exs Adds armory_usage table to track item usage from faction armory and od_drug_enabled setting
worker/mix.exs, worker/mix.lock Adds floki dependency for HTML parsing of faction news
worker/lib/workers/overdose_update_scheduler.ex Updated documentation to explain scheduling coordination with armory news updates
worker/lib/workers/overdose_update.ex Enhanced to determine drug used in overdose events from armory/user logs, removed redundant API parameter
worker/lib/workers/armory_news_update_scheduler.ex New scheduler to spawn armory news update jobs for factions with AA keys
worker/lib/workers/armory_news_update.ex New worker to process and insert armory news events from API responses
worker/lib/user/schema/user_settings.ex Adds od_drug_enabled field and helper function to check user preference
worker/lib/user/schema/torn_key.ex Changes access_level from integer to Ecto.Enum for better type safety
worker/lib/user/key.ex Refactored to support user ID parameter in addition to User struct
worker/lib/item/schema/item.ex Adds all() function to retrieve all items from database
worker/lib/item/name_cache.ex New bidirectional cache for item ID/name lookup with TTL expiration
worker/lib/faction/schema/armory_usage.ex New schema for armory usage logs with insert and mapping functions
worker/lib/faction/overdose.ex Enhanced with drug detection logic using armory logs and user logs
worker/lib/faction/news/news.ex New parser module for faction news with behavior definition
worker/lib/faction/news/armory_action.ex Implements parsing of armoryAction news from HTML text
worker/lib/application.ex Adds NameCache to supervision tree
worker/config/config.exs Updates cron schedules for overdose updates and adds armory news scheduler
worker/.iex.exs Configures IEx auto-reload for development
commons/tornium_commons/models/user_settings.py Adds od_drug_enabled field to Python model
application/templates/settings/settings.html Adds UI toggle for overdose drug sharing setting
application/static/settings/settings.js Adds event handlers for overdose drug toggle
application/controllers/api/v1/user_settings/init.py Adds API endpoint for toggling od_drug setting
application/controllers/api/v1/init.py Registers new od-drug settings endpoint
.gitignore Adds .tool-versions to ignored files

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

Comment on lines +72 to +102
if expired?(), do: rebuild()
end

def rebuild() do
# TODO: Revert to defp and Agent.cast
Agent.update(__MODULE__, fn %{ttl: ttl, ttl_unit: ttl_unit} = state ->
items_forward =
Tornium.Schema.Item.all()
|> Enum.map(fn %Tornium.Schema.Item{tid: item_id, name: item_name} -> {item_id, item_name} end)
|> Map.new()

items_backward = Map.new(items_forward, fn {item_id, item_name} -> {item_name, item_id} end)

state
|> Map.replace!(:expiration, expiration(ttl, ttl_unit))
|> Map.replace!(:forward, items_forward)
|> Map.replace!(:backward, items_backward)
end)
end

@spec expired?() :: boolean()
defp expired?() do
expiration = Agent.get(__MODULE__, & &1.expiration)

case expiration do
nil ->
true

%DateTime{} ->
DateTime.after?(DateTime.utc_now(), expiration)
end
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential race condition in ensure_fresh/0: The function checks if data is expired and then rebuilds it, but these are two separate operations. If multiple processes call ensure_fresh() simultaneously when the cache is expired, they will all trigger rebuild() concurrently, leading to redundant database queries and potential inconsistency. Consider using Agent.get_and_update/2 to make the check-and-rebuild atomic, or add a lock mechanism to prevent concurrent rebuilds.

Suggested change
if expired?(), do: rebuild()
end
def rebuild() do
# TODO: Revert to defp and Agent.cast
Agent.update(__MODULE__, fn %{ttl: ttl, ttl_unit: ttl_unit} = state ->
items_forward =
Tornium.Schema.Item.all()
|> Enum.map(fn %Tornium.Schema.Item{tid: item_id, name: item_name} -> {item_id, item_name} end)
|> Map.new()
items_backward = Map.new(items_forward, fn {item_id, item_name} -> {item_name, item_id} end)
state
|> Map.replace!(:expiration, expiration(ttl, ttl_unit))
|> Map.replace!(:forward, items_forward)
|> Map.replace!(:backward, items_backward)
end)
end
@spec expired?() :: boolean()
defp expired?() do
expiration = Agent.get(__MODULE__, & &1.expiration)
case expiration do
nil ->
true
%DateTime{} ->
DateTime.after?(DateTime.utc_now(), expiration)
end
Agent.get_and_update(__MODULE__, fn state ->
if expired_state?(state) do
new_state = do_rebuild(state)
{ :reloaded, new_state }
else
{ :ok, state }
end
end)
end
defp do_rebuild(%{ttl: ttl, ttl_unit: ttl_unit} = state) do
items_forward =
Tornium.Schema.Item.all()
|> Enum.map(fn %Tornium.Schema.Item{tid: item_id, name: item_name} -> {item_id, item_name} end)
|> Map.new()
items_backward = Map.new(items_forward, fn {item_id, item_name} -> {item_name, item_id} end)
state
|> Map.replace!(:expiration, expiration(ttl, ttl_unit))
|> Map.replace!(:forward, items_forward)
|> Map.replace!(:backward, items_backward)
end
defp expired_state?(%{expiration: nil}), do: true
defp expired_state?(%{expiration: expiration}) do
DateTime.after?(DateTime.utc_now(), expiration)

Copilot uses AI. Check for mistakes.
Comment on lines 213 to 241
event =
if Tornium.Schema.UserSettings.od_drug?(user_id) and not is_nil(api_key) and api_key.access_level == :full do
# The user needs to have their `od_drug_enabled` set to true and to have a default full access API
# key for this to pull their overdose logs.

overdose_logs = get_user_overdoses(api_key, overdose_last_updated)

case overdose_logs do
[%Torngen.Client.Schema.UserLog{timestamp: overdosed_at, data: %{"item" => overdosed_item_id}}] ->
event
|> Map.put(:created_at, overdosed_at)
|> Map.put(:drug, overdosed_item_id)

_ ->
# One of the following is true:
# - There are no overdose logs
# - The logs are of the wrong shape
# - There are more than one logs, so we can not be sure what drug the user overdosed on.
Map.put(event, :drug, nil)
end

event
else
# Ensure the drug is set to `nil` so the armory usage logs can attempt to find a matching log
# during its next run
Map.put(event, :drug, nil)
end

event
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event variable is unnecessarily reassigned and then returned. The reassignment on line 234 already contains the result from the if-else block (lines 214-239), so the final event assignment on line 241 is redundant. Consider removing line 241 and directly returning the result from line 234.

Suggested change
event =
if Tornium.Schema.UserSettings.od_drug?(user_id) and not is_nil(api_key) and api_key.access_level == :full do
# The user needs to have their `od_drug_enabled` set to true and to have a default full access API
# key for this to pull their overdose logs.
overdose_logs = get_user_overdoses(api_key, overdose_last_updated)
case overdose_logs do
[%Torngen.Client.Schema.UserLog{timestamp: overdosed_at, data: %{"item" => overdosed_item_id}}] ->
event
|> Map.put(:created_at, overdosed_at)
|> Map.put(:drug, overdosed_item_id)
_ ->
# One of the following is true:
# - There are no overdose logs
# - The logs are of the wrong shape
# - There are more than one logs, so we can not be sure what drug the user overdosed on.
Map.put(event, :drug, nil)
end
event
else
# Ensure the drug is set to `nil` so the armory usage logs can attempt to find a matching log
# during its next run
Map.put(event, :drug, nil)
end
event
if Tornium.Schema.UserSettings.od_drug?(user_id) and not is_nil(api_key) and api_key.access_level == :full do
# The user needs to have their `od_drug_enabled` set to true and to have a default full access API
# key for this to pull their overdose logs.
overdose_logs = get_user_overdoses(api_key, overdose_last_updated)
case overdose_logs do
[%Torngen.Client.Schema.UserLog{timestamp: overdosed_at, data: %{"item" => overdosed_item_id}}] ->
event
|> Map.put(:created_at, overdosed_at)
|> Map.put(:drug, overdosed_item_id)
_ ->
# One of the following is true:
# - There are no overdose logs
# - The logs are of the wrong shape
# - There are more than one logs, so we can not be sure what drug the user overdosed on.
Map.put(event, :drug, nil)
end
else
# Ensure the drug is set to `nil` so the armory usage logs can attempt to find a matching log
# during its next run
Map.put(event, :drug, nil)
end

Copilot uses AI. Check for mistakes.
user_id: non_neg_integer(),
user: Tornium.Schema.User.t(),
cpr_enabled: boolean(),
stat_db_enabled: boolean()
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing od_drug_enabled field in the type specification. The typespec on line 34-39 should include :od_drug_enabled as a boolean field to match the schema definition on line 50.

Suggested change
stat_db_enabled: boolean()
stat_db_enabled: boolean(),
od_drug_enabled: boolean()

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,112 @@
defmodule Tornium.Item.NameCache do
@moduledoc """
A bidirection item name cache for
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete docstring. The moduledoc text ends abruptly with "A bidirection item name cache for" without completing the sentence. Consider completing this documentation.

Suggested change
A bidirection item name cache for
A bidirectional item name cache for mapping item IDs to item names and vice versa.

Copilot uses AI. Check for mistakes.
Comment on lines +13 to +19
add :item_id, references(:item, column: :tid, type: :integer), null: false
add :is_energy_refill, :boolean, default: false, null: false
add :is_nerve_refill, :boolean, default: false, null: false

add :quantity, :integer, null: false
end

Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The item_id column allows NULL (not explicitly set to null: false), but both is_energy_refill and is_nerve_refill are set to null: false. This creates an inconsistent state where either an item_id should be present OR one of the refill flags should be true. Consider adding a database constraint to ensure at least one of these is set, or add validation logic in the schema to prevent invalid states where all three could be NULL/false.

Suggested change
add :item_id, references(:item, column: :tid, type: :integer), null: false
add :is_energy_refill, :boolean, default: false, null: false
add :is_nerve_refill, :boolean, default: false, null: false
add :quantity, :integer, null: false
end
add :item_id, references(:item, column: :tid, type: :integer)
add :is_energy_refill, :boolean, default: false, null: false
add :is_nerve_refill, :boolean, default: false, null: false
add :quantity, :integer, null: false
end
execute("""
ALTER TABLE armory_usage
ADD CONSTRAINT armory_usage_item_or_refill
CHECK (
item_id IS NOT NULL
OR is_energy_refill = TRUE
OR is_nerve_refill = TRUE
)
""")

Copilot uses AI. Check for mistakes.
end

@doc """
Insert multiple `Tornium.Faction.News.ArmoryAction` news logs for a specific fction.
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling error in docstring: 'fction' should be 'faction'.

Suggested change
Insert multiple `Tornium.Faction.News.ArmoryAction` news logs for a specific fction.
Insert multiple `Tornium.Faction.News.ArmoryAction` news logs for a specific faction.

Copilot uses AI. Check for mistakes.
Comment on lines +159 to +163
@spec old_data?(last_update :: DateTime.t()) :: boolean()
defp old_data?(last_update) do
DateTime.utc_now()
|> DateTime.add(-1, :day)
|> DateTime.after?(last_update)
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old_data?/1 function does not handle the case when last_update is nil. If overdose_last_updated from line 74-80 is nil, calling this function will result in an error. Consider adding a guard clause or pattern match to handle nil input, or ensure the function is only called when last_update is not nil.

Copilot uses AI. Check for mistakes.
quantity: String.to_integer(item_quantity),
item_name: item_name
}
end
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text_action_item/1 function doesn't have a catch-all clause for unmatched text patterns. If the news text doesn't match any of the expected patterns ("used", "filled", "loaned", "returned"), the function will raise a FunctionClauseError. Consider adding a catch-all clause that returns nil or raises a more descriptive error to handle unexpected news formats gracefully.

Suggested change
end
end
defp text_action_item(_), do: nil

Copilot uses AI. Check for mistakes.
Comment on lines +82 to +84
news
|> Map.put(:faction_id, faction_id)
|> Map.delete(:__struct__)
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The map/2 function doesn't handle the conversion of :energy_refill and :nerve_refill atoms to the corresponding boolean fields. When item_id is :energy_refill or :nerve_refill, it should set is_energy_refill or is_nerve_refill to true respectively, and set item_id to nil. Currently, the function will attempt to insert the atom value into the item_id column, which expects an integer, causing a type mismatch error.

Suggested change
news
|> Map.put(:faction_id, faction_id)
|> Map.delete(:__struct__)
news = Map.put(news, :faction_id, faction_id)
case Map.get(news, :item_id) do
:energy_refill ->
news
|> Map.put(:item_id, nil)
|> Map.put(:is_energy_refill, true)
|> Map.put(:is_nerve_refill, false)
|> Map.delete(:__struct__)
:nerve_refill ->
news
|> Map.put(:item_id, nil)
|> Map.put(:is_energy_refill, false)
|> Map.put(:is_nerve_refill, true)
|> Map.delete(:__struct__)
_ ->
news
|> Map.put(:is_energy_refill, false)
|> Map.put(:is_nerve_refill, false)
|> Map.delete(:__struct__)
end

Copilot uses AI. Check for mistakes.
|> where([k], k.default == true and k.disabled == false and k.paused == false)
|> join(:inner, [k], u in assoc(k, :user), on: u.tid == k.user_id)
|> where([k, u], u.faction_id == ^faction_id and u.faction_aa == true)
|> select([k, u, f], [k.api_key, u.tid])
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, the select clause on line 64 references a binding f that is not defined in the query. Only bindings k and u are available from the join on line 62. Remove the f binding from the select clause.

Suggested change
|> select([k, u, f], [k.api_key, u.tid])
|> select([k, u], [k.api_key, u.tid])

Copilot uses AI. Check for mistakes.
Signed-off-by: tiksan <webmaster@deek.sh>
@dssecret dssecret merged commit 6ac8942 into master Nov 9, 2025
3 of 4 checks passed
@dssecret dssecret deleted the feature/od-drug-detection branch November 9, 2025 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants