Skip to content

Include description in tasks polling ETag#698

Merged
Wirasm merged 2 commits intomainfrom
fix/task-description-etag
Sep 18, 2025
Merged

Include description in tasks polling ETag#698
Wirasm merged 2 commits intomainfrom
fix/task-description-etag

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented Sep 18, 2025

Pull Request

Summary

Ensure project task polling returns fresh descriptions by expanding the ETag payload generated by the tasks endpoint.

Changes Made

  • add task description and updated_at fields to the task list ETag hash
  • keep the endpoint response body unchanged while guaranteeing cache invalidation on content edits
  • note the outstanding backend test run blocked by missing supabase dependency in this environment

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

all be tests pass

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

None.

Additional Notes

Tests should be re-run in the standard dev container once the supabase dependency is available to confirm the updated ETag behavior end-to-end.

Summary by CodeRabbit

  • New Features

    • Added Last-Modified headers to task-related responses for better client-side caching.
    • Enhanced ETag generation to account for per-task description and updated time, improving change detection.
  • Bug Fixes

    • Standardized timestamps to UTC and improved ISO-8601 parsing (including “Z”), reducing cache inconsistencies.
    • More accurate conditional requests, decreasing unnecessary data transfers and refreshes.
  • Chores

    • Improved error logging with full exception details.
    • Minor internal notes and time-handling refinements without changing public APIs.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 18, 2025

Walkthrough

list_project_tasks now builds ETags from per-task fields including description and updated_at (parsed and normalized to UTC) and sets Last-Modified headers based on the latest task timestamp; 304 and 200 responses include ETag and Cache-Control, and Last-Modified is provided when appropriate. No public endpoint signatures changed.

Changes

Cohort / File(s) Summary
Projects API: ETag & Last-Modified logic
python/src/server/api_routes/projects_api.py
Reworked ETag generation to include each task's description and updated_at; parse/normalize updated_at (supporting datetimes and ISO-8601 strings) to UTC and track the latest timestamp as Last-Modified; set Last-Modified header on 200 and 304 responses; adjusted error logging to exc_info=True; added timezone and format_datetime imports and a comment about a complex /tasks endpoint.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant API as Projects API (list_project_tasks)
  participant Store as Task Store

  Client->>API: GET /projects/:id/tasks<br/>If-None-Match: "<etag>"<br/>(may send If-Modified-Since)
  API->>Store: Fetch tasks for project
  Store-->>API: Task list (each with updated_at, description, ...)
  note right of API #DDEBF7: Parse/normalize each task.updated_at → UTC,\ncollect etag_tasks, determine last_modified_dt
  note right of API #F7F4DD: Compute current_etag from {tasks, project_id, count}

  alt If-None-Match matches computed ETag
    API-->>Client: 304 Not Modified<br/>ETag: "<etag>"<br/>Cache-Control: ...<br/>Last-Modified: formatted last_modified_dt
  else No match
    API-->>Client: 200 OK + tasks<br/>ETag: "<etag>"<br/>Cache-Control: ...<br/>Last-Modified: formatted last_modified_dt
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I twitched my whiskers at each date,
Sniffed descriptions, parsed the slate.
I stitched new tags from every task,
Set clocks to UTC — no mask.
Hop, cache, and cheer, the headers sing! 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Include description in tasks polling ETag" is concise and accurately summarizes the primary change—adding task descriptions into the ETag used by the tasks polling endpoint—so it clearly communicates the main intent to reviewers.
Description Check ✅ Passed The PR description follows the repository template by including a Summary, Changes Made, Type of Change, Affected Services, and notes about blocked backend tests, but the Testing and Test Evidence sections lack concrete commands/results and the testing checkboxes remain unchecked, leaving verification details incomplete.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/task-description-etag

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
python/src/server/api_routes/projects_api.py (1)

597-612: Last-Modified should reflect the resource’s latest task updated_at and use RFC 7231 date format, not utcnow()/ISO.

Today it emits the request time in ISO-8601, which isn’t an HTTP-date. Use the max updated_at across tasks and format as IMF-fixdate.

-        current_etag = generate_etag(etag_data)
+        current_etag = generate_etag(etag_data)
+
+        # Compute Last-Modified as the latest updated_at across tasks (UTC)
+        last_modified_dt = None
+        for t in tasks:
+            v = t.get("updated_at")
+            dt = v if isinstance(v, datetime) else None
+            if dt is None and isinstance(v, str):
+                try:
+                    dt = datetime.fromisoformat(v.replace("Z", "+00:00"))
+                except ValueError:
+                    dt = None
+            if dt:
+                dt = dt.astimezone(timezone.utc)
+                last_modified_dt = dt if last_modified_dt is None or dt > last_modified_dt else last_modified_dt
@@
-            response.headers["Cache-Control"] = "no-cache, must-revalidate"
-            response.headers["Last-Modified"] = datetime.utcnow().isoformat()
+            response.headers["Cache-Control"] = "no-cache, must-revalidate"
+            response.headers["Last-Modified"] = (
+                format_datetime(last_modified_dt) if last_modified_dt else format_datetime(datetime.now(timezone.utc))
+            )
@@
-        response.headers["Cache-Control"] = "no-cache, must-revalidate"
-        response.headers["Last-Modified"] = datetime.utcnow().isoformat()
+        response.headers["Cache-Control"] = "no-cache, must-revalidate"
+        response.headers["Last-Modified"] = (
+            format_datetime(last_modified_dt) if last_modified_dt else format_datetime(datetime.now(timezone.utc))
+        )

Add required imports:

-from datetime import datetime
+from datetime import datetime, timezone
+from email.utils import format_datetime
🧹 Nitpick comments (3)
python/src/server/api_routes/projects_api.py (3)

581-583: Comment drift: you now include timestamps but the comment says you exclude them.

Update the comment to match behavior.

-        # Generate ETag from task data (excluding timestamps for consistency)
+        # Generate ETag from task data (includes description and updated_at to drive polling invalidation)

621-623: Log full stack traces per guidelines.

Use exc_info=True (or logger.exception) to preserve tracebacks.

-        logfire.error(f"Failed to list project tasks | error={str(e)} | project_id={project_id}")
+        logfire.error(f"Failed to list project tasks | project_id={project_id}", exc_info=True)

583-596: ETag: normalize fields; don't re-sort — TaskService already orders tasks

  • Sorting: TaskService.list_tasks already orders by task_order then created_at, so re-sorting in the endpoint is redundant (see python/src/server/services/projects/task_service.py -> .order("task_order", desc=False).order("created_at", desc=False)).
  • Normalize: when building etag_data in python/src/server/api_routes/projects_api.py, coerce values to stable types to avoid representation churn: text fields -> (task.get("...") or ""), task_order -> (task.get("task_order") or 0), updated_at -> (task.get("updated_at") and str(task.get("updated_at")) or "") — or omit updated_at entirely if you want to ignore time-only changes (see progress_api which excludes timestamp).
  • No change needed to generate_etag (it already uses json.dumps(..., sort_keys=True, default=str)).
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31cf56a and 91ee5af.

📒 Files selected for processing (1)
  • python/src/server/api_routes/projects_api.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
python/src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

python/src/**/*.py: Fail fast on critical conditions: service startup failures, missing configuration/env vars, database connection/auth failures, critical dependencies unavailable
Never accept or store corrupted data (e.g., zero embeddings, null foreign keys, malformed JSON); skip failed items entirely and continue processing
For batch/background operations, continue processing but log detailed per-item failures; for external APIs use retries with exponential backoff and then fail clearly
Error messages must include context, use specific exception types, preserve full stack traces (logging with exc_info=True), include relevant IDs/URLs, and never return None to indicate failure—raise instead; for batch ops report success counts and detailed failures
Backend uses Python 3.12 with a 120-character line length
Avoid introducing WebSocket support in the backend; updates are handled via HTTP polling
Adhere to Ruff lint rules (e.g., no unused imports) and provide type hints to satisfy MyPy

python/src/**/*.py: Fail fast on service startup failures (credentials, DB, service init); crash with clear errors
Treat missing configuration (env vars/invalid settings) as fatal; stop the system
Do not hide database connection failures; bubble up and surface clearly
Authentication/authorization failures must halt the operation and be visible
Never silently accept bad data; let Pydantic validation errors raise
If critical dependencies are unavailable, fail immediately
Reject invalid data that could corrupt state (e.g., zero embeddings, null FKs, malformed JSON)
Batch processing should complete remaining items but log detailed per-item failures
Background tasks (e.g., embedding generation) should finish queues while logging failures
Treat optional features as skippable: log and skip when disabled rather than crashing
External API calls: use retry with exponential backoff; on final failure, raise with clear service/context info
Never accept corrupted data during partial-failure work...

Files:

  • python/src/server/api_routes/projects_api.py
python/src/server/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use specific exception classes and FastAPI exception handlers to produce rich JSON error responses

python/src/server/**/*.py: Provide rich error responses in FastAPI (custom exceptions and handlers that return structured JSON with detail/type)
Implement HTTP polling support and ETag caching on endpoints to enable 304 responses

Files:

  • python/src/server/api_routes/projects_api.py
python/src/server/{api_routes,services}/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Follow Service Layer pattern: API routes delegate to services, which handle business logic and call the database layer

Files:

  • python/src/server/api_routes/projects_api.py
**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never return None/null to indicate failure; raise an exception with details instead

Files:

  • python/src/server/api_routes/projects_api.py
python/src/server/api_routes/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Place API route handlers under python/src/server/api_routes/

Files:

  • python/src/server/api_routes/projects_api.py

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
python/src/server/api_routes/projects_api.py (4)

12-13: Good: timezone-aware utilities imported.

These enable correct RFC 1123 Last-Modified headers. See follow-up comments to apply consistently across endpoints.


582-621: ETag payload now includes description/updated_at; minor typing tweak.

Implementation looks solid. Prefer Any over object for JSON-shaped dicts to satisfy mypy.

Apply:

-        etag_tasks: list[dict[str, object]] = []
+        etag_tasks: list[dict[str, Any]] = []

135-139: Normalize timestamps and Last-Modified in /projects to RFC 1123 and UTC.

Use format_datetime + timezone.utc consistently and include Last-Modified on the 304 path.

Apply:

-        response_data = {
-            "projects": formatted_projects,
-            "timestamp": datetime.utcnow().isoformat(),
-            "count": len(formatted_projects)
-        }
+        response_data = {
+            "projects": formatted_projects,
+            "timestamp": datetime.now(timezone.utc).isoformat(),
+            "count": len(formatted_projects)
+        }

         # Check if client's ETag matches
         if check_etag(if_none_match, current_etag):
             response.status_code = http_status.HTTP_304_NOT_MODIFIED
             response.headers["ETag"] = current_etag
             response.headers["Cache-Control"] = "no-cache, must-revalidate"
+            response.headers["Last-Modified"] = format_datetime(datetime.now(timezone.utc))
             return None

         # Set headers
         response.headers["ETag"] = current_etag
-        response.headers["Last-Modified"] = datetime.utcnow().isoformat()
+        response.headers["Last-Modified"] = format_datetime(datetime.now(timezone.utc))
         response.headers["Cache-Control"] = "no-cache, must-revalidate"

Also applies to: 141-147, 149-151


313-319: Apply same Last-Modified normalization to task-counts endpoint.

Add Last-Modified on 304 and switch to RFC 1123 on 200.

Apply:

         if check_etag(if_none_match, current_etag):
             response.status_code = 304
             response.headers["ETag"] = current_etag
             response.headers["Cache-Control"] = "no-cache, must-revalidate"
+            response.headers["Last-Modified"] = format_datetime(datetime.now(timezone.utc))
             logfire.debug(f"Task counts unchanged, returning 304 | etag={current_etag}")
             return None

         # Set ETag headers for successful response
         response.headers["ETag"] = current_etag
         response.headers["Cache-Control"] = "no-cache, must-revalidate"
-        response.headers["Last-Modified"] = datetime.utcnow().isoformat()
+        response.headers["Last-Modified"] = format_datetime(datetime.now(timezone.utc))

Also applies to: 323-324

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91ee5af and ce6c445.

📒 Files selected for processing (1)
  • python/src/server/api_routes/projects_api.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
python/src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

python/src/**/*.py: Fail fast on critical conditions: service startup failures, missing configuration/env vars, database connection/auth failures, critical dependencies unavailable
Never accept or store corrupted data (e.g., zero embeddings, null foreign keys, malformed JSON); skip failed items entirely and continue processing
For batch/background operations, continue processing but log detailed per-item failures; for external APIs use retries with exponential backoff and then fail clearly
Error messages must include context, use specific exception types, preserve full stack traces (logging with exc_info=True), include relevant IDs/URLs, and never return None to indicate failure—raise instead; for batch ops report success counts and detailed failures
Backend uses Python 3.12 with a 120-character line length
Avoid introducing WebSocket support in the backend; updates are handled via HTTP polling
Adhere to Ruff lint rules (e.g., no unused imports) and provide type hints to satisfy MyPy

python/src/**/*.py: Fail fast on service startup failures (credentials, DB, service init); crash with clear errors
Treat missing configuration (env vars/invalid settings) as fatal; stop the system
Do not hide database connection failures; bubble up and surface clearly
Authentication/authorization failures must halt the operation and be visible
Never silently accept bad data; let Pydantic validation errors raise
If critical dependencies are unavailable, fail immediately
Reject invalid data that could corrupt state (e.g., zero embeddings, null FKs, malformed JSON)
Batch processing should complete remaining items but log detailed per-item failures
Background tasks (e.g., embedding generation) should finish queues while logging failures
Treat optional features as skippable: log and skip when disabled rather than crashing
External API calls: use retry with exponential backoff; on final failure, raise with clear service/context info
Never accept corrupted data during partial-failure work...

Files:

  • python/src/server/api_routes/projects_api.py
python/src/server/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use specific exception classes and FastAPI exception handlers to produce rich JSON error responses

python/src/server/**/*.py: Provide rich error responses in FastAPI (custom exceptions and handlers that return structured JSON with detail/type)
Implement HTTP polling support and ETag caching on endpoints to enable 304 responses

Files:

  • python/src/server/api_routes/projects_api.py
python/src/server/{api_routes,services}/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Follow Service Layer pattern: API routes delegate to services, which handle business logic and call the database layer

Files:

  • python/src/server/api_routes/projects_api.py
**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never return None/null to indicate failure; raise an exception with details instead

Files:

  • python/src/server/api_routes/projects_api.py
python/src/server/api_routes/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Place API route handlers under python/src/server/api_routes/

Files:

  • python/src/server/api_routes/projects_api.py
🧬 Code graph analysis (1)
python/src/server/api_routes/projects_api.py (1)
python/src/server/utils/etag_utils.py (2)
  • generate_etag (8-24)
  • check_etag (27-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Backend Tests (Python + pytest)
🔇 Additional comments (2)
python/src/server/api_routes/projects_api.py (2)

628-631: Correct: RFC 1123 Last-Modified on both 304 and 200.

Nice touch to normalize to UTC and include on 304 and 200.

Also applies to: 637-639


650-650: Better error logging.

Using exc_info=True preserves stack traces; matches logging guidance.

@Wirasm Wirasm merged commit 89fa9b4 into main Sep 18, 2025
8 checks passed
@Wirasm Wirasm deleted the fix/task-description-etag branch September 18, 2025 12:18
leonj1 pushed a commit to leonj1/Archon that referenced this pull request Oct 13, 2025
* Include description in tasks polling ETag

* Align tasks endpoint headers with HTTP cache expectations
coleam00 pushed a commit that referenced this pull request Apr 7, 2026
* docs: add QUICKSTART.md for new users

Step-by-step guide from zero to running in 10 minutes. Covers:
- Prerequisites with root user warning and user creation steps
- Clone location guidance (home vs /opt)
- GitHub token + Claude auth setup
- bun link PATH fix for archon CLI
- --host 0.0.0.0 note for homelab/server users
- Troubleshooting: stale symlink fix, bun PATH, port conflicts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add Windows PowerShell install command for Bun

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add Windows PowerShell clone instructions for Step 1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add preferred directory comment to clone step

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: address review feedback on QUICKSTART.md

- Clarify --host flag only affects Vite dev server, not backend API
- Remove sudo rm -rf from troubleshooting, suggest manual deletion instead
- Add note about future `archon isolation cleanup` command

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: remove rm -rf node_modules from troubleshooting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
* docs: add QUICKSTART.md for new users

Step-by-step guide from zero to running in 10 minutes. Covers:
- Prerequisites with root user warning and user creation steps
- Clone location guidance (home vs /opt)
- GitHub token + Claude auth setup
- bun link PATH fix for archon CLI
- --host 0.0.0.0 note for homelab/server users
- Troubleshooting: stale symlink fix, bun PATH, port conflicts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add Windows PowerShell install command for Bun

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add Windows PowerShell clone instructions for Step 1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add preferred directory comment to clone step

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: address review feedback on QUICKSTART.md

- Clarify --host flag only affects Vite dev server, not backend API
- Remove sudo rm -rf from troubleshooting, suggest manual deletion instead
- Add note about future `archon isolation cleanup` command

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: remove rm -rf node_modules from troubleshooting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
* docs: add QUICKSTART.md for new users

Step-by-step guide from zero to running in 10 minutes. Covers:
- Prerequisites with root user warning and user creation steps
- Clone location guidance (home vs /opt)
- GitHub token + Claude auth setup
- bun link PATH fix for archon CLI
- --host 0.0.0.0 note for homelab/server users
- Troubleshooting: stale symlink fix, bun PATH, port conflicts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add Windows PowerShell install command for Bun

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add Windows PowerShell clone instructions for Step 1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: add preferred directory comment to clone step

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: address review feedback on QUICKSTART.md

- Clarify --host flag only affects Vite dev server, not backend API
- Remove sudo rm -rf from troubleshooting, suggest manual deletion instead
- Add note about future `archon isolation cleanup` command

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: remove rm -rf node_modules from troubleshooting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant