Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 0 additions & 25 deletions lambdas/my_function.py

This file was deleted.

7 changes: 7 additions & 0 deletions lambdas/tokenizer_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ def lambda_handler(event: dict, lambda_context: Context) -> dict:

query_tokenizer = _get_tokenizer()

# Handle ping or health check events
# We add this after query_tokenizer initialization to ensure the tokenizer is
# initialized during cold start, even for pings
Comment on lines +47 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Really appreciated this comment. I got concerned that the tokenizer was re-initialized for each invocation of the lambda, but then (re)remembered cached _get_tokenizer() pattern. So I'm digging the comment as it provides just enough friction to slow down and think about why that's placed there.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah I had the ping condition earlier in the handler initially as I wanted it to shortcut all the heavy lifting and then realized it might defeat the "keep warm" aspect of our health checks if it didn't actually happen after the tokenizer initialization.

if "ping" in event:
logger.debug("Received ping event")
return {"status": "ok"}

# Generate query tokens
query = event.get("query", "")

Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ exclude = ["tests/"]

[tool.pytest.ini_options]
log_level = "INFO"

Copilot AI Mar 19, 2026

Copy link

Choose a reason for hiding this comment

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

The new integration tests are marked, but nothing here configures pytest to exclude them by default. Given the repo's default test command (make test) runs plain pytest, these slow disk/model-loading tests will run on every CI/local unit-test run unless callers remember -m "not integration". Consider adding a default addopts that excludes integration tests (and documenting how to opt in), or otherwise ensuring CI/test tooling explicitly selects/filters markers.

Suggested change
log_level = "INFO"
log_level = "INFO"
addopts = "-m 'not integration'"

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My local testing shows the tests didn't add significant time so I did not change the defaults.

markers = [
"integration: tests that load real models/files from disk (slow)",
]

[tool.ruff]
target-version = "py314"
Expand Down
7 changes: 0 additions & 7 deletions tests/test_my_function.py

This file was deleted.

36 changes: 36 additions & 0 deletions tests/test_query_tokenizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,39 @@ def test_load_idf_raises_filenotfounderror_when_idf_file_missing():
pytest.raises(FileNotFoundError),
):
QueryTokenizer()


# ---------------------------------------------------------------------------
# Integration tests — load the real tokenizer and IDF from disk
# Run only integration tests: uv run pytest -m integration
# Run all except integration tests: uv run pytest -m "not integration"
# ---------------------------------------------------------------------------


@pytest.mark.integration

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

+1 to the integration mark, just in case it could be helpful.

def test_integration_tokenize_query_returns_nonempty_dict():
"""Real tokenizer and IDF: tokenize_query returns a non-empty dict."""
qt = QueryTokenizer()
result = qt.tokenize_query("machine learning")
assert isinstance(result, dict)
assert len(result) > 0


@pytest.mark.integration
def test_integration_tokenize_query_values_are_positive_floats():
"""Real tokenizer and IDF: all weights are positive floats."""
qt = QueryTokenizer()
result = qt.tokenize_query("machine learning")
for token, weight in result.items():
Comment on lines +141 to +155

Copilot AI Mar 19, 2026

Copy link

Choose a reason for hiding this comment

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

These integration tests each instantiate QueryTokenizer() separately, which can be noticeably slow because it loads tokenizer assets from disk each time. Since they’re all validating behavior of the same real tokenizer, consider using a module-scoped fixture (or caching) so the tokenizer is loaded once per test session/module.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This currently does not seem to be an issue. Test skipping integration tests and tests with the integration tests are largely as slow as the test startup time.

assert isinstance(token, str), f"Expected str key, got {type(token)}"
assert isinstance(weight, float), f"Expected float weight, got {type(weight)}"
assert weight > 0, f"Expected positive weight for {token!r}, got {weight}"


@pytest.mark.integration
def test_integration_tokenize_query_deterministic():
"""Real tokenizer and IDF: same input always produces the same output."""
qt = QueryTokenizer()
result_a = qt.tokenize_query("open access repositories")
result_b = qt.tokenize_query("open access repositories")
assert result_a == result_b
32 changes: 32 additions & 0 deletions tests/test_tokenizer_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,35 @@ def test_returns_error_for_whitespace_only_query(mock_query_tokenizer):
result = tokenizer_handler.lambda_handler({"query": " "}, {})
assert result == {"error": "Query is required in the event payload."}
mock_query_tokenizer.tokenize_query.assert_not_called()


def test_ping_event_returns_ok_status(mock_query_tokenizer):
result = tokenizer_handler.lambda_handler({"ping": True}, {})
assert result == {"status": "ok"}


def test_ping_event_does_not_call_tokenize_query(mock_query_tokenizer):
tokenizer_handler.lambda_handler({"ping": True}, {})
mock_query_tokenizer.tokenize_query.assert_not_called()


def test_non_ping_event_is_not_short_circuited(mock_query_tokenizer):
mock_query_tokenizer.tokenize_query.return_value = {"hello": 1.5}
result = tokenizer_handler.lambda_handler({"query": "hello"}, {})
assert "query" in result
mock_query_tokenizer.tokenize_query.assert_called_once()


# ---------------------------------------------------------------------------
# Integration tests — load the real tokenizer and IDF from disk
# Run only integration tests: uv run pytest -m integration
# Run all except integration tests: uv run pytest -m "not integration"
# ---------------------------------------------------------------------------


@pytest.mark.integration
def test_integration_lambda_handler_returns_opensearch_query():
tokenizer_handler._get_tokenizer.cache_clear() # ensure cold start
result = tokenizer_handler.lambda_handler({"query": "open access"}, {})
assert "query" in result
assert "bool" in result["query"]
Loading