[Refactor] ai report 데이터 추가 및 5각형 레이더 UI 추가#102
Conversation
Summary of ChangesHello @jxhee99, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 AI 리포트의 품질과 사용자 경험을 향상시키기 위해 백엔드 데이터 처리 로직과 프론트엔드 UI를 개선합니다. 주간 변화율 계산을 위한 데이터 수집을 확장하고, 웹사이트의 전반적인 상태를 직관적으로 파악할 수 있는 Health Score를 도입하여 리포트의 깊이와 유용성을 높였습니다. 또한, 이 Health Score를 시각적으로 보여주는 레이더 차트를 추가하여 사용자가 데이터를 더 쉽게 이해하고 분석할 수 있도록 지원합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the AI report by introducing new data points and a 5-axis radar chart UI. The backend now collects 14 days of data to calculate week-over-week deltas for various metrics, which are then used to determine severity in diagnostics, and a new 'Health Score' model has been added, calculated server-side. The frontend has been updated to display these new delta percentages and visualize the health score, with LLM prompts updated to utilize these new data points effectively. However, critical security vulnerabilities were identified: a lack of site isolation in the data collection process could allow users to access aggregated analytics data from other sites, and the LLM prompt construction is vulnerable to injection via the user-provided analyst note. Additionally, the backend uses a broad Exception catch in _collect_report_bundle, which could mask specific issues, and some helper functions within _collect_report_bundle could be extracted for better modularity.
| device_type, browser_family, country_code, path, event_name, | ||
| CASE WHEN time >= TIMESTAMP '{mid_str}' THEN 'current' ELSE 'previous' END AS period, |
There was a problem hiding this comment.
Vulnerability: Broken Access Control (Missing Site Isolation)
The SQL query in _collect_report_bundle fetches analytics data without filtering by site_id. Since the application appears to be multi-tenant (as evidenced by the site_id parameter in the request and schema), this allows any user to generate a report that includes aggregated data from all sites in the database. This is a significant information exposure vulnerability.
Remediation
- Update
_collect_report_bundleto accept asite_idparameter. - Add a
WHERE site_id = ...clause to all SQL queries in this function (e.g.,WHERE site_id = '{site_id}' AND ...). - Pass the
site_idfromgenerate_reportto_collect_report_bundle.
There was a problem hiding this comment.
As I mentioned in the previous pr, it's not a multi-tenancy environment so it's not a problem
| "- `diagnostics`: 2~4 core issues by device/browser/country, citing actual numbers AND week-over-week delta_pct from the data. " | ||
| "Set severity based on the delta (High/Medium/Low per the rules). Include delta_pct as a number.\n" |
There was a problem hiding this comment.
Vulnerability: Prompt Injection
The LLM prompt construction incorporates the soft_prompt variable (derived from user input) without sanitization. This makes the feature vulnerable to prompt injection attacks, where a user can manipulate the analyst note to override system instructions and influence the generated report's content.
Remediation
Implement strict validation for the prompt input. Use clear delimiters to separate user-provided content from system instructions in the prompt, and explicitly instruct the LLM to treat the user input as a non-binding hint.
| except Exception as exc: | ||
| log.warning("InfluxDB query failed: %s", exc) |
There was a problem hiding this comment.
The _collect_report_bundle function uses a broad except Exception as exc: block. It's generally better to catch more specific exceptions to avoid masking unexpected errors and to handle different error types appropriately. For example, you might want to catch InfluxDBClient3Error or httpx.RequestError if those are the expected failure modes.
_collect_report_bundle 함수는 광범위한 except Exception as exc: 블록을 사용하고 있습니다. 예기치 않은 오류를 가리지 않고 다양한 오류 유형을 적절하게 처리하기 위해 더 구체적인 예외를 포착하는 것이 일반적으로 좋습니다. 예를 들어, 예상되는 실패 모드가 InfluxDBClient3Error 또는 httpx.RequestError인 경우 해당 예외를 포착할 수 있습니다.
|
|
LGTM :) |
2 participants
🧾 ApiLog
✏️ Summary
ai 리포트 제공 데이터 추가 및 UI 개선했습니다.
📢 Motivation
리포트 품질 개선을 위해 진행했습니다.
📌 Type of Change
🛠️ Implementation Details
[BE]
[FE]
🧪 Testing
데이터 생성 실패 시 로그 데이터 부족인지 아니면 api key 미설정 때문인지 구분해서 테스트 진행했습니다.
🖼️ Screenshots / Demos
[데이터 생성 성공]
[데이터 생성 실패]
[데이터 부족시]
✅ Checklist