feat(calendar): add advanced event search - #218
Conversation
eef9769 to
51d53c9
Compare
Signed-off-by: Dick Tump <dick@tump.me> Assisted-by: Codex:gpt-5.6-sol
51d53c9 to
e3510a0
Compare
|
Hi @dtump Undeclared direct dependenciescalendar_search.py imports recurring_ical_events, icalendar, and lxml, but none are in pyproject.toml. They resolve today only transitively via nc-py-api[calendar] → caldav 3.2.1, which declares icalendar >6.0.0, lxml *, recurring-ical-events >=2.0.0. That means a caldav release dropping or bumping any of them silently breaks this tool with an ImportError at module load — and because tools.py calls spec.loader.exec_module() inside the tool-loading loop with no try/except, an import failure here takes down loading of all tool categories, not just this one. Please add explicit constraints to tool.poetry.dependencies and regenerate poetry.lock. expand_and_filter_eventsexpand_and_filter_events is called synchronously and can take a lot of time depending on the amount of items it handles. Perhaps wrap the parse/expand phase in asyncio.to_thread? Per-calendar REPORTs seem to be fully sequentialCan we parallelize these using asyncio so we can get data faster? UNTIL in recurrencesIt seems that UNTIL clauses are ignored in occurrence estimation. This should probably be fixed. Subscribed calendars?Does this also work with subscribed calendars? |
| return "floating" | ||
| if start.utcoffset() == timedelta(0): | ||
| return "UTC" | ||
| return str(start.tzinfo) |
There was a problem hiding this comment.
When the expanded occurrence has no TZID param surviving expansion, this falls through to str(tzinfo). With icalendar 6.x's default zoneinfo provider that yields "Europe/Berlin" (fine), but a non-zoneinfo tz provider or a custom VTIMEZONE can yield an opaque repr. Guarding on getattr(tzinfo, "key", None) and omitting the field otherwise would be safer than emitting something the model may quote back to the user.
| result_limit = _validate_result_limit(limit) | ||
| validated_names = _validate_calendar_names(calendar_names) | ||
| groups = _validate_text_term_groups(text_term_groups) | ||
| return SearchBounds(start=start, end=end), validated_names, groups, result_limit |
There was a problem hiding this comment.
this function raises a value error, rather than sticking to the convention you established in the other functions to return errors. It might make sense to change this?
| MAX_XML_BYTES = 10 * 1024 * 1024 | ||
| MAX_ICALENDAR_BYTES = 512 * 1024 | ||
| RECURRENCE_UNIT_SECONDS = { | ||
| "SECONDLY": 1, |
There was a problem hiding this comment.
I noticed that the W rule set is enabled in ruff (I use ruff in my projects as well), but W191 actually flags tabs (so that means you shouldn't use them). I'm happy to change this of course, but I wanted to double check with you. Or should the ruff config be fixed as well?
And I think Black is used as well. I'm not very familiar with Black, but when I ran Black it also tried to change tabs back to spaces.
|
Thanks for the detailed response. I'll check it out and come back with an updated PR. The performance remarks are good ones: I'm using Nextcloud only personally for a few users, so I don't run into performance issues easily, but I can imagine that it's different when you have a work calendar with 10+ daily items. Will give an update when I'm finished, will pick them up one by one (with some AI help 😄) |
Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Dick Tump <dick@tump.me>
Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Dick Tump <dick@tump.me>
Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Dick Tump <dick@tump.me>
2 participants
Closes #216.
I built this because I wanted Assistant to answer calendar questions more reliably, for example:
This adds a separate, read-only
Calendar: Advanced Searchcategory. It searches a required, bounded date range across the current user’s event calendars, expands recurring events and exceptions, supports grouped content matching, and reports incomplete or truncated results explicitly.I kept it separate from
calendar.pybecause the implementation is substantial and it reads full event resources inside Context Agent for recurrence and content matching. The separate category lets admins enable or disable that access independently. Only bounded selected fields are returned to the LLM; descriptions can be matched locally but are not returned. I am happy to adjust the structure if maintainers prefer another approach.Validated against Context Agent 2.7.0 and current main, with 27 synthetic tests in a separate development harness, the upstream Ruff, Black and isort configuration, and a live Nextcloud test. Because this repository currently has no Python unit-test setup, I did not introduce one solely for this PR. I hope that is okay.
The implementation and test hardening received material AI assistance. I did functional testing myself.
Screenshot because the proof is in the pudding:
