fix: strip inline comments from INI config values by hongquanli · Pull Request #414 · Cephla-Lab/Squid

hongquanli · 2026-01-04T23:38:07Z

Summary

Fix conf_attribute_reader to strip inline comments from INI config values before parsing
Preserve # inside valid JSON strings (like {"color": "#FF0000"})

Problem

When .ini config files contain inline comments like:

DOWNSAMPLED_PLATE_RESOLUTION_UM = 10.0  # Auto-added to DOWNSAMPLED_WELL_RESOLUTIONS_UM if not present

The value was being parsed as a string ("10.0 # Auto-added...") instead of a float, causing:

TypeError: '<' not supported between instances of 'str' and 'float'

Solution

Try JSON parsing first to handle valid JSON with # inside strings
Only treat # as comment when preceded by whitespace ( # or \t#)
For JSON-like values, try stripping from rightmost # positions to handle {"color": "#FF0000"} # comment
Find earliest comment separator to handle multiple # correctly

Examples

Input	Output	Reason
`10.0 # comment`	`10.0`	Comment stripped
`{"color": "#FF0000"}`	`{"color": "#FF0000"}`	Valid JSON preserved
`{"color": "#FF0000"} # red`	`{"color": "#FF0000"}`	JSON preserved, trailing comment stripped
`[1, 2, 3] # list`	`[1, 2, 3]`	JSON parsed after stripping comment
`my#tag`	`"my#tag"`	No whitespace before `#`, preserved
`value\t# c1 # c2`	`"value"`	Strips at earliest separator

Test plan

Added 19 tests for conf_attribute_reader covering all edge cases
All 25 tests pass

🤖 Generated with Claude Code

The conf_attribute_reader function now strips inline comments (text after #) before parsing values. This fixes a bug where .ini config files with inline comments like: DOWNSAMPLED_PLATE_RESOLUTION_UM = 10.0 # some comment would be parsed as strings instead of floats, causing TypeError when sorting mixed string/float lists. Also adds type validation to ensure_plate_resolution_in_well_resolutions for clearer error messages when invalid types are passed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR fixes a bug where inline comments in INI configuration files were not properly stripped before parsing values, causing type errors when string values with comments were compared to numeric types. It also adds defensive type validation with clear error messages.

Key Changes:

Modified conf_attribute_reader to strip inline comments (text after #) from config values before parsing
Added type validation to ensure_plate_resolution_in_well_resolutions to catch configuration errors early
Added comprehensive test coverage for both changes

Reviewed changes

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

File	Description
`software/control/_def.py`	Added inline comment stripping logic to `conf_attribute_reader` that preserves `#` in JSON structures
`software/control/core/downsampled_views.py`	Added type validation to `ensure_plate_resolution_in_well_resolutions` to reject non-numeric values with clear error messages
`software/tests/control/test_def.py`	Added 14 comprehensive tests for `conf_attribute_reader` covering basic types, inline comments, and edge cases
`software/tests/control/core/test_downsampled_views.py`	Added 4 tests for type validation including error cases and edge cases like empty lists and integer values

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

Copilot · 2026-01-04T23:43:00Z

software/control/_def.py

+    if "#" in actualvalue:
+        # Check if it looks like a JSON value (starts with [ or {)
+        if not (actualvalue.startswith("[") or actualvalue.startswith("{")):
+            # Strip comment - take everything before the first #
+            actualvalue = actualvalue.split("#")[0].strip()


The comment stripping logic doesn't handle inline comments after JSON lists or dicts. If a config value like [1, 2, 3] # comment is encountered, the check for startswith("[") will prevent comment stripping, but then json.loads will fail because JSON doesn't support comments. Consider stripping comments for all value types, but doing so after checking if the value is a JSON structure, or using a more sophisticated approach that preserves hash characters only within JSON string values.

[Claude Code] Fixed in commit 4ebab0a - refactored to try JSON parsing first, then strip comments and retry. This preserves # inside valid JSON strings while handling inline comments after JSON arrays/objects.

Copilot · 2026-01-04T23:43:01Z

software/control/core/downsampled_views.py

+    # Validate plate_resolution is numeric
+    if not isinstance(plate_resolution, (int, float)):
+        raise TypeError(
+            f"plate_resolution must be numeric (int or float), got {type(plate_resolution).__name__}: {plate_resolution!r}"
+        )
+
+    # Validate all well_resolutions are numeric
+    for i, res in enumerate(well_resolutions):
+        if not isinstance(res, (int, float)):


The isinstance check will accept boolean values because bool is a subclass of int in Python. This means True and False would pass validation and be treated as 1 and 0 respectively. Consider explicitly excluding booleans if resolution values should not accept them: if not isinstance(plate_resolution, (int, float)) or isinstance(plate_resolution, bool):

Suggested change

# Validate plate_resolution is numeric

if not isinstance(plate_resolution, (int, float)):

raise TypeError(

f"plate_resolution must be numeric (int or float), got {type(plate_resolution).__name__}: {plate_resolution!r}"

)

# Validate all well_resolutions are numeric

for i, res in enumerate(well_resolutions):

if not isinstance(res, (int, float)):

# Validate plate_resolution is numeric (excluding bool)

if not isinstance(plate_resolution, (int, float)) or isinstance(plate_resolution, bool):

raise TypeError(

f"plate_resolution must be numeric (int or float), got {type(plate_resolution).__name__}: {plate_resolution!r}"

)

# Validate all well_resolutions are numeric (excluding bool)

for i, res in enumerate(well_resolutions):

if not isinstance(res, (int, float)) or isinstance(res, bool):

[Claude Code] Not in scope - this file (downsampled_views.py) is not part of this PR. The boolean subclass concern is also a false positive in practice since no user would configure pixel resolutions as boolean values.

Copilot · 2026-01-04T23:43:01Z

software/control/core/downsampled_views.py

+    if not isinstance(plate_resolution, (int, float)):
+        raise TypeError(
+            f"plate_resolution must be numeric (int or float), got {type(plate_resolution).__name__}: {plate_resolution!r}"
+        )
+
+    # Validate all well_resolutions are numeric
+    for i, res in enumerate(well_resolutions):
+        if not isinstance(res, (int, float)):


The isinstance check will accept boolean values because bool is a subclass of int in Python. Consider explicitly excluding booleans if resolution values should not accept them: if not isinstance(res, (int, float)) or isinstance(res, bool):

Suggested change

if not isinstance(plate_resolution, (int, float)):

raise TypeError(

f"plate_resolution must be numeric (int or float), got {type(plate_resolution).__name__}: {plate_resolution!r}"

)

# Validate all well_resolutions are numeric

for i, res in enumerate(well_resolutions):

if not isinstance(res, (int, float)):

if not isinstance(plate_resolution, (int, float)) or isinstance(plate_resolution, bool):

raise TypeError(

f"plate_resolution must be numeric (int or float), got {type(plate_resolution).__name__}: {plate_resolution!r}"

)

# Validate all well_resolutions are numeric

for i, res in enumerate(well_resolutions):

if not isinstance(res, (int, float)) or isinstance(res, bool):

[Claude Code] Not in scope - same as above, this file is not part of this PR.

Refactor conf_attribute_reader to try JSON parsing first before stripping inline comments. This fixes the edge case where `[1, 2, 3] # comment` would fail to parse because the comment wasn't stripped for JSON-like values. The new approach: 1. Try JSON first (preserves # inside valid JSON like {"color": "#FF0000"}) 2. If JSON fails, strip inline comments 3. Retry JSON and fall through to other types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

For JSON-like values (starting with [ or {), try stripping from rightmost # positions until JSON parses successfully. This handles edge cases like: {"color": "#FF0000"} # red color Added tests for: - JSON list/dict with inline comments - JSON containing # in strings with trailing comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

These type validation changes belong in a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

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

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

Copilot · 2026-01-05T00:18:13Z

software/control/_def.py

-            return None
-    except:
+        return json.loads(actualvalue)
+    except (json.JSONDecodeError, ValueError):


The bare except: blocks in the removed code have been replaced with more specific except (json.JSONDecodeError, ValueError) and except ValueError blocks, which is a good improvement. However, note that on line 28, catching both json.JSONDecodeError and ValueError may be redundant since json.JSONDecodeError is a subclass of ValueError in Python 3. Consider simplifying to just except json.JSONDecodeError: for clarity, unless there's a specific reason to explicitly catch both.

[Claude Code] Skipped - being explicit about both exceptions doesn't hurt and makes intent clearer.

Copilot · 2026-01-05T00:18:14Z

software/control/_def.py

+        # For non-JSON or if all JSON attempts failed, strip at first #
+        actualvalue = actualvalue.split("#")[0].strip()


The comment stripping logic on line 44 will incorrectly strip # characters that appear in non-JSON string values. For example, if the config value is "my#tag" (a string containing a hash), it would be split to just "my", losing the rest of the string.

This should be safe in practice since INI files with inline comments typically require spaces around the # comment marker (like # comment), but the current implementation doesn't check for this. Consider either:

Only stripping comments that have whitespace before the # (e.g., actualvalue.split(" #")[0] or using a regex)

Adding documentation that # characters in config values must be part of valid JSON structures

The current implementation may cause data loss for edge cases with hash symbols in plain string values.

Suggested change

# For non-JSON or if all JSON attempts failed, strip at first #

actualvalue = actualvalue.split("#")[0].strip()

# For non-JSON or if all JSON attempts failed, only strip inline comments

# where the comment marker has preceding whitespace (e.g., "value # comment").

comment_index = actualvalue.find(" #")

if comment_index == -1:

comment_index = actualvalue.find("\t#")

if comment_index != -1:

actualvalue = actualvalue[:comment_index].rstrip()

[Claude Code] Fixed in commit 2484eb9 - now requires whitespace (space or tab) before # to treat as comment. Values like "my#tag" are preserved.

Copilot · 2026-01-05T00:18:14Z

software/tests/control/test_def.py

+class TestConfAttributeReader:
+    """Tests for conf_attribute_reader function."""
+
+    def test_parses_none(self):
+        """Test parsing None value."""
+        assert conf_attribute_reader("None") is None
+
+    def test_parses_bool_true(self):
+        """Test parsing boolean True."""
+        assert conf_attribute_reader("True") is True
+        assert conf_attribute_reader("true") is True
+
+    def test_parses_bool_false(self):
+        """Test parsing boolean False."""
+        assert conf_attribute_reader("False") is False
+        assert conf_attribute_reader("false") is False
+
+    def test_parses_int(self):
+        """Test parsing integer values."""
+        assert conf_attribute_reader("42") == 42
+        assert conf_attribute_reader("-10") == -10
+
+    def test_parses_float(self):
+        """Test parsing float values."""
+        assert conf_attribute_reader("3.14") == 3.14
+        assert conf_attribute_reader("10.0") == 10.0
+        assert conf_attribute_reader("-2.5") == -2.5
+
+    def test_parses_list(self):
+        """Test parsing JSON list values."""
+        assert conf_attribute_reader("[1, 2, 3]") == [1, 2, 3]
+        assert conf_attribute_reader("[5.0, 10.0, 20.0]") == [5.0, 10.0, 20.0]
+
+    def test_parses_dict(self):
+        """Test parsing JSON dict values."""
+        assert conf_attribute_reader('{"key": "value"}') == {"key": "value"}
+
+    def test_returns_string_for_unparseable(self):
+        """Test that unparseable values are returned as strings."""
+        assert conf_attribute_reader("hello world") == "hello world"
+
+    def test_strips_inline_comments_from_float(self):
+        """Test that inline comments are stripped from float values."""
+        # This is the bug that caused TypeError in ensure_plate_resolution_in_well_resolutions
+        result = conf_attribute_reader("10.0  # Auto-added to DOWNSAMPLED_WELL_RESOLUTIONS_UM if not present")
+        assert result == 10.0
+        assert isinstance(result, float)
+
+    def test_strips_inline_comments_from_int(self):
+        """Test that inline comments are stripped from int values."""
+        result = conf_attribute_reader("42  # some comment")
+        assert result == 42
+        assert isinstance(result, int)
+
+    def test_strips_inline_comments_from_bool(self):
+        """Test that inline comments are stripped from bool values."""
+        assert conf_attribute_reader("True  # enable feature") is True
+        assert conf_attribute_reader("False  # disable feature") is False
+
+    def test_preserves_hash_in_json_list(self):
+        """Test that # in JSON list values is preserved."""
+        # A list should not have comments stripped
+        result = conf_attribute_reader('["#FFFFFF", "#000000"]')
+        assert result == ["#FFFFFF", "#000000"]
+
+    def test_preserves_hash_in_json_dict(self):
+        """Test that # in JSON dict values is preserved."""
+        result = conf_attribute_reader('{"color": "#FF0000"}')
+        assert result == {"color": "#FF0000"}
+
+    def test_strips_whitespace(self):
+        """Test that leading/trailing whitespace is stripped."""
+        assert conf_attribute_reader("  10.0  ") == 10.0
+        assert conf_attribute_reader("  True  ") is True
+
+    def test_strips_inline_comments_from_json_list(self):
+        """Test that inline comments after JSON lists are stripped."""
+        result = conf_attribute_reader("[1, 2, 3]  # some comment")
+        assert result == [1, 2, 3]
+        assert isinstance(result, list)
+
+    def test_strips_inline_comments_from_json_dict(self):
+        """Test that inline comments after JSON dicts are stripped."""
+        result = conf_attribute_reader('{"key": "value"}  # config comment')
+        assert result == {"key": "value"}
+        assert isinstance(result, dict)
+
+    def test_preserves_hash_in_json_while_stripping_trailing_comment(self):
+        """Test that # inside JSON is preserved while trailing comments are stripped."""
+        # This tests the fix for the edge case where JSON contains # AND has a trailing comment
+        result = conf_attribute_reader('{"color": "#FF0000"}  # red color')
+        assert result == {"color": "#FF0000"}
+        result = conf_attribute_reader('["#FFFFFF", "#000000"]  # colors')
+        assert result == ["#FFFFFF", "#000000"]


Missing test coverage for plain string values containing # characters. The current implementation would incorrectly split such values at the # character. Consider adding a test case like:

conf_attribute_reader("my#tag") should return "my#tag" (the full string)

Currently it would return "my" (truncated)

This edge case should be tested to ensure the comment stripping logic doesn't inadvertently remove valid data from string configuration values.

[Claude Code] Fixed in commit 2484eb9 - added test_preserves_hash_in_string_without_whitespace covering my#tag, color#123, and test#value # comment cases.

Only strip inline comments when # is preceded by space or tab. This preserves values like "my#tag" while still stripping "value # comment". Added test for string values containing # without whitespace. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

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

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

Copilot · 2026-01-05T00:50:39Z

software/tests/control/test_def.py

+    def test_parses_bool_true(self):
+        """Test parsing boolean True."""
+        assert conf_attribute_reader("True") is True
+        assert conf_attribute_reader("true") is True
+
+    def test_parses_bool_false(self):
+        """Test parsing boolean False."""
+        assert conf_attribute_reader("False") is False
+        assert conf_attribute_reader("false") is False


The test at line 56 checks conf_attribute_reader("True") but line 57 checks conf_attribute_reader("true"). Both test True, but it would be more consistent and thorough to also test that "TRUE" or other case variations either work or don't work as expected. The same applies to the False tests at lines 61-62. This would make the behavior more explicit and prevent future case-sensitivity bugs.

[Claude Code] Skipped - current behavior (True/true/False/false only) is intentional and matches common INI conventions.

Copilot · 2026-01-05T00:50:39Z

software/tests/control/test_def.py

+class TestConfAttributeReader:
+    """Tests for conf_attribute_reader function."""
+
+    def test_parses_none(self):
+        """Test parsing None value."""
+        assert conf_attribute_reader("None") is None
+
+    def test_parses_bool_true(self):
+        """Test parsing boolean True."""
+        assert conf_attribute_reader("True") is True
+        assert conf_attribute_reader("true") is True
+
+    def test_parses_bool_false(self):
+        """Test parsing boolean False."""
+        assert conf_attribute_reader("False") is False
+        assert conf_attribute_reader("false") is False
+
+    def test_parses_int(self):
+        """Test parsing integer values."""
+        assert conf_attribute_reader("42") == 42
+        assert conf_attribute_reader("-10") == -10
+
+    def test_parses_float(self):
+        """Test parsing float values."""
+        assert conf_attribute_reader("3.14") == 3.14
+        assert conf_attribute_reader("10.0") == 10.0
+        assert conf_attribute_reader("-2.5") == -2.5
+
+    def test_parses_list(self):
+        """Test parsing JSON list values."""
+        assert conf_attribute_reader("[1, 2, 3]") == [1, 2, 3]
+        assert conf_attribute_reader("[5.0, 10.0, 20.0]") == [5.0, 10.0, 20.0]
+
+    def test_parses_dict(self):
+        """Test parsing JSON dict values."""
+        assert conf_attribute_reader('{"key": "value"}') == {"key": "value"}
+
+    def test_returns_string_for_unparseable(self):
+        """Test that unparseable values are returned as strings."""
+        assert conf_attribute_reader("hello world") == "hello world"
+
+    def test_strips_inline_comments_from_float(self):
+        """Test that inline comments are stripped from float values."""
+        # This is the bug that caused TypeError in ensure_plate_resolution_in_well_resolutions
+        result = conf_attribute_reader("10.0  # Auto-added to DOWNSAMPLED_WELL_RESOLUTIONS_UM if not present")
+        assert result == 10.0
+        assert isinstance(result, float)
+
+    def test_strips_inline_comments_from_int(self):
+        """Test that inline comments are stripped from int values."""
+        result = conf_attribute_reader("42  # some comment")
+        assert result == 42
+        assert isinstance(result, int)
+
+    def test_strips_inline_comments_from_bool(self):
+        """Test that inline comments are stripped from bool values."""
+        assert conf_attribute_reader("True  # enable feature") is True
+        assert conf_attribute_reader("False  # disable feature") is False
+
+    def test_preserves_hash_in_json_list(self):
+        """Test that # in JSON list values is preserved."""
+        # A list should not have comments stripped
+        result = conf_attribute_reader('["#FFFFFF", "#000000"]')
+        assert result == ["#FFFFFF", "#000000"]
+
+    def test_preserves_hash_in_json_dict(self):
+        """Test that # in JSON dict values is preserved."""
+        result = conf_attribute_reader('{"color": "#FF0000"}')
+        assert result == {"color": "#FF0000"}
+
+    def test_strips_whitespace(self):
+        """Test that leading/trailing whitespace is stripped."""
+        assert conf_attribute_reader("  10.0  ") == 10.0
+        assert conf_attribute_reader("  True  ") is True
+
+    def test_strips_inline_comments_from_json_list(self):
+        """Test that inline comments after JSON lists are stripped."""
+        result = conf_attribute_reader("[1, 2, 3]  # some comment")
+        assert result == [1, 2, 3]
+        assert isinstance(result, list)
+
+    def test_strips_inline_comments_from_json_dict(self):
+        """Test that inline comments after JSON dicts are stripped."""
+        result = conf_attribute_reader('{"key": "value"}  # config comment')
+        assert result == {"key": "value"}
+        assert isinstance(result, dict)
+
+    def test_preserves_hash_in_json_while_stripping_trailing_comment(self):
+        """Test that # inside JSON is preserved while trailing comments are stripped."""
+        # This tests the fix for the edge case where JSON contains # AND has a trailing comment
+        result = conf_attribute_reader('{"color": "#FF0000"}  # red color')
+        assert result == {"color": "#FF0000"}
+        result = conf_attribute_reader('["#FFFFFF", "#000000"]  # colors')
+        assert result == ["#FFFFFF", "#000000"]
+
+    def test_preserves_hash_in_string_without_whitespace(self):
+        """Test that # in string values is preserved when not preceded by whitespace."""
+        # Hash without preceding whitespace is part of the value, not a comment
+        assert conf_attribute_reader("my#tag") == "my#tag"
+        assert conf_attribute_reader("color#123") == "color#123"
+        assert conf_attribute_reader("test#value  # comment") == "test#value"


The tests don't cover some edge cases that could occur in configuration files: empty strings, whitespace-only strings, strings that are only comments (e.g., "# comment"), or strings starting with hash (e.g., "#value"). These edge cases could help verify that the parser handles malformed or unusual configuration entries gracefully.

[Claude Code] Skipped - edge case tests are nice-to-have but not critical for this PR's scope (inline comment stripping).

Copilot · 2026-01-05T00:50:40Z

software/control/_def.py

+        for sep in (" #", "\t#"):
+            if sep in actualvalue:
+                actualvalue = actualvalue.split(sep)[0].rstrip()
+                break


There's a potential bug in the comment stripping logic. The code checks if " #" is in the string before checking for "\t#". For a string like "value\t# comment1 # comment2", the check for " #" will succeed (due to comment2) and split on " #", resulting in "value\t" (with trailing tab). While .rstrip() at line 48 will remove the tab, the issue is that the code should ideally strip at the first comment marker (the \t# at comment1), not at the second marker. Consider finding the earliest occurrence of any comment separator rather than checking them sequentially.

Suggested change

for sep in (" #", "\t#"):

if sep in actualvalue:

actualvalue = actualvalue.split(sep)[0].rstrip()

break

comment_separators = (" #", "\t#")

positions = [

actualvalue.find(sep) for sep in comment_separators if sep in actualvalue

]

if positions:

cut_pos = min(positions)

actualvalue = actualvalue[:cut_pos].rstrip()

[Claude Code] Fixed in commit 40877a4 - now finds earliest occurrence of " #" or "\t#" using min() on positions. Added test for this edge case.

Copilot · 2026-01-05T00:50:40Z

software/control/_def.py

-    None -> bool -> dict/list (via json) -> int -> float -> string
-    REMEMBER TO ENCLOSE PROPERTY NAMES IN LISTS/DICTS IN
-    DOUBLE QUOTES
+    JSON -> None -> bool -> int -> float -> string


The comment at line 19 states the parsing priority as "JSON -> None -> bool -> int -> float -> string", but this is slightly misleading. The actual implementation tries JSON parsing at three different points: line 27 (before comment stripping), line 61 (after comment stripping), and lines 40-42 (for JSON-like values with hash positions). The priority order might be better described as "JSON (with comments stripped if needed) -> None -> bool -> int -> float -> string" to clarify that JSON parsing is attempted multiple times with different preprocessing.

Suggested change

JSON -> None -> bool -> int -> float -> string

JSON (with comments stripped if needed) -> None -> bool -> int -> float -> string

[Claude Code] Fixed in commit 40877a4 - updated docstring to "JSON (with comments stripped if needed) -> None -> bool -> int -> float -> string".

- Find earliest occurrence of " #" or "\t#" instead of checking sequentially - Fixes edge case: "value\t# c1 # c2" now correctly returns "value" - Updated docstring to clarify JSON is tried multiple times 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

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

hongquanli requested a review from Copilot January 4, 2026 23:38

Copilot started reviewing on behalf of hongquanli January 4, 2026 23:38 View session

Copilot AI reviewed Jan 4, 2026

View reviewed changes

hongquanli and others added 3 commits January 4, 2026 16:09

chore: remove unrelated downsampled_views changes from PR

2f7980e

These type validation changes belong in a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

hongquanli requested a review from Copilot January 5, 2026 00:14

Copilot started reviewing on behalf of hongquanli January 5, 2026 00:14 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

hongquanli requested a review from Copilot January 5, 2026 00:44

Copilot started reviewing on behalf of hongquanli January 5, 2026 00:45 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

hongquanli requested a review from Copilot January 5, 2026 01:01

Copilot started reviewing on behalf of hongquanli January 5, 2026 01:01 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

hongquanli merged commit 3c08a70 into master Jan 5, 2026
9 checks passed

		# For non-JSON or if all JSON attempts failed, strip at first #
		actualvalue = actualvalue.split("#")[0].strip()

-        # For non-JSON or if all JSON attempts failed, strip at first #
-        actualvalue = actualvalue.split("#")[0].strip()
+        # For non-JSON or if all JSON attempts failed, only strip inline comments
+        # where the comment marker has preceding whitespace (e.g., "value  # comment").
+        comment_index = actualvalue.find(" #")
+        if comment_index == -1:
+            comment_index = actualvalue.find("\t#")
+        if comment_index != -1:
+            actualvalue = actualvalue[:comment_index].rstrip()

-        for sep in (" #", "\t#"):
-            if sep in actualvalue:
-                actualvalue = actualvalue.split(sep)[0].rstrip()
-                break
+        comment_separators = (" #", "\t#")
+        positions = [
+            actualvalue.find(sep) for sep in comment_separators if sep in actualvalue
+        ]
+        if positions:
+            cut_pos = min(positions)
+            actualvalue = actualvalue[:cut_pos].rstrip()

	JSON -> None -> bool -> int -> float -> string
	JSON (with comments stripped if needed) -> None -> bool -> int -> float -> string

Conversation

hongquanli commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Examples

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

hongquanli commented Jan 4, 2026 •

edited

Loading