Fix multiple bugs found during code review

[eman]

Summary

Comprehensive bug review and fixes addressing critical, high, and medium severity issues found in the codebase.

Bugs Fixed

Critical

• factory.py: Resource leak when authentication fails - session not closed on error

High

• temperature.py: Missing default case in to_fahrenheit_with_formula() could return implicit None

Medium

• converters.py: div_10/mul_10 skip conversion for string inputs (returns unconverted value)
• unit_system.py: is_metric_preferred() returns None instead of bool when no override/context set
• subscriptions.py: Duplicate handler registration when subscribe() called twice with same topic/callback

Low

• command_queue.py: Silently dropped commands on QueueFull exception
• encoding.py: Reordered checks to validate chunk length before checking all-zeros content

Testing

• ✅ All 402 tests pass (3 skipped)
• ✅ Linting: All checks passed
• ✅ Type checking: No errors

Changes

• 8 files modified
• 23 insertions, 19 deletions

[Copilot]

Pull request overview

This PR fixes 6 legitimate bugs across the codebase ranging from critical resource leaks to medium-severity data conversion issues. The changes are focused and well-tested (402 tests passing). However, the PR also includes two unintended files (package.json and package-lock.json) that don't belong in this Python-only project.

Changes:

• Fixed critical resource leak in factory.py when authentication fails
• Corrected div_10/mul_10 converters to properly handle string inputs
• Fixed is_metric_preferred() to return bool instead of None
• Added default case handling in temperature conversion
• Prevented duplicate MQTT handler registration
• Changed command queue to raise exceptions instead of silently dropping commands
• Reordered validation checks in reservation decoding for efficiency
• Unintended: Added empty package.json files (should be removed)

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/nwp500/factory.py	Fixed resource leak by ensuring session cleanup if authentication fails during aenter()
src/nwp500/converters.py	Fixed div_10/mul_10 to divide/multiply string inputs after float conversion instead of skipping operation
src/nwp500/unit_system.py	Fixed return type by returning False (Fahrenheit default) instead of None when no unit system configured
src/nwp500/temperature.py	Replaced specific STANDARD case with default case (_) for better future-proofing
src/nwp500/mqtt/subscriptions.py	Prevented duplicate callback registration when subscribe() called multiple times
src/nwp500/mqtt/command_queue.py	Changed to raise QueueFull exception instead of silently logging error
src/nwp500/encoding.py	Reordered checks to validate chunk length before checking content (minor optimization)
tests/test_model_converters.py	Updated test expectations and docstrings to match corrected converter behavior
package.json	Empty Node.js package file - appears unintended for Python project
package-lock.json	Empty Node.js lockfile - appears unintended for Python project

[eman]

✅ All Review Comments Addressed

I've addressed all 3 review comments:

1. Removed unintended files

The package.json and package-lock.json files have been removed from the PR using git rm and force-pushed. The PR now contains only 8 source files (no Node.js files).

2. Updated TestDiv10Converter docstring (line 186)

Changed from: "Only divides numeric types (int, float), returns float(value) for others"
Changed to: "Divides all input types (converts to float first if needed)."

3. Updated TestMul10Converter docstring (line 250)

Changed from: "Only multiplies numeric types (int, float), returns float(value) for others"
Changed to: "Multiplies all input types (converts to float first if needed)."

Current Status:

• ✅ All changes pushed to fix/code-review-bugs branch
• ✅ 402 tests passing
• ✅ Linting checks passing
• ✅ Type checking passing

The old comments visible in the GitHub interface are from the previous commits (which have been force-pushed). The current PR commits (4baab7a and 3e5641b) contain all the fixes.