Commit Β·
9bccc12
1
Parent(s): e936cfa
[KM-680] feat(report): show author display name (users.fullname)
Browse filesThe report header showed the raw user_id under 'generated by'. Resolve a real display name instead:
- AnalysisReport gains user_name; generator renders 'by {user_name or user_id}'.
- api/v1/report.py::_resolve_user_name reads users.fullname via AsyncSessionLocal, never-throw (missing user / read error -> None -> falls back to user_id so a report never fails on the name lookup).
- Decision: resolve in Python (self-contained, unblocked); swap to a Go-passed display name later if preferred.
- DEV_PLAN: #19 marked done; Β§6 decision resolved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- DEV_PLAN.md +3 -3
- src/agents/report/generator.py +5 -2
- src/agents/report/schemas.py +1 -0
- src/api/v1/report.py +21 -1
DEV_PLAN.md
CHANGED
|
@@ -86,7 +86,7 @@ Status legend: β¬ not started Β· π in progress Β· β
done Β· β blocked Β·
|
|
| 86 |
| 2 | Drop `problem_validated`: gate neutered; `is_report_ready`/`report_floor` β **β₯1 completed analysis** only, no-LLM | Rifqi | β
| Done 2026-06-25. `gate.py` no-op, gate call site commented in `chat_handler.py`, `report_floor` drops the goal check. Tests updated (`test_gate`/`test_chat_handler`/`test_readiness`). Suite: **284 passed, 7 skipped**; ruff clean |
|
| 87 |
| 3 | dedorch `analysis` migration: drop `problem_statement`/`problem_validated`, add `objective` + `business_questions`, rename `owner_id`β`user_id` | Harry | β¬ | Blocks #4 |
|
| 88 |
| 4 | Update Python `analysis` model + `state_store` + `analysis.py` to match #3; `owner_id`β`user_id` | Rifqi/Sofhia | β after #3 | getattr-tolerant so deploy order isn't a hard blocker |
|
| 89 |
-
| 5 | Report generator β `objective`+`business_questions`, "generated by {user}", formal outline | Sofhia | π pushed (KM-674) | Goal-shape done
|
| 90 |
| 6 | Report skill input contract: `analysis_id` + `user_id` (+ `chat_history`?) | Sofhia/Rifqi | β¬ | Records-based β chat_history unused β see #17 |
|
| 91 |
| 7 | `report_id` state update via request to Go, not direct DB | Sofhia + Harry | β¬ | Needs Go endpoint. See #18 for the rest of the writes |
|
| 92 |
| 8 | Expose/confirm 4 FE endpoints; comment `check_data` + analysis CRUD | Sofhia | β
| KM-678: `list_tools` trimmed to `/help` + `/report` (analytics/check/retrieve commented in the **menu**). `help` confirmed as a `call_agent` intent β no own endpoint. Analysis CRUD endpoint left **registered**: "comment the rest" was about the FE slash menu, not killing HTTP routes Go needs |
|
|
@@ -100,7 +100,7 @@ Status legend: β¬ not started Β· π in progress Β· β
done Β· β blocked Β·
|
|
| 100 |
| 16 | Decide `analysis_records` creation under `SKIP_INIT_DB`: dedorch migration (Harry) or Python carve-out | Rifqi + Harry | β¬ new | Table won't exist post-cutover otherwise |
|
| 101 |
| 17 | Reconcile report contract with records-based: remove/flag `chat_history` | Sofhia/Rifqi | β¬ new | Resolves #5 β #6 contradiction |
|
| 102 |
| 18 | Confirm Go owns ALL analysis-state writes + both creation gates; make Python `state_store.ensure` read-only | Rifqi + Harry | β¬ new | Extends #7 to the full decision #4 (G5) |
|
| 103 |
-
| 19 | Decide report author display-name source (`users.fullname` vs Go-passed name) | Sofhia |
|
| 104 |
| 20 | **Help handoff:** update `handlers/help.py` + `help.md` β drop the `problem_validated` tier + `define_problem_statement` action (the skill it points at is gone as of #1) | Sofhia | β
| Done 2026-06-25. `help.py`: actions = `ask_analysis_question` (always) + `generate_report` (if ready); renders objective/business_questions (getattr-tolerant). `help.md` v1βv2: 3 tiers, no `/problem_statement`, `/generate report`β`/report`. Local test_help updated β 11 pass |
|
| 105 |
|
| 106 |
## 5. Critical path & sequencing
|
|
@@ -115,6 +115,6 @@ Status legend: β¬ not started Β· π in progress Β· β
done Β· β blocked Β·
|
|
| 115 |
|
| 116 |
- `analysis_records`: dedorch-owned vs Python-owned-with-carve-out (#16/#14).
|
| 117 |
- ~~Whether `help` is its own endpoint or via `call_agent` (#8).~~ RESOLVED: `help` is a `call_agent` intent (no own endpoint).
|
| 118 |
-
- Author display-name source for the report (#19).
|
| 119 |
- Confirm Go takes over analysis creation + both creation gates (data-first + mandatory fields) (#18).
|
| 120 |
- Keep vs drop `chat_history` in the report contract (#17).
|
|
|
|
| 86 |
| 2 | Drop `problem_validated`: gate neutered; `is_report_ready`/`report_floor` β **β₯1 completed analysis** only, no-LLM | Rifqi | β
| Done 2026-06-25. `gate.py` no-op, gate call site commented in `chat_handler.py`, `report_floor` drops the goal check. Tests updated (`test_gate`/`test_chat_handler`/`test_readiness`). Suite: **284 passed, 7 skipped**; ruff clean |
|
| 87 |
| 3 | dedorch `analysis` migration: drop `problem_statement`/`problem_validated`, add `objective` + `business_questions`, rename `owner_id`β`user_id` | Harry | β¬ | Blocks #4 |
|
| 88 |
| 4 | Update Python `analysis` model + `state_store` + `analysis.py` to match #3; `owner_id`β`user_id` | Rifqi/Sofhia | β after #3 | getattr-tolerant so deploy order isn't a hard blocker |
|
| 89 |
+
| 5 | Report generator β `objective`+`business_questions`, "generated by {user}", formal outline | Sofhia | π pushed (KM-674) | Goal-shape done; author name done (#19). Open: background/EDA/insights mapping |
|
| 90 |
| 6 | Report skill input contract: `analysis_id` + `user_id` (+ `chat_history`?) | Sofhia/Rifqi | β¬ | Records-based β chat_history unused β see #17 |
|
| 91 |
| 7 | `report_id` state update via request to Go, not direct DB | Sofhia + Harry | β¬ | Needs Go endpoint. See #18 for the rest of the writes |
|
| 92 |
| 8 | Expose/confirm 4 FE endpoints; comment `check_data` + analysis CRUD | Sofhia | β
| KM-678: `list_tools` trimmed to `/help` + `/report` (analytics/check/retrieve commented in the **menu**). `help` confirmed as a `call_agent` intent β no own endpoint. Analysis CRUD endpoint left **registered**: "comment the rest" was about the FE slash menu, not killing HTTP routes Go needs |
|
|
|
|
| 100 |
| 16 | Decide `analysis_records` creation under `SKIP_INIT_DB`: dedorch migration (Harry) or Python carve-out | Rifqi + Harry | β¬ new | Table won't exist post-cutover otherwise |
|
| 101 |
| 17 | Reconcile report contract with records-based: remove/flag `chat_history` | Sofhia/Rifqi | β¬ new | Resolves #5 β #6 contradiction |
|
| 102 |
| 18 | Confirm Go owns ALL analysis-state writes + both creation gates; make Python `state_store.ensure` read-only | Rifqi + Harry | β¬ new | Extends #7 to the full decision #4 (G5) |
|
| 103 |
+
| 19 | Decide report author display-name source (`users.fullname` vs Go-passed name) | Sofhia | β
| Done 2026-06-25. `AnalysisReport.user_name`; `generator` renders `user_name or user_id`; `api/v1/report.py::_resolve_user_name` reads `users.fullname` never-throw (fallback `user_id`). Decided: resolve in Python (unblocked); swap to Go-passed name later if preferred |
|
| 104 |
| 20 | **Help handoff:** update `handlers/help.py` + `help.md` β drop the `problem_validated` tier + `define_problem_statement` action (the skill it points at is gone as of #1) | Sofhia | β
| Done 2026-06-25. `help.py`: actions = `ask_analysis_question` (always) + `generate_report` (if ready); renders objective/business_questions (getattr-tolerant). `help.md` v1βv2: 3 tiers, no `/problem_statement`, `/generate report`β`/report`. Local test_help updated β 11 pass |
|
| 105 |
|
| 106 |
## 5. Critical path & sequencing
|
|
|
|
| 115 |
|
| 116 |
- `analysis_records`: dedorch-owned vs Python-owned-with-carve-out (#16/#14).
|
| 117 |
- ~~Whether `help` is its own endpoint or via `call_agent` (#8).~~ RESOLVED: `help` is a `call_agent` intent (no own endpoint).
|
| 118 |
+
- ~~Author display-name source for the report (#19).~~ RESOLVED: Python resolves `users.fullname` (fallback `user_id`); swap to a Go-passed name later if preferred.
|
| 119 |
- Confirm Go takes over analysis creation + both creation gates (data-first + mandatory fields) (#18).
|
| 120 |
- Keep vs drop `chat_history` in the report contract (#17).
|
src/agents/report/generator.py
CHANGED
|
@@ -187,8 +187,9 @@ def _render_markdown(report: AnalysisReport) -> str:
|
|
| 187 |
# after rendering and lives in the structured `version` field / API metadata.
|
| 188 |
parts: list[str] = ["# Analysis Report"]
|
| 189 |
meta = f"*Generated {report.generated_at:%Y-%m-%d}"
|
| 190 |
-
|
| 191 |
-
|
|
|
|
| 192 |
meta += f" Β· {len(report.record_ids)} analyses Β· {len(report.data_sources)} source(s)*"
|
| 193 |
parts.append(meta)
|
| 194 |
|
|
@@ -294,6 +295,7 @@ class ReportGenerator:
|
|
| 294 |
analysis_id: str,
|
| 295 |
user_id: str | None = None,
|
| 296 |
problem_statement: ProblemStatement | None = None,
|
|
|
|
| 297 |
) -> AnalysisReport:
|
| 298 |
records = await self._ensure_record_store().list_for_analysis(analysis_id)
|
| 299 |
if not records:
|
|
@@ -313,6 +315,7 @@ class ReportGenerator:
|
|
| 313 |
report = AnalysisReport(
|
| 314 |
analysis_id=analysis_id,
|
| 315 |
user_id=user_id,
|
|
|
|
| 316 |
version=0, # assigned by ReportStore.save under the advisory lock
|
| 317 |
generated_at=datetime.now(UTC),
|
| 318 |
problem_statement=ps,
|
|
|
|
| 187 |
# after rendering and lives in the structured `version` field / API metadata.
|
| 188 |
parts: list[str] = ["# Analysis Report"]
|
| 189 |
meta = f"*Generated {report.generated_at:%Y-%m-%d}"
|
| 190 |
+
author = report.user_name or report.user_id
|
| 191 |
+
if author:
|
| 192 |
+
meta += f" by {author}"
|
| 193 |
meta += f" Β· {len(report.record_ids)} analyses Β· {len(report.data_sources)} source(s)*"
|
| 194 |
parts.append(meta)
|
| 195 |
|
|
|
|
| 295 |
analysis_id: str,
|
| 296 |
user_id: str | None = None,
|
| 297 |
problem_statement: ProblemStatement | None = None,
|
| 298 |
+
user_name: str | None = None,
|
| 299 |
) -> AnalysisReport:
|
| 300 |
records = await self._ensure_record_store().list_for_analysis(analysis_id)
|
| 301 |
if not records:
|
|
|
|
| 315 |
report = AnalysisReport(
|
| 316 |
analysis_id=analysis_id,
|
| 317 |
user_id=user_id,
|
| 318 |
+
user_name=user_name,
|
| 319 |
version=0, # assigned by ReportStore.save under the advisory lock
|
| 320 |
generated_at=datetime.now(UTC),
|
| 321 |
problem_statement=ps,
|
src/agents/report/schemas.py
CHANGED
|
@@ -76,6 +76,7 @@ class AnalysisReport(BaseModel):
|
|
| 76 |
report_id: str = Field(default_factory=lambda: uuid4().hex)
|
| 77 |
analysis_id: str
|
| 78 |
user_id: str | None = None
|
|
|
|
| 79 |
version: int
|
| 80 |
generated_at: datetime
|
| 81 |
# Frozen snapshots.
|
|
|
|
| 76 |
report_id: str = Field(default_factory=lambda: uuid4().hex)
|
| 77 |
analysis_id: str
|
| 78 |
user_id: str | None = None
|
| 79 |
+
user_name: str | None = None # display name for "generated by"; falls back to user_id
|
| 80 |
version: int
|
| 81 |
generated_at: datetime
|
| 82 |
# Frozen snapshots.
|
src/api/v1/report.py
CHANGED
|
@@ -59,6 +59,25 @@ def _problem_statement_from(state) -> ProblemStatement:
|
|
| 59 |
return ProblemStatement(objective=objective, business_questions=business_questions)
|
| 60 |
|
| 61 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 62 |
async def _record_report_on_state(analysis_id: str, report_id: str) -> None:
|
| 63 |
"""Write the new `report_id` back onto the Analysis State (never-throw).
|
| 64 |
|
|
@@ -117,8 +136,9 @@ async def generate_report(
|
|
| 117 |
|
| 118 |
try:
|
| 119 |
problem_statement = _problem_statement_from(state)
|
|
|
|
| 120 |
report = await _generator.generate(
|
| 121 |
-
analysis_id, user_id, problem_statement=problem_statement
|
| 122 |
)
|
| 123 |
except ReportError as e:
|
| 124 |
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) from e
|
|
|
|
| 59 |
return ProblemStatement(objective=objective, business_questions=business_questions)
|
| 60 |
|
| 61 |
|
| 62 |
+
async def _resolve_user_name(user_id: str) -> str | None:
|
| 63 |
+
"""Best-effort display name (`users.fullname`) for the report's "generated by".
|
| 64 |
+
|
| 65 |
+
Never-throw: a missing user or read error falls back to None, so the generator
|
| 66 |
+
shows the raw `user_id`. Resolving it here keeps the report self-contained (#19);
|
| 67 |
+
swap to a Go-passed display name later if the team prefers.
|
| 68 |
+
"""
|
| 69 |
+
try:
|
| 70 |
+
from src.db.postgres.connection import AsyncSessionLocal
|
| 71 |
+
from src.db.postgres.models import User
|
| 72 |
+
|
| 73 |
+
async with AsyncSessionLocal() as session:
|
| 74 |
+
user = await session.get(User, user_id)
|
| 75 |
+
return user.fullname if user is not None else None
|
| 76 |
+
except Exception as e: # noqa: BLE001 β never block a report on the name lookup
|
| 77 |
+
logger.warning("report: user name resolve failed", user_id=user_id, error=str(e))
|
| 78 |
+
return None
|
| 79 |
+
|
| 80 |
+
|
| 81 |
async def _record_report_on_state(analysis_id: str, report_id: str) -> None:
|
| 82 |
"""Write the new `report_id` back onto the Analysis State (never-throw).
|
| 83 |
|
|
|
|
| 136 |
|
| 137 |
try:
|
| 138 |
problem_statement = _problem_statement_from(state)
|
| 139 |
+
user_name = await _resolve_user_name(user_id)
|
| 140 |
report = await _generator.generate(
|
| 141 |
+
analysis_id, user_id, problem_statement=problem_statement, user_name=user_name
|
| 142 |
)
|
| 143 |
except ReportError as e:
|
| 144 |
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(e)) from e
|