Fix committed-offset bookkeeping advancing when commit fails

[wbarnha]

Note: Before submitting this pull request, please review our contributing guidelines.

Description

Fixes #316.

Consumer._commit_offsets (faust/transport/consumer.py) updated self._committed_offset and fired self.app.monitor.on_tp_commit(...) unconditionally, outside the if did_commit: guard that already existed for the tables.on_commit(...) call two lines above.

On any transient commit failure (e.g. UnknownMemberIdError during a rebalance — the scenario in the original report), Faust's internal bookkeeping silently advanced to record those offsets as committed even though the underlying Kafka commit never happened. A later _should_commit() check (committed = self._committed_offset[tp]; return committed is None or offset > committed) would then see those offsets as already committed and skip retrying them — risking real offset loss under the default at-least-once guarantee.

Fix

Move self._committed_offset.update(...) and self.app.monitor.on_tp_commit(...) inside the existing if did_commit: block, so bookkeeping only advances when the commit actually succeeded.

Tests

• Extended test_commit_offsets__did_not_commit to assert monitor.on_tp_commit is not called and _committed_offset is unchanged when the commit fails (did_commit=False).
• Extended test_commit_offsets (happy path) to assert monitor.on_tp_commit is called and _committed_offset is updated when the commit succeeds, so both branches of the guard are covered.
• Verified the new negative-path assertions actually catch the bug: reverting the fix while keeping the new test makes it fail with AssertionError: Expected 'on_tp_commit' to not have been called. Called 1 times. — confirming the test is not vacuous.
• flake8, black --check, isort --check-only clean on both changed files.

🤖 Generated with Claude Code

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.15%. Comparing base (3073eb9) to head (39fb1f8).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #692   +/-   ##
=======================================
  Coverage   94.14%   94.15%           
=======================================
  Files         104      104           
  Lines       11136    11136           
  Branches     1201     1201           
=======================================
+ Hits        10484    10485    +1     
+ Misses        551      550    -1     
  Partials      101      101           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.