fix: Adjust commented out kv behavior to match expected behavior of comment ownership rules - #277
Merged
Conversation
…live When a pending comment run before a KV contains dead entries (commented-out KVs like `# a = 1`) mixed with alive prose, but the LAST comment is alive, only the suffix after the last dead entry is assigned to the KV. The dead entries and anything before them are pinned. If the run ends with a dead entry, the whole run still belongs to the KV (one prose line defeats R6).
…elow A commented-out KV (like `# a = 1`) now acts as a barrier in a pending comment run: only comments after the LAST dead entry whose key differs from the following KV are assigned to that KV. Dead entries whose key matches the KV stay in the run (they are related, e.g. `# b = 8080` above `b = 2`). Added commentedOutFirstKey() to extract the first key segment from a commented-out entry for comparison with the KV key from getMemberKey().
Replace the single IS_COMMENTED_OUT_KEY_VALUE regex (which matched any `# key =` prefix) with two distinct patterns: - IS_PURE_COMMENTED_OUT_KV: full-line match requiring a single value token after `=`, rejecting multi-word prose like `# key = 1 is something...` - looksLikeKV(): used for barrier detection, also matches lines with an inline comment after the value (`# key = val # note`), which still sever ownership even though they are not pure dead entries. The barrier loop in scanSlots now uses looksLikeKV() instead of the old isCommentedOutEntry() to correctly handle inline-commented KVs while ignoring prose sentences that happen to contain `=`.
Previously when every comment in a pending run was a dead entry (R6), the entire run was unconditionally pinned. Now the barrier logic also runs for all-dead runs: dead entries whose key matches the KV stay in the run and are removed with the KV, while dead entries with different keys are still pinned. Added edge-case tests for quoted values, booleans, dotted keys, inline comments with matching/non-matching keys.
Replaced the inline-comment-only heuristic in looksLikeKV() with a combined approach: a comment acts as a barrier when it matches the lenient KV regex AND either has an inline `#` comment after the value, or has ≤3 words after `=`. This correctly handles short multi-word values like `# "x key" = old value` (2 words → barrier) while still rejecting obvious prose like `# key = 1 is something to consider` (5 words → not a barrier). Added tests for quoted-key deletion with matching and non-matching commented-out KVs.
DecimalTurn
changed the title
There was a problem hiding this comment.
Pull request overview
Adjusts how “commented-out key/value” lines are detected and how they affect comment ownership, so dead entries don’t incorrectly get treated as prose (or vice versa) when determining which comments are owned/preserved during patching.
Changes:
- Tightens commented-out KV detection to require a single-token value (treating multi-word
# key = ...prose as “alive” comments). - Updates slot-scanning ownership logic so commented-out KVs can act as barriers based on key mismatch, including support for inline trailing comments and short values.
- Expands/updates test coverage for the revised comment ownership semantics.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/comment-ownership.ts | Refines regex-based detection of commented-out KVs and updates ownership barrier logic in scanSlots. |
| src/tests/patch.test.ts | Converts a previously failing test to passing and adds multiple new cases around commented-out KV ownership/barrier behavior. |
| src/tests/comment-ownership.test.ts | Updates expected behavior for an R2 adjacency case involving a commented-out KV with a differing key. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…*=.*#/ The broad pattern /#.*=.*#/ would match a # inside a quoted value (e.g. `# k = "a # b" extra`), incorrectly classifying the line as a barrier. Use a regex that requires the full KV pattern followed by a single value token and then a # inline comment marker.
The regex for detecting inline comments (`# key = val # note`) used VALUE_TOKEN which backtracks from a quoted string to [^\s#]+, causing `# k = "a # b" extra` to match `"a` as the value and ` #` as the inline comment start. Replaced with hasInlineCommentAfterValue() which walks characters after `=`, tracking quote state, and only counts a `#` as an inline comment when outside both single and double quotes. Added a test that would have failed with the old regex.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.