Skip to content

fix: restore failure_memory tests — remove skip, fix imports and key names#130

Closed
acailic wants to merge 3 commits into
mainfrom
fix/issue-127-failure-memory-tests
Closed

fix: restore failure_memory tests — remove skip, fix imports and key names#130
acailic wants to merge 3 commits into
mainfrom
fix/issue-127-failure-memory-tests

Conversation

@acailic
Copy link
Copy Markdown
Owner

@acailic acailic commented Apr 3, 2026

Summary

Fixes #127

  • Remove skip mark: pytestmark = pytest.mark.skip(...) removed — the feature is fully implemented in collector/failure_memory.py
  • Fix imports: Replaced local placeholder dataclasses (EmbeddingGenerationError, FailureSignature, SimilarFailureMatch) with imports from collector.failure_memory, so isinstance assertions work correctly
  • Fix metadata key names: add_args[1]["metadata"][...]add_args[1]["metadatas"][0][...] and update_args[1]["metadata"][...]update_args[1]["metadatas"][0][...] to match the actual vector DB call signatures (metadatas=[...])
  • Fix fixture usage: tool_name passed via data= dict in make_error_event; remember_session_failures fed a plain dict as it expects dict[str, Any]

Test plan

  • python3 -m pytest -q tests/test_feature_2_failure_memory.py → 12 passed, 0 failed
  • All 12 tests now run (previously 0 ran due to module-level skip)

🤖 Generated with Claude Code

…and key names

Closes #127

- Remove pytestmark skip (feature is implemented in collector/failure_memory.py)
- Import EmbeddingGenerationError, FailureSignature, SimilarFailureMatch from
  collector.failure_memory instead of defining local placeholder classes
- Fix metadata key assertions: metadata → metadatas[0] to match vector_db API
- Fix make_error_event call: pass tool_name inside data= dict
- Fix make_session call: pass plain dict instead of Session object (method
  accepts dict[str, Any])

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 3, 2026 10:50
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores and updates the Failure Memory test suite so it runs again and validates the implemented collector.failure_memory types and vector DB call signatures (fixes #127).

Changes:

  • Re-enabled the Failure Memory tests by removing the module-level skip.
  • Updated tests to import real collector.failure_memory classes (avoids placeholder/dataclass mismatches in isinstance checks).
  • Corrected vector DB call-arg assertions to use metadatas=[...] shapes and fixed fixture usage for tool_name / session input types.

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

Comment thread tests/test_feature_2_failure_memory.py Outdated
Comment on lines +358 to +359
make_decision_event,
make_session,
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make_decision_event and make_session are injected into test_link_to_why_button_analysis but not used. Removing unused fixture parameters will avoid unnecessary fixture setup and keep the test focused on the behavior under test.

Suggested change
make_decision_event,
make_session,

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Removed and from the fixture parameters — neither was referenced in the test body. Also fixed a pre-existing ruff import-sort warning (I001) in the same file. 12 tests pass.

acailic and others added 2 commits April 3, 2026 14:20
…_button_analysis

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove the incorrect skip guard and replace local placeholder classes
with real imports from collector.failure_memory; fix assertion keys
to use metadatas (plural) matching the actual vector_db.add/update
call signatures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@acailic acailic closed this Apr 3, 2026
@acailic acailic deleted the fix/issue-127-failure-memory-tests branch April 3, 2026 20:35
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.

bug: failure_memory tests incorrectly skipped; feature is implemented

2 participants