🎨 Palette: Enhance SSP report with dynamic Zero Trust maturity and navigation#150
🎨 Palette: Enhance SSP report with dynamic Zero Trust maturity and navigation#150AGI-Corporation wants to merge 4 commits intomainfrom
Conversation
…vigation - Replace static "See assessment" with dynamic ZT pillar progress bars - Add "Back to Top" navigation links to the SSP Markdown report - Add dynamic findings summary line (e.g., "Showing X of Y findings") - Refactor dashboard and reports to use shared ZT_PILLAR_DOMAINS constant - Update get_status_emoji for better visual scannability (🛑 -> 🚫) - Standardize maturity calculation with 0.5 weight for partial implementations - Add verification tests in tests/test_palette_ux.py Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughModified SSP report generation to introduce Zero Trust pillar maturity calculations and visualization. Added a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…vigation - Replace static "See assessment" with dynamic ZT pillar progress bars - Add "Back to Top" navigation links to the SSP Markdown report - Add dynamic findings summary line (e.g., "Showing X of Y findings") - Refactor dashboard and reports to use shared ZT_PILLAR_DOMAINS constant - Update get_status_emoji for better visual scannability (🛑 -> 🚫) - Standardize maturity calculation with 0.5 weight for partial implementations - Add verification tests in tests/test_palette_ux.py Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d0d232ee7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pillar_assessments = [ | ||
| a | ||
| for a in assessments | ||
| if a.control_id.split(".")[0] in domains | ||
| or (controls.get(a.control_id) and controls[a.control_id].domain in domains) |
There was a problem hiding this comment.
Base pillar maturity on full control set, not assessed subset
In generate_ssp, pillar maturity is computed from assessments only, so controls in the pillar that have no current assessment are dropped from the denominator. That means a pillar can show very high (or 100%) maturity when only a small assessed subset is implemented, which materially overstates compliance posture in the SSP. This affects any environment where assessments are partial/incomplete; use the mapped controls for each pillar as the denominator and treat missing assessments explicitly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/routers/reports.py (2)
178-187: Build the pillar table rows fromZT_PILLAR_DOMAINSto avoid config/content drift.These rows duplicate the same pillar/domain mapping already defined in code. If the constant changes, markdown can become stale.
♻️ Suggested refactor
@@ - ssp = f"""# System Security Plan (SSP) + zt_rows = "\n".join( + f"| {pillar} | {', '.join(domains)} | {get_progress_bar(zt_maturity[pillar])} |" + for pillar, domains in ZT_PILLAR_DOMAINS.items() + ) + + ssp = f"""# System Security Plan (SSP) @@ | ZT Pillar | CMMC Domains | Maturity | |-----------|--------------|----------| -| User | AC, IA, PS | {get_progress_bar(zt_maturity["User"])} | -| Device | CM, MA, PE | {get_progress_bar(zt_maturity["Device"])} | -| Network | SC, AC | {get_progress_bar(zt_maturity["Network"])} | -| Application | CM, CA, SI | {get_progress_bar(zt_maturity["Application"])} | -| Data | MP, SC, AU | {get_progress_bar(zt_maturity["Data"])} | -| Visibility & Analytics | AU, IR, RA | {get_progress_bar(zt_maturity["Visibility & Analytics"])} | -| Automation & Orchestration | IR, SI, CA | {get_progress_bar(zt_maturity["Automation & Orchestration"])} | +{zt_rows}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routers/reports.py` around lines 178 - 187, The markdown table rows are hard-coded and duplicate the existing ZT_PILLAR_DOMAINS mapping; replace the literal rows by iterating ZT_PILLAR_DOMAINS to generate each row and call get_progress_bar(zt_maturity[pillar]) for the Maturity column so the table stays in sync; locate the rendering code in reports.py that currently emits the seven literal rows (references: ZT_PILLAR_DOMAINS, get_progress_bar, zt_maturity) and build the table lines with a loop over ZT_PILLAR_DOMAINS.items() (or equivalent) formatting pillar, domains, and progress bar dynamically.
26-34: Consolidate ZT pillar-domain mapping in one shared source.Line 26 introduces
ZT_PILLAR_DOMAINS, but a duplicate map still exists inagents/orchestrator/agent.py(Line 83-91 in provided context). Keeping two canonical copies can drift and produce inconsistent rollups.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/routers/reports.py` around lines 26 - 34, ZT_PILLAR_DOMAINS is defined here and duplicated in agents/orchestrator/agent.py; extract this mapping into a single shared constant module (e.g., create or reuse a central constants or config module) and have both reports.py and agents.orchestrator.agent import that shared symbol instead of redefining it, then remove the duplicate mapping from agents/orchestrator/agent.py so there is only one canonical ZT_PILLAR_DOMAINS used across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/routers/reports.py`:
- Around line 178-187: The markdown table rows are hard-coded and duplicate the
existing ZT_PILLAR_DOMAINS mapping; replace the literal rows by iterating
ZT_PILLAR_DOMAINS to generate each row and call
get_progress_bar(zt_maturity[pillar]) for the Maturity column so the table stays
in sync; locate the rendering code in reports.py that currently emits the seven
literal rows (references: ZT_PILLAR_DOMAINS, get_progress_bar, zt_maturity) and
build the table lines with a loop over ZT_PILLAR_DOMAINS.items() (or equivalent)
formatting pillar, domains, and progress bar dynamically.
- Around line 26-34: ZT_PILLAR_DOMAINS is defined here and duplicated in
agents/orchestrator/agent.py; extract this mapping into a single shared constant
module (e.g., create or reuse a central constants or config module) and have
both reports.py and agents.orchestrator.agent import that shared symbol instead
of redefining it, then remove the duplicate mapping from
agents/orchestrator/agent.py so there is only one canonical ZT_PILLAR_DOMAINS
used across the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 564daf14-3ae6-43e8-a821-ee6079cd6c42
📒 Files selected for processing (2)
backend/routers/reports.pytests/test_palette_ux.py
| assert "🚫" in content or "not_implemented" in content | ||
| assert "Zero Trust Pillar Alignment" in content | ||
| assert "Maturity" in content | ||
| assert "[↑ Back to Top](#system-security-plan-ssp)" in content | ||
| assert "Showing" in content and "findings" in content |
There was a problem hiding this comment.
Tighten assertions to validate the actual UX elements, not fallback text.
Line 78 can pass even if 🚫 regresses, because "not_implemented" appears elsewhere in static content. Line 82 is also too broad for the findings summary format.
✅ More precise assertions
+import re
@@
- assert "🚫" in content or "not_implemented" in content
+ assert "| Not Implemented | 🚫 " in content
@@
- assert "Showing" in content and "findings" in content
+ assert re.search(r"Showing\s+\d+\s+of\s+\d+\s+findings", content)…vigation - Replace static "See assessment" with dynamic ZT pillar progress bars - Add "Back to Top" navigation links to the SSP Markdown report - Add dynamic findings summary line (e.g., "Showing X of Y findings") - Refactor dashboard and reports to use shared ZT_PILLAR_DOMAINS constant - Update get_status_emoji for better visual scannability (🛑 -> 🚫) - Standardize maturity calculation with 0.5 weight for partial implementations - Clean up unused imports and fix PEP8 linting issues in reports router - Add verification tests in tests/test_palette_ux.py Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
…vigation - Replace static "See assessment" with dynamic ZT pillar progress bars - Add "Back to Top" navigation links to the SSP Markdown report - Add dynamic findings summary line (e.g., "Showing X of Y findings") - Refactor dashboard and reports to use shared ZT_PILLAR_DOMAINS constant - Update get_status_emoji for better visual scannability (🛑 -> 🚫) - Standardize maturity calculation with 0.5 weight for partial implementations - Clean up unused imports and fix PEP8 linting issues in reports router - Add verification tests in tests/test_palette_ux.py Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
🎨 Palette: SSP Report UX Enhancement
💡 What:
This enhancement transforms the System Security Plan (SSP) report from a static document into a dynamic, visually rich, and navigable report. Key changes include:
ZT_PILLAR_DOMAINSconstant.🎯 Why:
The previous SSP report used static placeholders ("See assessment") for Zero Trust alignment, making it difficult for stakeholders to gauge compliance at a glance. Long reports were also difficult to navigate, and users lacked context on the total number of findings.
📸 Before/After:
████░░░░░░40.0%.♿ Accessibility:
PR created automatically by Jules for task 11756707536476559921 started by @AGI-Corporation
Summary by CodeRabbit
New Features
Style