Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions apps/common/utils/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
import hashlib
import io
import json
import math
import mimetypes
import pickle
import re
import secrets
import shutil
import uuid
from decimal import Decimal, ROUND_HALF_UP
from functools import lru_cache, reduce
from typing import Dict, List

Expand All @@ -34,6 +36,43 @@
from ..exception.app_exception import AppApiException


_NUMBER_UNITS = ["", "K", "M", "B", "T"]


def _to_fixed(value, digits):
if value is None:
value = 0
dec = Decimal(str(value)).quantize(Decimal("1").scaleb(-digits), rounding=ROUND_HALF_UP)
return f"{dec:.{digits}f}"


def _to_thousands(text):
return re.sub(r"\B(?=(\d{3})+(?!\d))", ",", text)


def number_format(value):
"""Replicate the frontend numberFormat util (thousands separators / 'k' abbreviation)."""
if value is None:
value = 0
if value < 1000:
return _to_thousands(str(value))
return f"{_to_thousands(_to_fixed(value / 1000, 1))}k"


def format_token_number(value):
"""Replicate the frontend formatTokenNumber util (K/M/B/T abbreviation)."""
if value is None or not isinstance(value, (int, float)) or not math.isfinite(value):
return "-"
normalized = float(value)
unit_index = 0
while abs(normalized) >= 1000 and unit_index < len(_NUMBER_UNITS) - 1:
normalized /= 1000
unit_index += 1
if unit_index == 0:
return str(math.floor(normalized + 0.5))
return f"{_to_fixed(normalized, 1)}{_NUMBER_UNITS[unit_index]}"


def _legacy_md5_hash(row_password):
"""
Legacy MD5 hashing — used only to detect old hashes during migration.
Expand Down
45 changes: 31 additions & 14 deletions apps/homepage/serializers/homepage.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import datetime
import os
from decimal import Decimal, ROUND_HALF_UP
from typing import List, Dict

import openpyxl
Expand Down Expand Up @@ -37,7 +38,7 @@
from common.auth.constants.role_constants import RoleConstants
from common.auth.struct.permission import Permission, Role
from common.db.search import native_search, get_dynamics_model, page_search
from common.utils.common import get_file_content
from common.utils.common import format_token_number, get_file_content, number_format
from knowledge.models import Knowledge
from maxkb.conf import PROJECT_DIR
from models_provider.base_model_provider import ModelTypeConst
Expand All @@ -50,6 +51,22 @@
TOKEN_EXPR = F("chatrecord__message_tokens") + F("chatrecord__answer_tokens")


def _round_ratio(value, divisor):
if divisor <= 0:
return 0
dec = (Decimal(value) / Decimal(divisor)).quantize(Decimal("0.1"), rounding=ROUND_HALF_UP)
result = float(dec)
return int(result) if result.is_integer() else result


def _proportion_percentage(value, total):
if not total:
return 0
dec = (Decimal(value) / Decimal(total) * 100).quantize(Decimal("0.1"), rounding=ROUND_HALF_UP)
result = min(100.0, max(0.0, float(dec)))
return int(result) if result.is_integer() else result


def hasPermission(auth, permission):
if "USER" in auth.roles:
return True
Expand Down Expand Up @@ -305,10 +322,10 @@ def export(self, auth, with_valid=True):
row = [
index,
username,
total_tokens,
total_tokens / token_count if token_count else 0,
chat_record_count,
total_tokens / chat_record_count if chat_record_count else 0,
format_token_number(total_tokens),
_proportion_percentage(total_tokens, token_count),
number_format(chat_record_count),
format_token_number(_round_ratio(total_tokens, chat_record_count)),
]
worksheet.append(row)
response = HttpResponse(content_type="application/vnd.ms-excel")
Expand Down Expand Up @@ -476,10 +493,10 @@ def export(self, auth, with_valid=True):
row = [
index,
item.name,
item.chat_record_count_total,
item.chat_record_count_total / chat_record_number if chat_record_number != 0 else 0,
item.chat_user_count,
item.chat_user_count / item.chat_record_count_total if item.chat_record_count_total != 0 else 0,
number_format(item.chat_record_count_total),
_proportion_percentage(item.chat_record_count_total, chat_record_number),
number_format(item.chat_user_count),
number_format(_round_ratio(item.chat_record_count_total, item.chat_user_count)),
]
worksheet.append(row)
response = HttpResponse(content_type="application/vnd.ms-excel")
Expand Down Expand Up @@ -605,11 +622,11 @@ def export(self, auth, with_valid=True):
row = [
index,
item.name,
total_tokens,
total_tokens / tokens_total if tokens_total else 0,
item.chat_user_count,
chat_record_count_total,
total_tokens / chat_record_count_total if chat_record_count_total else 0,
format_token_number(total_tokens),
_proportion_percentage(total_tokens, tokens_total),
number_format(chat_record_count_total),
number_format(item.chat_user_count),
format_token_number(_round_ratio(total_tokens, chat_record_count_total)),
]
worksheet.append(row)
response = HttpResponse(content_type="application/vnd.ms-excel")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The diff is syntactically plausible, but there are several semantic/data-presentation issues. The most important ones are below.

Critical issues

  1. The second export reverses a ratio formula

    • Original:
      item.chat_user_count / item.chat_record_count_total if item.chat_record_count_total != 0 else 0
    • New:
      _round_ratio(item.chat_record_count_total, item.chat_user_count)

    This now computes chat_record_count_total / chat_user_count, which is the reciprocal of the original metric. If the original metric was intentional, this is a bug. If the intended metric is “records per user”, then the Excel header and any downstream consumers must also be changed.

  2. The third export swaps two columns

    • Original order:
      item.chat_user_count,
      chat_record_count_total,
    • New order:
      number_format(chat_record_count_total),
      number_format(item.chat_user_count),

    Unless the Excel headers were also swapped in a part of the file not shown in this diff, the exported columns will no longer match their headers. This is likely a data-alignment bug.

  3. _proportion_percentage changes ratio semantics from 0–1 to 0–100

    • Original values such as total_tokens / token_count were ratios in the range 0.0–1.0.
    • New helper returns percentages in the range 0–100, rounded to one decimal.
      This may be intentional, but it requires the Excel headers and any consumers of the export to be updated to indicate “%” or “percentage”. Otherwise the meaning of the column silently changes.
  4. Formatted numbers may become text cells in Excel

    • format_token_number(...) and number_format(...) likely return strings such as "1,234" or "1.2K".
    • If appended to the worksheet as strings, Excel will treat those cells as text, not numbers. That breaks sorting, filtering, summing, formulas, and numeric analysis.
    • Preferred approach: append numeric values and set the cell display format, e.g.:
      worksheet.append([... numeric values ...])
      cell.number_format = "#,##0"
    • Also avoid using format_token_number for ratios/averages unless that is explicitly desired. A ratio is not a token count.

Potential issues

  1. Helpers do not handle None safely

    • _round_ratio(None, 10) raises on Decimal(None).
    • _round_ratio(10, None) raises on None <= 0.
    • _proportion_percentage(None, total) raises on Decimal(None).
      Consider guarding with value or 0 and if divisor is None or divisor <= 0.
  2. Decimal(value) can introduce binary-float artifacts
    If value or divisor can be float, use:

    Decimal(str(value))

    instead of Decimal(value). For integer counts this is less important, but it is safer.

  3. Converting Decimal to float can lose precision
    In _round_ratio and _proportion_percentage, converting the Decimal result to float before checking .is_integer() can lose precision for very large values.
    Safer pattern:

    if dec == dec.to_integral_value():
        return int(dec)
    return float(dec)
  4. Clamping percentages to 100 hides data anomalies
    min(100.0, max(0.0, ...)) silently caps invalid values. If value > total indicates a data-quality problem, consider logging or exposing it rather than silently clamping.

  5. Rounding before export can make percentages not sum to 100%
    This is often acceptable for display, but if exact totals are needed, keep numeric values and round only at presentation time.

Optimization suggestions

  • Add type hints to _round_ratio and _proportion_percentage.
  • Add unit tests for:
    • zero denominator,
    • None values,
    • value > total,
    • reciprocal ratio cases,
    • column order and header alignment.
  • Keep numeric data numeric in Excel and use openpyxl cell number formats for display.
  • Use a consistent naming convention, e.g. _round_quotient and _percentage, since _round_ratio actually computes a rounded quotient.
  • Verify that format_token_number and number_format exist in the imported common.utils.common module for all supported branches/versions.

Suggested fixes

For the second export, if the original metric was intended:

number_format(_round_ratio(item.chat_user_count, item.chat_record_count_total))

For the third export, restore the original column order:

number_format(item.chat_user_count),
number_format(chat_record_count_total),

For percentage columns, either update the headers to indicate %, or change the helper back to a ratio:

# returns 0.0–1.0 ratio
def _proportion_ratio(value, total):
    if not total:
        return 0
    dec = (Decimal(value) / Decimal(total)).quantize(
        Decimal("0.001"), rounding=ROUND_HALF_UP
    )
    return int(dec) if dec == dec.to_integral_value() else float(dec)

Overall: the main risks are the inverted formula, the swapped columns, the ratio-to-percentage semantic change, and the conversion of numeric Excel cells into formatted strings.

Expand Down
Loading