fix(#7): use auto-incrementing counter for item IDs

[fullsend-ai-coder]

The create_item() function generated IDs via len(_get_items()) + 1, which produced duplicate IDs after deletions reduced the list length.

Replace the length-based ID with a module-level monotonically increasing counter (_next_id) that never reuses values. The reset_items() helper resets the counter for test isolation.

Added test_no_id_collision_after_delete to verify that creating an item after a deletion does not produce a colliding ID.

---

Closes #7

Post-script verification

• Branch is not main/master (agent/7-fix-id-collision)
• Secret scan passed (gitleaks — 47f7f1511df8ad7036c082d9b2179085cd9ec107..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

Review

Findings

Low

• [race-condition] app.py:69 — The _new_id() function uses a non-atomic read-increment-return on a global counter, which is not thread-safe under threaded WSGI servers. However, this is a pre-existing concern — the original len(_get_items()) + 1 was equally non-thread-safe, and the shared _items_store list itself is unprotected. This PR does not regress thread safety. For future hardening, consider itertools.count(1) with next(), which is implemented in C and effectively atomic under CPython.

• [test-adequacy] test_app.py:68 — The new test_no_id_collision_after_delete validates the primary bug scenario (create-delete-create) well. A complementary edge case — multiple deletions followed by multiple creates — would provide stronger regression protection against alternative broken implementations (e.g., max(ids) + 1).

Info

• [test-docstring-convention] test_app.py:68 — test_no_id_collision_after_delete() includes a docstring, but none of the existing 8 test functions use docstrings. Minor convention deviation; the test name is already descriptive.

• [test-integrity] test_app.py — No existing test assertions were weakened or removed. The new test is purely additive and the reset_items fixture correctly resets the new _next_id counter, preserving test isolation. No split-payload concern detected.

[fullsend-ai-review]

[low] race-condition

The _new_id() function uses a non-atomic read-increment-return on a global counter, which is not thread-safe under threaded WSGI servers. However, this is a pre-existing concern — the original len(_get_items()) + 1 was equally non-thread-safe. This PR does not regress thread safety.

Suggested fix: Use itertools.count(1) with next() for an effectively atomic counter under CPython, or protect with threading.Lock if broader runtime support is needed.

[fullsend-ai-review]

[low] test-adequacy

The new test validates the primary bug scenario (single delete then create) but does not cover multi-delete scenarios, which would provide stronger regression protection.

Suggested fix: Add a test that deletes multiple items then creates multiple new ones, verifying no ID collisions occur.

[fullsend-ai-review]

[info] test-docstring-convention

test_no_id_collision_after_delete() includes a docstring, but none of the existing 8 test functions use docstrings. Minor convention deviation.

[manish-jangra]

/fs-fix fix low findings (race-condition and test-adequacy) feedback from the review agent

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed both low findings: replaced global counter with itertools.count for thread-safe ID generation, and added multi-delete regression test.

Fixed (2):

1. race-condition: _new_id() uses non-atomic read-increment-return on global counter (app.py): Replaced manual global _next_id counter with itertools.count(1) and next(), which is implemented in C and effectively atomic under CPython. Updated reset_items() to reassign the counter.
2. test-adequacy: missing edge case for multiple deletions followed by multiple creates (test_app.py): Added test_no_id_reuse_after_multiple_deletes that creates 3 items, deletes all 3, then creates 2 more and asserts all 5 assigned IDs are unique.

Tests: passed

_{Updated by fullsend fix agent}

[manish-jangra]

/fs-review