Skip to content

Remove duplicated HIGH-SIGNAL block in code-review, add a dup test#4

Merged
waybarrios merged 1 commit into
mainfrom
code-review-remove-duplicate-block
May 27, 2026
Merged

Remove duplicated HIGH-SIGNAL block in code-review, add a dup test#4
waybarrios merged 1 commit into
mainfrom
code-review-remove-duplicate-block

Conversation

@waybarrios

@waybarrios waybarrios commented May 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • The **Critical: only HIGH-SIGNAL issues should be flagged.** block appeared
    twice, back to back, in code-review's SKILL.md and command. The second copy
    was a strict subset of the first (it dropped the "Concrete edge cases" bullet),
    so it was pure redundancy. The duplicate is removed; the complete first copy stays.
  • The skill-lint suite gains a per-skill "no duplicated block" check (no
    non-empty line ≥ 50 chars repeats within a SKILL.md or its command), catching
    this class of copy-paste error going forward.
  • SKILL.md and command stay in sync.

Validation

  • Deterministic: npm test55/55 pass (44 prior + 11 new duplicate-block
    checks; code-review: no duplicated block ✔).
  • Behavioral: ran code-review locally via opencode run against a fixture
    with a planted off-by-one bug (return items[idx + 1]). Both models caught it at
    confidence 100 — full responses below.

Model outputs

DeepSeek V4 Flash Free — full finding
**Summary of change:** Modifies `util.py:get_item()` to add a comment and change the return from `items[idx]` to `items[idx + 1]`.

---

## Issues Found

### Critical

**1. Off-by-one bug in `get_item` — `util.py:3`**
- **Severity:** Critical
- **Confidence:** 100
- **Problem:** `return items[idx + 1]` returns the element *after* the requested index. The comment added in the same change confirms the intent was "return the item at idx" — the code contradicts it.
- **Category:** Logic error (off-by-one)
- **Reproduction scenario:** `get_item([10, 20, 30], 0)` returns `20` instead of `10`; `get_item([10, 20, 30], 2)` raises `IndexError`.
- **Fix:** Change `items[idx + 1]` to `items[idx]` on line 3.

One issue found and fixed. The change introduced a clear off-by-one bug on `util.py:3` — `items[idx + 1]` should be `items[idx]`. The comment added in the same diff confirms the intent was to return the item at `idx`, not `idx + 1`. Fix has been applied.
GPT-5.5 — full finding
**Important**
`util.py:3`
Severity: Important
Confidence: 100
Problem: `get_item(items, idx)` now returns `items[idx + 1]`, so it returns the next element instead of the requested index and fails at the last valid index.

Reproduction:
```python
get_item(["a", "b"], 0)  # returns "b", expected "a"
get_item(["a", "b"], 1)  # raises IndexError, expected "b"
```

Fix: restore `return items[idx]`.

Bug category: Logic error / off-by-one boundary error.

Both runs used opencode run --command-equivalent prompts on the same fixture; GPT-5.5 additionally dispatched the seven reviewer passes as parallel sub-agents.

The "Critical: only HIGH-SIGNAL issues should be flagged" block appeared
twice, back to back, in code-review's SKILL.md and command. The second copy
was a strict subset of the first (it dropped the "Concrete edge cases"
bullet), so it was pure redundancy. The duplicate is removed; the complete
first copy stays.

The skill-lint suite gains a per-skill "no duplicated block" check (no
non-empty line >= 50 chars repeats within a SKILL.md or its command), which
catches this class of copy-paste error. SKILL.md and command stay in sync.
@waybarrios waybarrios merged commit f9bdd3f into main May 27, 2026
1 check passed
@waybarrios waybarrios deleted the code-review-remove-duplicate-block branch May 27, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant