Dev

[gamesguru]

No description provided.

[gamesguru]

@cubic-dev-ai review this pull request

[cubic-dev-ai]

@cubic-dev-ai review this pull request

@gamesguru I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

23 issues found across 58 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name=".github/workflows/install-win32.yml">

<violation number="1" location=".github/workflows/install-win32.yml:25">
P0: Python 3.4 is not supported by actions/setup-python@v5. Python 3.4 reached EOL in 2019, and setup-python's minimum supported version is Python 3.5+. This workflow will fail when attempting to set up Python 3.4. Consider either keeping actions/setup-python@v4 (though support is not guaranteed) or updating the Python version to 3.5 or later.</violation>
</file>

<file name="tests/services/test_bug.py">

<violation number="1" location="tests/services/test_bug.py:29">
P3: `len(_bugs) >= 0` is a tautology, so this test never validates the output of `bugs.list_bugs`. Replace it with an assertion that checks actual behavior (e.g., the type or expected contents of `_bugs`).</violation>

<violation number="2" location="tests/services/test_bug.py:44">
P3: `len(_bug.values()) >= 0` is always true for any dict, so this assertion does not test `list_bugs`. Replace it with a meaningful check of the bug entry's contents.</violation>
</file>

<file name="ntclient/persistence/sql/usda/__init__.py">

<violation number="1" location="ntclient/persistence/sql/usda/__init__.py:120">
P1: Parameter selection logic incorrectly uses truthiness check. If `params=()` is explicitly passed, it will fallback to `values` due to empty sequence being falsy. Use a sentinel value like `None` to properly distinguish between "not provided" and "empty sequence".</violation>
</file>

<file name="ntclient/persistence/sql/nt/funcs.py">

<violation number="1" location="ntclient/persistence/sql/nt/funcs.py:12">
P1: Handle None value from MAX(id) query on empty tables. When the table is empty, MAX(id) returns NULL (None in Python), causing int(None) to raise TypeError. Use a default value of 0 for empty tables.</violation>
</file>

<file name="requirements-optional.txt">

<violation number="1" location="requirements-optional.txt:1">
P1: Switching to the `Levenshtein` package breaks support for Python 3.4–3.7 that the project still advertises, because Levenshtein only installs on Python ≥3.8.</violation>
</file>

<file name="ntclient/services/bugs.py">

<violation number="1" location="ntclient/services/bugs.py:88">
P0: Logic bug: when show_all is False, no bugs will be printed at all because line 88 continues immediately. The condition should likely check both show_all and submission status together, or the logic should be inverted to print unsubmitted bugs when show_all is False.</violation>
</file>

<file name="ntclient/core/nutprogbar.py">

<violation number="1" location="ntclient/core/nutprogbar.py:53">
P0: Function will crash when processing multiple foods with overlapping nutrients. The summing logic is commented out and replaced with a ValueError, making the function non-functional for its intended purpose.</violation>
</file>

<file name="ntclient/persistence/csv_manager.py">

<violation number="1" location="ntclient/persistence/csv_manager.py:14">
P2: Creating the log without a directory component raises FileNotFoundError because os.makedirs is called with an empty path. Guard the directory creation so files in the current working directory can be created successfully.</violation>
</file>

<file name="ntclient/services/recipe/recipe.py">

<violation number="1" location="ntclient/services/recipe/recipe.py:55">
P2: Typo in pragma directive: should be `# pragma: no cover` (singular) not `# pragma: no covers` (plural). This prevents the coverage exclusion from working correctly.</violation>
</file>

<file name="ntclient/__main__.py">

<violation number="1" location="ntclient/__main__.py:105">
P2: Bug reports will not capture command-line arguments when program is invoked normally. When `main()` is called without arguments, `args` is None, causing `handle_runtime_exception(args, exception)` to store None instead of the actual sys.argv arguments in bug reports.</violation>
</file>

<file name="ntclient/persistence/sql/__init__.py">

<violation number="1" location="ntclient/persistence/sql/__init__.py:85">
P1: List handling with `executemany` is commented out, breaking support for list values. Either uncomment this functionality or update the function signature and all docstrings to indicate only tuples are accepted. This is a functional regression.</violation>

<violation number="2" location="ntclient/persistence/sql/__init__.py:88">
P1: Error message claims "'values' must be a list or tuple" but lists are no longer supported (executemany is commented out). This is misleading and inconsistent with the actual implementation.</violation>
</file>

<file name="requirements.txt">

<violation number="1" location="requirements.txt:4">
P1: Bump the minimum `requests` version to at least 2.20.0 to avoid CVE-2018-18074, which leaks Authorization headers on HTTPS→HTTP redirects.</violation>
</file>

<file name="tests/test_cli.py">

<violation number="1" location="tests/test_cli.py:388">
P2: Use timezone-aware datetime.datetime.now(datetime.timezone.utc) instead of deprecated datetime.datetime.utcnow(). This avoids deprecation warnings in Python 3.12+ and follows modern Python best practices for handling UTC timestamps.</violation>
</file>

<file name="ntclient/services/api/__init__.py">

<violation number="1" location="ntclient/services/api/__init__.py:35">
P1: Add requests.exceptions.Timeout to the exception handling to gracefully handle timeout scenarios and try the next mirror.</violation>
</file>

<file name="ntclient/__init__.py">

<violation number="1" location="ntclient/__init__.py:54">
P1: RuntimeError is raised with two separate arguments, which will be displayed as a tuple instead of properly formatted error messages. Combine them into a single string with a newline separator.</violation>
</file>

<file name=".github/workflows/install-linux.yml">

<violation number="1" location=".github/workflows/install-linux.yml:18">
P1: Python 3.5 is incompatible with actions/setup-python@v5 and will cause workflow failures due to SSL certificate verification errors. Python 3.5 reached EOL in 2020. Consider using supported Python versions like 3.8, 3.9, 3.10, 3.11, or 3.12 instead.</violation>
</file>

<file name="ntclient/services/analyze.py">

<violation number="1" location="ntclient/services/analyze.py:190">
P2: Remove debug print statement or gate it behind CLI_CONFIG.debug flag. This unconditionally prints a large dictionary (~180 nutrients) to stdout on every execution.</violation>
</file>

<file name="ntclient/persistence/sql/usda/funcs.py">

<violation number="1" location="ntclient/persistence/sql/usda/funcs.py:26">
P1: Missing empty set validation in sql_servings function. If _food_ids is empty, the query will have invalid SQL syntax "IN ()" causing a runtime error.</violation>

<violation number="2" location="ntclient/persistence/sql/usda/funcs.py:86">
P1: Missing empty set validation in sql_analyze_foods function. If food_ids is empty, the query will have invalid SQL syntax "IN ()" causing a runtime error.</violation>
</file>

<file name="tests/aaa/test_init.py">

<violation number="1" location="tests/aaa/test_init.py:7">
P2: The test suite relies on this file running before other tests to prepare shared state, making the tests order-dependent and brittle. Use fixtures (e.g., a session-scoped setup in conftest.py) so each test has the prerequisites it needs regardless of execution order.</violation>
</file>

<file name="ntclient/services/logs.py">

<violation number="1" location="ntclient/services/logs.py:29">
P1: Path traversal vulnerability: date_str parameter is not validated before being used in filename construction. Malicious input like "../../etc/passwd" could access files outside NUTRA_HOME. Add validation to ensure date_str matches expected format (e.g., YYYY-MM-DD) or sanitize it to remove path separators.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P0: Python 3.4 is not supported by actions/setup-python@v5. Python 3.4 reached EOL in 2019, and setup-python's minimum supported version is Python 3.5+. This workflow will fail when attempting to set up Python 3.4. Consider either keeping actions/setup-python@v4 (though support is not guaranteed) or updating the Python version to 3.5 or later.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/install-win32.yml, line 25:

<comment>Python 3.4 is not supported by actions/setup-python@v5. Python 3.4 reached EOL in 2019, and setup-python's minimum supported version is Python 3.5+. This workflow will fail when attempting to set up Python 3.4. Consider either keeping actions/setup-python@v4 (though support is not guaranteed) or updating the Python version to 3.5 or later.</comment>

<file context>
@@ -17,18 +17,18 @@ jobs:
 
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
</file context>

[cubic-dev-ai]

P0: Logic bug: when show_all is False, no bugs will be printed at all because line 88 continues immediately. The condition should likely check both show_all and submission status together, or the logic should be inverted to print unsubmitted bugs when show_all is False.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At ntclient/services/bugs.py, line 88:

<comment>Logic bug: when show_all is False, no bugs will be printed at all because line 88 continues immediately. The condition should likely check both show_all and submission status together, or the logic should be inverted to print unsubmitted bugs when show_all is False.</comment>

<file context>
@@ -0,0 +1,142 @@
+    print()
+
+    for bug in bugs:
+        if not show_all:
+            continue
+        # Skip submitted bugs by default
</file context>

[cubic-dev-ai]

P0: Function will crash when processing multiple foods with overlapping nutrients. The summing logic is commented out and replaced with a ValueError, making the function non-functional for its intended purpose.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At ntclient/core/nutprogbar.py, line 53:

<comment>Function will crash when processing multiple foods with overlapping nutrients. The summing logic is commented out and replaced with a ValueError, making the function non-functional for its intended purpose.</comment>

<file context>
@@ -1,38 +1,166 @@
+                nut_amts[int(nutrient_id)] = amt
+            else:  # pragma: no cover
+                # nut_amts[int(nutrient_id)] += amt
+                raise ValueError("Not implemented yet, need to sum up nutrient amounts")
+
+    print_bars()
</file context>

Suggested change

	raise ValueError("Not implemented yet, need to sum up nutrient amounts")
	nut_amts[int(nutrient_id)] += amt

[cubic-dev-ai]

P1: Parameter selection logic incorrectly uses truthiness check. If params=() is explicitly passed, it will fallback to values due to empty sequence being falsy. Use a sentinel value like None to properly distinguish between "not provided" and "empty sequence".

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At ntclient/persistence/sql/usda/__init__.py, line 120:

<comment>Parameter selection logic incorrectly uses truthiness check. If `params=()` is explicitly passed, it will fallback to `values` due to empty sequence being falsy. Use a sentinel value like `None` to properly distinguish between "not provided" and "empty sequence".</comment>

<file context>
@@ -98,37 +98,26 @@ def usda_ver() -> str:
-
-    con = usda_sqlite_connect(version_check=version_check)
+    # Support params alias
+    _values = params if params else values
 
     # TODO: support argument: _sql(..., params=params, ...)
</file context>

[cubic-dev-ai]

P2: Use timezone-aware datetime.datetime.now(datetime.timezone.utc) instead of deprecated datetime.datetime.utcnow(). This avoids deprecation warnings in Python 3.12+ and follows modern Python best practices for handling UTC timestamps.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_cli.py, line 388:

<comment>Use timezone-aware datetime.datetime.now(datetime.timezone.utc) instead of deprecated datetime.datetime.utcnow(). This avoids deprecation warnings in Python 3.12+ and follows modern Python best practices for handling UTC timestamps.</comment>

<file context>
@@ -375,8 +384,8 @@ def test_800_usda_upgrades_or_downgrades(self):
-            "INSERT INTO version (version) VALUES (?)",
-            values=(new_version,),
+            "INSERT INTO version (version, created) VALUES (?,?)",
+            values=(new_version, datetime.datetime.utcnow()),
             version_check=False,
         )
</file context>

Suggested change

	values=(new_version, datetime.datetime.utcnow()),
	values=(new_version, datetime.datetime.now(datetime.timezone.utc)),

[cubic-dev-ai]

P2: Remove debug print statement or gate it behind CLI_CONFIG.debug flag. This unconditionally prints a large dictionary (~180 nutrients) to stdout on every execution.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At ntclient/services/analyze.py, line 190:

<comment>Remove debug print statement or gate it behind CLI_CONFIG.debug flag. This unconditionally prints a large dictionary (~180 nutrients) to stdout on every execution.</comment>

<file context>
@@ -166,7 +186,8 @@ def day_analyze(day_csv_paths: list, rda_csv_path: str = str()) -> tuple:
                     print("INJECT RDA: {0} -->  {1}".format(substr, _nutrient[4]))
-    nutrients = {x[0]: x for x in nutrients_lists}
+    nutrients = {int(x[0]): tuple(x) for x in nutrients_lists}
+    print(nutrients)
 
     # Analyze foods
</file context>

[cubic-dev-ai]

P2: The test suite relies on this file running before other tests to prepare shared state, making the tests order-dependent and brittle. Use fixtures (e.g., a session-scoped setup in conftest.py) so each test has the prerequisites it needs regardless of execution order.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/aaa/test_init.py, line 7:

<comment>The test suite relies on this file running before other tests to prepare shared state, making the tests order-dependent and brittle. Use fixtures (e.g., a session-scoped setup in conftest.py) so each test has the prerequisites it needs regardless of execution order.</comment>

<file context>
@@ -0,0 +1,22 @@
+
+@author: shane
+
+NOTE: these tests are in a folder "aaa\" which is alphabetically RUN FIRST.
+      Other tests, such as test_bug, depend on having the newer version of nt.sqlite3
+"""
</file context>

[cubic-dev-ai]

P3: len(_bugs) >= 0 is a tautology, so this test never validates the output of bugs.list_bugs. Replace it with an assertion that checks actual behavior (e.g., the type or expected contents of _bugs).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/services/test_bug.py, line 29:

<comment>`len(_bugs) >= 0` is a tautology, so this test never validates the output of `bugs.list_bugs`. Replace it with an assertion that checks actual behavior (e.g., the type or expected contents of `_bugs`).</comment>

<file context>
@@ -0,0 +1,95 @@
+        exit_code, _bugs = bugs.list_bugs(show_all=True)
+
+        assert exit_code == 0
+        assert len(_bugs) >= 0
+        # assert len(rows) >= 0
+        # assert len(headers) == 11
</file context>

Suggested change

	assert len(_bugs) >= 0
	assert isinstance(_bugs, list)

[cubic-dev-ai]

P3: len(_bug.values()) >= 0 is always true for any dict, so this assertion does not test list_bugs. Replace it with a meaningful check of the bug entry's contents.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/services/test_bug.py, line 44:

<comment>`len(_bug.values()) >= 0` is always true for any dict, so this assertion does not test `list_bugs`. Replace it with a meaningful check of the bug entry's contents.</comment>

<file context>
@@ -0,0 +1,95 @@
+        assert exit_code == 0
+        assert len(_bugs) == 1
+        _bug = _bugs[0]
+        assert len(_bug.values()) >= 0
+        assert len(_bug.keys()) == 1
+
</file context>