Skip to content

Conversation

@bobbravo2
Copy link
Contributor

@bobbravo2 bobbravo2 commented Dec 18, 2025

Testing before merge checklist

Before merge:

  • Run mkdocs serve and verify all 4 new diagrams render correctly
  • Spot-check 2-3 updated doc pages in browser
  • Verify components/README.md diagram links work
  • Update CLAUDE.md migration status note (or create follow-up issue)
  • Create tracking issue for code migration (or confirm one exists)

Change summary

  • Update all API groups from vteam.ambient-code to acp.ambient-code
  • Update namespaces from vteam-dev to acp-dev
  • Update container images from vteam_* to acp_*
  • Update routes/URLs from vteam-frontend/backend to acp-frontend/backend
  • Replace 'vTeam' product name with 'Ambient Code Platform' or 'ACP'
  • Add new Mermaid architecture diagrams to docs/
  • Update components/README.md with diagram references

Affected areas:

  • User guides and getting started documentation
  • GitLab integration documentation (5 files)
  • Development and testing documentation
  • Build metadata and deployment docs
  • Design documents and ADRs
  • Implementation plans
  • Reference documentation and API docs
  • Lab exercises

Total: 27 modified files, 4 new diagram files
All vteam references removed from platform/docs/

- Update all API groups from vteam.ambient-code to acp.ambient-code
- Update namespaces from vteam-dev to acp-dev
- Update container images from vteam_* to acp_*
- Update routes/URLs from vteam-frontend/backend to acp-frontend/backend
- Replace 'vTeam' product name with 'Ambient Code Platform' or 'ACP'
- Add new Mermaid architecture diagrams to docs/
- Update components/README.md with diagram references

Affected areas:
- User guides and getting started documentation
- GitLab integration documentation (5 files)
- Development and testing documentation
- Build metadata and deployment docs
- Design documents and ADRs
- Implementation plans
- Reference documentation and API docs
- Lab exercises

Total: 27 modified files, 4 new diagram files
All vteam references removed from platform/docs/
@bobbravo2 bobbravo2 added this to the v0.0.15 milestone Dec 18, 2025
@bobbravo2 bobbravo2 added documentation Improvements or additions to documentation good first issue Good for newcomers labels Dec 18, 2025
@github-actions

This comment has been minimized.

@bobbravo2 bobbravo2 removed the good first issue Good for newcomers label Dec 18, 2025
@bobbravo2 bobbravo2 marked this pull request as ready for review January 5, 2026 13:44
@Gkrumbach07
Copy link
Collaborator

looks good, but is the crd planned to be migrated after this name change

But the actual CRD files in components/manifests/ still use vteam.ambient-code.

@bobbravo2
Copy link
Contributor Author

looks good, but is the crd planned to be migrated after this name change

But the actual CRD files in components/manifests/ still use vteam.ambient-code.

I think that's wise. Docs first, less risk of breaking infra this way.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

Claude Code Review

Summary

This PR successfully renames vTeam references to "Ambient Code Platform" (ACP) across documentation while adding new Mermaid architecture diagrams. The changes are primarily documentation updates with no code modifications, making this a low-risk refactoring.

Scope: 31 files changed (27 modified, 4 new diagrams)

  • ✅ Documentation updates (user guides, API docs, GitLab integration)
  • ✅ New Mermaid architecture diagrams
  • ✅ Consistent naming convention throughout
  • ⚠️ Important: Code migration not included (noted in CLAUDE.md)

Issues by Severity

🚫 Blocker Issues

None - This is a documentation-only PR with no blockers.


🔴 Critical Issues

None - Documentation changes are straightforward and well-executed.


🟡 Major Issues

1. Code Migration Not Addressed

Location: This PR updates docs only, but code still uses vteam naming.

Impact: Potential confusion between documentation (ACP) and actual system behavior (vteam).

Current State (per CLAUDE.md):

> **Note:** This project was formerly known as "vTeam". Technical artifacts 
> (image names, namespaces, API groups) still use "vteam" for backward compatibility.

Recommendation:

  • ✅ Current approach is acceptable if documented
  • Create tracking issue for code migration:
    • API groups: vteam.ambient-codeacp.ambient-code
    • Namespaces: vteam-devacp-dev
    • Images: vteam_*acp_*
    • Routes: vteam-frontendacp-frontend
  • Consider migration impact on existing deployments
  • Add migration guide for operators

🔵 Minor Issues

1. Mermaid Diagram Accessibility

Location: New diagram files (.mmd)

Issue: Mermaid diagrams may not render in all contexts (GitHub web, IDEs without plugins).

Evidence:

  • docs/platform-architecture.mmd: Uses dark theme
  • docs/agentic-session-flow.mmd: Uses dark theme
  • docs/component-structure.mmd: Uses base theme
  • docs/deployment-stack.mmd: Uses base theme

Recommendations:

  1. ✅ Add HTML comment with plain-text description for accessibility
  2. Consider adding README.md in docs/ explaining how to view diagrams
  3. Test rendering in:
    • GitHub web interface
    • MkDocs (if documentation uses it)
    • VS Code with Mermaid preview

Example addition:

<!-- 
Plain-text flow: User → Frontend → Backend → Operator → Job → Pod → Results
For best viewing: Use Mermaid preview or render at mermaid.live
-->

2. Incomplete URL Updates in Some Files

Location: docs/gitlab-integration.md, docs/gitlab-token-setup.md

Issue: While most vteam references updated, need to verify all example URLs changed.

Found in PR diff: URLs properly updated from vteam-backend to acp-backend

# Before
curl -X POST http://vteam-backend:8080/api/auth/gitlab/connect

# After  
curl -X POST http://acp-backend:8080/api/auth/gitlab/connect

Status: ✅ Appears complete, but recommend spot-check during mkdocs serve testing.

3. Testing Checklist Partially Complete

Location: PR description

Checklist items:

- [ ] Run mkdocs serve and verify all 4 new diagrams render correctly
- [ ] Spot-check 2-3 updated doc pages in browser
- [ ] Verify components/README.md diagram links work
- [ ] Update CLAUDE.md migration status note (or create follow-up issue)
- [ ] Create tracking issue for code migration (or confirm one exists)

Recommendation: Complete these before merge to ensure quality.


Positive Highlights

🌟 Excellent Documentation Structure

  1. New Mermaid Diagrams - High quality architectural visualizations:

    • platform-architecture.mmd: Clear system overview
    • agentic-session-flow.mmd: Detailed sequence diagram
    • component-structure.mmd: Development workflow guide
    • deployment-stack.mmd: Technology stack overview
  2. Comprehensive Coverage:

    • Updated 27 files systematically
    • GitLab integration docs (5 files) thoroughly updated
    • API endpoints, examples, and error messages all consistent
  3. Backward Compatibility Note:

    • CLAUDE.md clearly documents the vteam → ACP transition
    • Explains why code still uses vteam naming
  4. Consistent Naming Convention:

    • Product name: "Ambient Code Platform" or "ACP"
    • No mixing of old/new terminology within updated files

🔧 Good Engineering Practices

  1. Focused PR: Documentation-only change, no code mixed in
  2. Clear PR Description: Lists all affected areas
  3. Pre-merge Checklist: Testing steps defined
  4. Diagram Integration: Added diagram references to components/README.md

Recommendations

Priority 1 (Before Merge)

  1. Complete Testing Checklist:

    # Test diagrams render
    mkdocs serve
    # Open http://127.0.0.1:8000
    # Check all 4 new diagrams load
  2. Verify Diagram Links: Test links in components/README.md:

    • platform-architecture.mmd
    • component-structure.mmd
    • agentic-session-flow.mmd
    • deployment-stack.mmd
  3. Create Migration Tracking Issue: Document code migration plan

    • API groups, namespaces, image names, routes
    • Impact on existing deployments
    • Migration timeline
    • Link issue in CLAUDE.md

Priority 2 (Post-Merge)

  1. Add Mermaid Viewing Guide:

    # docs/README.md
    ## Viewing Architecture Diagrams
    
    Mermaid diagrams (`.mmd` files) can be viewed:
    - GitHub web (native support)
    - VS Code: Install "Mermaid Preview" extension
    - Online: Copy to https://mermaid.live
    - MkDocs: Diagrams render automatically
  2. Accessibility Enhancement: Add plain-text flow descriptions to diagrams

  3. Monitor for Missed References:

    # Search for any remaining vteam references in docs
    grep -r "vteam" docs/ --exclude="*.mmd" --exclude="adr/*"

Priority 3 (Future Work)

  1. Code Migration (separate PR):

    • Update CRD API groups
    • Update namespaces
    • Update container image names
    • Update routes
    • Migration script for existing deployments
  2. Documentation Site: If using MkDocs, configure Mermaid plugin:

    # mkdocs.yml
    markdown_extensions:
      - pymdownx.superfences:
          custom_fences:
            - name: mermaid
              class: mermaid
              format: !!python/name:pymdownx.superfences.fence_code_format

Compliance Check

✅ Follows CLAUDE.md Standards

  • Documentation updates follow colocated documentation pattern
  • Improves existing docs rather than proliferating new files
  • Cross-cutting concerns (architecture) documented appropriately
  • No code changes = no security/auth/error-handling concerns

✅ No Security Issues

  • Documentation-only PR
  • No tokens, credentials, or sensitive data
  • No code execution paths modified

✅ Architecture Alignment

  • Diagrams accurately reflect system described in CLAUDE.md
  • Component descriptions match actual implementation
  • Technology stack correctly documented

Final Verdict

✅ APPROVE with minor recommendations

This PR successfully achieves its goal of renaming vTeam to Ambient Code Platform in documentation. The new Mermaid diagrams are high-quality additions that improve understanding of the system architecture.

Merge Status: Safe to merge after completing testing checklist

Post-Merge Actions:

  1. Create tracking issue for code migration
  2. Update CLAUDE.md with migration status
  3. Add Mermaid viewing guide to docs

Risk Level: 🟢 LOW - Documentation-only changes with no code impact


Review Metadata

  • Reviewer: Claude Code (Sonnet 4.5)
  • Review Date: 2026-01-05
  • Standards Applied:
    • CLAUDE.md project guidelines
    • Documentation standards
    • Architecture pattern validation
  • Files Reviewed: 31 (27 modified, 4 new)
  • Lines Changed: +654/-322

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

  1. CLAUDE.md - Master project instructions, development standards
  2. backend-development.md - Go backend, K8s integration patterns
  3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
  4. security-standards.md - Auth, RBAC, token handling
  5. k8s-client-usage.md - User token vs service account patterns
  6. error-handling.md - Consistent error patterns
  7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

- Revert API group to vteam.ambient-code (maintain backward compatibility)
- Revert namespace to vteam-dev (avoid breaking deployments)
- Revert service/route names to vteam-* (maintain service discovery)
- Revert container image names to vteam-* in examples
- Keep product name changes (vTeam → ACP in narrative text)
- Keep new Mermaid architecture diagrams

Per ACP Constitution naming guidelines, technical artifacts must
remain as 'vteam' for backward compatibility. Only user-facing
narrative text uses 'Ambient Code Platform' or 'ACP'.

Fixes 24 files with breaking changes while preserving improvements.
@bobbravo2 bobbravo2 force-pushed the docs/replace-vteam-with-acp branch from 3ff93ce to 9eb2677 Compare January 5, 2026 19:46
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

Claude Code Review

Summary

This PR successfully completes the documentation rebranding from "vTeam" to "Ambient Code Platform (ACP)". The changes are documentation-only with no code modifications, making this a low-risk update. The PR adds valuable Mermaid architecture diagrams that significantly enhance documentation quality.

Overall Assessment: ✅ Approved with minor recommendations


Issues by Severity

🚫 Blocker Issues

None - This is a documentation-only change with no blockers.


🔴 Critical Issues

1. Inconsistent vteam references in component-structure.mmd

Location: docs/component-structure.mmd:33

Issue: The diagram still references the old URL pattern:

FrontendURL[Frontend URL<br/>https://vteam-frontend-vteam-dev.apps-crc.testing]

Should be:

FrontendURL[Frontend URL<br/>https://acp-frontend-acp-dev.apps-crc.testing]

Impact: This contradicts the PR's stated goal of removing all vteam references from docs/. Users following the diagram will expect the wrong URL.

Recommendation: Update line 33 to use acp-frontend-acp-dev for consistency.


🟡 Major Issues

1. CLAUDE.md migration status needs update

Location: PR checklist item not addressed

Issue: The PR checklist states:

- [ ] Update CLAUDE.md migration status note (or create follow-up issue)

This was not completed. CLAUDE.md still references:

  • "This project was formerly known as 'vTeam'. Technical artifacts (image names, namespaces, API groups) still use 'vteam' for backward compatibility."

Recommendation: After this PR merges, update CLAUDE.md to clarify:

  • Docs have been fully migrated to ACP naming
  • Code migration is tracked in [issue link]
  • Timeline for code migration (if known)

2. Missing tracking issue for code migration

Location: PR checklist

Issue: Checklist item:

- [ ] Create tracking issue for code migration (or confirm one exists)

Recommendation: Before merging, either:

  • Create a tracking issue for migrating code artifacts (CRDs, namespaces, image names, routes)
  • Reference an existing issue if one already exists
  • Link it in the PR description for visibility

🔵 Minor Issues

1. .gitignore changes unrelated to PR scope

Location: .gitignore:139-141

Issue: Added security scan artifacts:

# Security scan artifacts (transient)
.security-scan/
.security-scan.zip

While not harmful, this is scope creep for a "docs rebranding" PR. This type of change should be in a separate commit for better git history.

Recommendation: Either:

  • Revert this change and submit separately
  • Or keep it (low impact) and note in commit message why it was included

2. Diagram accessibility considerations

Location: All 4 new .mmd files

Issue: Mermaid diagrams use emojis and color-coding for visual clarity, but:

  • Screen readers may not handle emojis well
  • Color-only differentiation (without shape/pattern) may affect colorblind users

Recommendation (Future enhancement):

  • Consider adding alt-text descriptions in markdown when embedding diagrams
  • Test diagram rendering with screen readers
  • Ensure diagrams remain usable when printed in grayscale

Note: This is a nice-to-have improvement, not a blocker.


3. Inconsistent comment style in SECURITY_DEV_MODE.md

Location: docs/SECURITY_DEV_MODE.md:112

Before:

"vteam-dev",       // Legacy local dev namespace

After:

"vteam-dev",         // Local dev namespace

Issue: Removed "Legacy" but code still uses vteam-dev namespace (not renamed to acp-dev). Comment is now misleading - it's not "local dev namespace", it's "legacy local dev namespace" (since acp-dev should be the new one).

Recommendation: Restore "Legacy" in the comment until the code is actually migrated:

"vteam-dev",       // Legacy local dev namespace (migrate to acp-dev)

Positive Highlights

Excellent architecture diagrams - The 4 new Mermaid diagrams (platform-architecture.mmd, component-structure.mmd, agentic-session-flow.mmd, deployment-stack.mmd) significantly improve documentation quality.

Comprehensive coverage - 27 files updated, covering user guides, GitLab integration, development docs, and API references. Very thorough.

Consistent naming convention - Replaced "vTeam" with "Ambient Code Platform" and "ACP" consistently throughout.

No code changes - Low risk: this is purely documentation, no behavior changes.

Clear separation of concerns - Correctly preserves backward compatibility in code (vteam_ prefixes remain) while modernizing docs.

Good diagram organization - Diagrams are colocated in docs/ and referenced from components/README.md, making them discoverable.


Recommendations

Before Merge (Critical Path)

  1. Fix component-structure.mmd URL (line 33) - Change vteam-frontend-vteam-dev to acp-frontend-acp-dev
  2. Test diagram rendering - Run mkdocs serve and verify all 4 diagrams display correctly (as per PR checklist)
  3. Create/link migration tracking issue - Document the plan for code artifact migration
  4. Restore "Legacy" comment in SECURITY_DEV_MODE.md:112

After Merge (Follow-up)

  1. Update CLAUDE.md migration status - Add note about docs migration completion and code migration timeline
  2. Consider diagram accessibility - Add alt-text or descriptions for screen reader users
  3. Clean up .gitignore change - If unrelated, consider reverting in a follow-up

Testing Validation

Per the PR checklist, ensure these are completed before merge:

  • Run mkdocs serve and verify all 4 new diagrams render correctly
  • Spot-check 2-3 updated doc pages in browser (especially GitLab integration docs)
  • Verify components/README.md diagram links work
  • Update CLAUDE.md migration status note
  • Create tracking issue for code migration

Architecture & Code Quality Assessment

Does it follow CLAUDE.md patterns? ✅ Yes

  • This is documentation-only, no code patterns to evaluate
  • Follows documentation standards (colocated docs, improve existing files)

Security concerns? ✅ None

  • No code changes
  • No secrets exposed
  • GitLab docs correctly reference token handling

Performance impact? ✅ None

  • Documentation changes have no runtime impact

Testing coverage? ✅ N/A

  • No code changes to test
  • Diagrams should be manually verified (per checklist)

Final Verdict

Recommendation: ✅ Approve after addressing critical issue #1

This is an excellent documentation improvement that modernizes the project branding. The new Mermaid diagrams are a significant value-add. The only blocking issue is the inconsistent URL in component-structure.mmd.

Once the vteam URL is fixed and the migration tracking issue is created/linked, this PR is ready to merge.

Great work on the thorough rebranding effort! 🎉

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

Claude Code Review

Summary

This PR performs a documentation rename from "vTeam" to "Ambient Code Platform (ACP)" and adds new Mermaid architecture diagrams. While the overall intent is good and most changes are correct, there are BLOCKER ISSUES that must be fixed before merge.

Overall Assessment: Documentation-only changes with good architecture diagrams, but incomplete migration and accidental inclusion of security scan artifacts.

Issues by Severity

🚫 Blocker Issues

1. Security Scan Artifacts Accidentally Committed

Location: .security-scan/ directory (156KB, 5 files)
Issue: The entire .security-scan/ directory was accidentally committed to the PR, including:

  • security-scan-2025-12-18-113830/repo-analysis.md (114 additions)
  • security-scan-2025-12-18-113830/scan-metadata.json (3,588 additions!)
  • security-scan-2025-12-18-113830/scan-summary.md (107 additions)
  • security-scan-2025-12-18-113830/security-report.md (660 additions)
  • .security-scan.zip

Impact:

  • Inflates PR size (4,998 additions, but 4,469 are from security scan)
  • Contains potentially sensitive security analysis
  • Not relevant to documentation rename
  • Should be in .gitignore

Fix Required:

# Remove from PR
git rm -r .security-scan/ .security-scan.zip
git commit -m "Remove accidentally committed security scan artifacts"

# Add to .gitignore
echo ".security-scan/" >> .gitignore
echo ".security-scan.zip" >> .gitignore
git commit -m "gitignore: Add .security-scan artifacts"

2. Incomplete Migration - Old "vteam" References Still Present

Locations:

  1. docs/component-structure.mmd:32 - Old frontend URL still uses vteam-frontend-vteam-dev:

    + FrontendURL[Frontend URL<br/>https://vteam-frontend-vteam-dev.apps-crc.testing]

    Should be: https://acp-frontend-acp-dev.apps-crc.testing

  2. docs/gitlab-testing-procedures.md - Multiple kubectl commands still reference old namespace/labels:

    + kubectl logs -l app=vteam-backend -n vteam-backend

    Should be: kubectl logs -l app=acp-backend -n acp-backend (appears in 2 locations)

  3. docs/SECURITY_DEV_MODE.md:112 - Comment says "Local dev namespace" but still uses vteam-dev:

    + "vteam-dev",         // Local dev namespace

    Should be: "acp-dev", // Local dev namespace

  4. docs/gitlab-testing-procedures.md - Broken GitHub URL with literal "Ambient Code Platform" in path:

    + - Ambient Code Platform GitHub Issues: https://github.com/natifridman/Ambient Code Platform/issues

    Should be: https://github.com/ambient-code/platform/issues

Impact:

  • Inconsistent documentation will confuse users
  • Commands will fail if users copy-paste them
  • Broken GitHub URL won't resolve

Fix Required: Update all 4 locations above to use correct ACP naming.

🔴 Critical Issues

None identified (after blockers are fixed).

🟡 Major Issues

1. PR Testing Checklist Not Completed

Location: PR description
Issue: The PR includes a testing checklist with 5 unchecked items, but PR is ready for review:

- [ ] Run `mkdocs serve` and verify all 4 new diagrams render correctly
- [ ] Spot-check 2-3 updated doc pages in browser
- [ ] Verify `components/README.md` diagram links work
- [ ] Update CLAUDE.md migration status note (or create follow-up issue)
- [ ] Create tracking issue for code migration (or confirm one exists)

Recommendation:

  • Complete checklist before requesting review, OR
  • Remove checklist if already completed, OR
  • Mark as draft until testing is done

2. Code Migration Tracking Needed

Issue: This PR only updates documentation (docs/ and components/README.md). The actual codebase still uses:

  • API group: vteam.ambient-code → should be acp.ambient-code
  • Namespaces: vteam-dev → should be acp-dev
  • Container images: vteam_backend, vteam_frontend, etc. → should be acp_*
  • Routes: vteam-frontend, vteam-backend → should be acp-*

From CLAUDE.md (lines 9, 333, 335):

> **Note:** This project was formerly known as "vTeam". Technical artifacts (image names, 
> namespaces, API groups) still use "vteam" for backward compatibility.

- **Default namespace**: `ambient-code` (production), `vteam-dev` (local dev)
- **CRD group**: `vteam.ambient-code`

Recommendation:

  • Create a tracking issue for code migration (as suggested in PR checklist)
  • Link it in the PR description
  • Consider updating CLAUDE.md note to explicitly mention docs are updated but code migration is pending

🔵 Minor Issues

1. Mermaid Diagram Quality - Good Addition

Positive: The new architecture diagrams are well-structured:

  • docs/platform-architecture.mmd - Clean high-level architecture
  • docs/agentic-session-flow.mmd - Detailed sequence diagram (57 lines, very thorough)
  • docs/component-structure.mmd - Development workflow
  • docs/deployment-stack.mmd - Technology stack

Minor suggestion: Consider adding a diagram index page (docs/diagrams/README.md) if more diagrams are planned.

2. Documentation Scope Appropriate

Positive: The PR correctly limits scope to documentation:

  • 17 actual documentation files changed (excluding security scan)
  • Consistent renaming pattern throughout
  • No code changes (appropriate for docs-only PR)

Positive Highlights

Comprehensive Documentation Coverage: Updated 27 files across all doc categories (user guides, API docs, testing, GitLab integration, ADRs, labs)

Architecture Diagrams Added: Four new Mermaid diagrams provide excellent visual documentation of:

  • Platform architecture and data flow
  • Component structure and development workflow
  • Agentic session execution sequence
  • Deployment stack and technology choices

Consistent Naming Pattern: Most replacements follow a clear pattern:

  • vTeamAmbient Code Platform or ACP
  • vteam.ambient-codeacp.ambient-code
  • vteam-devacp-dev (where updated)
  • vteam_backendacp_backend

GitLab Integration Docs Updated: All 5 GitLab documentation files updated consistently

Follows Project Standards:

  • No code changes (appropriate for docs PR)
  • Documentation colocated appropriately
  • Mermaid diagrams follow established patterns

Recommendations

Priority 1 (Must Fix Before Merge)

  1. Remove security scan artifacts from PR (.security-scan/, .security-scan.zip)
  2. Add .security-scan/ to .gitignore
  3. Fix incomplete vteam→ACP migrations (4 locations identified above)
  4. Fix broken GitHub URL in gitlab-testing-procedures.md

Priority 2 (Should Do)

  1. Complete or remove PR testing checklist in description
  2. Create tracking issue for code migration and link in PR
  3. Update CLAUDE.md to note docs are migrated but code migration pending

Priority 3 (Nice to Have)

  1. Verify Mermaid diagrams render via mkdocs serve (from checklist)
  2. Consider adding diagram index page if more diagrams planned

Validation Checklist

Based on loaded memory system files:

Documentation Standards (CLAUDE.md:1070-1075):

  • ✅ Improving existing documentation (not creating unnecessary new files)
  • ✅ Following established patterns
  • ⚠️ Some docs not fully consistent (vteam refs remain)

Security Standards:

  • ⚠️ Security scan artifacts should not be committed
  • ✅ No code changes, so no security implications

Project Structure:

  • ✅ Documentation colocated appropriately (docs/, not top-level)
  • ✅ Component docs updated (components/README.md)

Decision: ❌ Do not merge until blocker issues are fixed.

After fixes, this will be a valuable documentation update that aligns naming with the project's Ambient Code Platform branding while adding helpful architecture diagrams.


🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

  1. CLAUDE.md - Master project instructions, development standards
  2. backend-development.md - Go backend, K8s integration patterns
  3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
  4. security-standards.md - Auth, RBAC, token handling
  5. k8s-client-usage.md - User token vs service account patterns
  6. error-handling.md - Consistent error patterns
  7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

- Fix incorrect URL: github.com/natifridman/Ambient Code Platform/issues
  → https://github.com/ambient-code/platform/issues
- Replace placeholder: github.com/your-org/platform
  → https://github.com/ambient-code/platform

Updated files:
- docs/gitlab-self-hosted.md (incorrect org/repo name)
- docs/user-guide/working-with-amber.md (placeholder URLs)
Copy link
Contributor Author

@bobbravo2 bobbravo2 left a comment

Choose a reason for hiding this comment

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

✅ Fixed all infra breaking changes:
Reverted acp.ambient-code → vteam.ambient-code (API group)
Reverted acp-dev → vteam-dev (namespace)
Reverted acp-backend, acp-frontend, acp-operator → vteam-* (services/routes)
Reverted container image names in examples
✅ Kept the good changes:
Product name updates (vTeam → Ambient Code Platform/ACP in narrative text)
New Mermaid architecture diagrams (4 new diagram files)

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

Claude Code Review

Summary

This PR successfully migrates documentation from 'vteam' to 'ACP' (Ambient Code Platform) naming and adds valuable Mermaid architecture diagrams. The changes are documentation-only with no code modifications, making this a low-risk, high-value update that improves project clarity and professionalism.

Overall Assessment: ✅ Approved with minor recommendations


Issues by Severity

🚫 Blocker Issues

None - This PR is safe to merge.


🔴 Critical Issues

None - All changes are documentation-only with no security, functionality, or architecture concerns.


🟡 Major Issues

1. Inconsistent URL in component-structure.mmd

Location: docs/component-structure.mmd:33

FrontendURL[Frontend URL<br/>https://vteam-frontend-vteam-dev.apps-crc.testing]

Issue: This diagram still references vteam-frontend-vteam-dev instead of the new acp-frontend-acp-dev naming.

Expected:

FrontendURL[Frontend URL<br/>https://acp-frontend-acp-dev.apps-crc.testing]

Impact: Misleads users about actual development URL pattern.

Recommendation: Update to acp-frontend-acp-dev for consistency with the migration.


2. Incomplete migration check needed

Location: PR body mentions "Update CLAUDE.md migration status note" but CLAUDE.md wasn't modified in this PR.

Issue: CLAUDE.md still contains references to "vteam" technical artifacts as "legacy" naming, but doesn't mention this documentation migration or when code migration is expected.

Current state in CLAUDE.md (line 9):

> **Note:** This project was formerly known as "vTeam". Technical artifacts (image names, namespaces, API groups) still use "vteam" for backward compatibility.

Recommendation: Either:

  1. Update this note to clarify that docs now use ACP naming while code migration is planned, OR
  2. Create a tracking issue for code migration and reference it in CLAUDE.md

🔵 Minor Issues

1. .gitignore additions seem unrelated

Location: .gitignore:139-142

# Security scan artifacts (transient)
.security-scan/
.security-scan.zip

Issue: These security scan entries appear unrelated to the vteam→ACP migration. While not harmful, they should ideally be in a separate commit/PR for cleaner git history.

Recommendation: Consider moving to a separate PR, or document why they're included here.


2. SECURITY_DEV_MODE.md comment inconsistency

Location: docs/SECURITY_DEV_MODE.md:112

Before:

"vteam-dev",       // Legacy local dev namespace

After:

"vteam-dev",         // Local dev namespace

Issue: Removed "Legacy" but the actual namespace name is still vteam-dev, not acp-dev. The comment change is misleading since the code still uses vteam naming.

Recommendation: Either:

  1. Keep "Legacy" in the comment to be accurate, OR
  2. Add a note like "// Local dev namespace (vteam naming retained for backward compatibility)"

Positive Highlights

🌟 Excellent Documentation Enhancements

  1. Outstanding Mermaid Diagrams: The four new architecture diagrams are exceptionally well-crafted:

    • platform-architecture.mmd: Clear visual representation of the entire stack
    • agentic-session-flow.mmd: Detailed sequence diagram showing exact flow
    • component-structure.mmd: Developer-focused directory structure view
    • deployment-stack.mmd: Comprehensive technology stack overview

    These diagrams will significantly improve onboarding and system understanding. 👏

  2. Comprehensive Migration Coverage: Changed 27 files across user guides, API docs, testing docs, design documents, and lab exercises. Very thorough!

  3. Consistent Terminology: Replaced "vTeam" with proper "Ambient Code Platform" or "ACP" acronym throughout, maintaining professional branding.

  4. Improved Navigation: Added diagram links to components/README.md, making architecture documentation discoverable.

📊 Quality Metrics

  • Scope: Documentation-only (zero code changes)
  • Risk Level: Very Low
  • Test Coverage: Mentioned in PR checklist (mkdocs serve verification)
  • Backward Compatibility: No breaking changes (code still uses vteam naming)

Recommendations

Priority 1: Fix Before Merge

  1. Update component-structure.mmd URL to use acp-frontend-acp-dev pattern (line 33)
  2. Verify all diagrams render in mkdocs as mentioned in your checklist

Priority 2: Consider for Follow-up

  1. Update CLAUDE.md with migration status clarification
  2. Create tracking issue for actual code migration (API groups, namespaces, image names)
  3. Document security scan .gitignore additions or move to separate PR
  4. Review SECURITY_DEV_MODE.md comment for accuracy

Priority 3: Enhancement Opportunities

  1. Consider adding a migration guide document explaining:

    • What has been migrated (docs)
    • What hasn't (code artifacts)
    • Timeline for code migration
    • Impact on existing deployments
  2. Add diagram rendering validation to CI/CD (mkdocs build check)


Security Assessment

No security concerns

  • Documentation changes only
  • No token handling modifications
  • No authentication/authorization changes
  • No secrets or credentials exposed
  • No API endpoint changes

Testing Checklist Validation

Your PR checklist mentions:

  • Run mkdocs serve and verify all 4 new diagrams render correctly
  • Spot-check 2-3 updated doc pages in browser
  • Verify components/README.md diagram links work
  • Update CLAUDE.md migration status note (or create follow-up issue)
  • Create tracking issue for code migration (or confirm one exists)

Recommendation: Complete items 4-5 before merge, rest can be done during final review.


Final Verdict

Status: ✅ APPROVED (with minor fixes recommended)

This is excellent work that significantly improves documentation quality and consistency. The Mermaid diagrams are production-ready and will be invaluable for new developers and stakeholders.

Merge Readiness: 95%

  • Fix the one URL inconsistency in component-structure.mmd
  • Verify diagrams render in mkdocs
  • Optionally address CLAUDE.md migration note

Impact: High positive impact on developer experience and project professionalism with minimal risk.

Great job! 🎉


Reviewed by Claude Code (Sonnet 4.5) using project memory system and security standards


🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

  1. CLAUDE.md - Master project instructions, development standards
  2. backend-development.md - Go backend, K8s integration patterns
  3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
  4. security-standards.md - Auth, RBAC, token handling
  5. k8s-client-usage.md - User token vs service account patterns
  6. error-handling.md - Consistent error patterns
  7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

Add note that routes also use 'vteam' naming and that documentation
uses ACP naming. This clarifies the distinction between technical
artifacts (which remain vteam) and user-facing documentation.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2026

Claude Code Review

Summary

This PR successfully migrates documentation from "vTeam" branding to "Ambient Code Platform" (ACP) naming conventions and adds valuable Mermaid architecture diagrams. The changes are documentation-only with no code modifications, making this a low-risk update. The documentation updates are consistent, comprehensive, and well-executed.

Overall Assessment: ✅ Approved with Minor Recommendations

The PR is ready to merge after addressing the pre-merge checklist items. All changes follow documentation standards and maintain consistency with the project's transition strategy.


Issues by Severity

🚫 Blocker Issues

None - This is a documentation-only PR with no blockers.


🔴 Critical Issues

None - All critical documentation patterns are followed correctly.


🟡 Major Issues

1. Incomplete Migration Tracking

Location: CLAUDE.md:9
Issue: The note about backward compatibility mentions "Technical artifacts (image names, namespaces, API groups, routes) still use 'vteam' for backward compatibility" but doesn't provide a migration timeline or tracking.

Current:

> **Note:** This project was formerly known as "vTeam". Technical artifacts (image names, namespaces, API groups, routes) still use "vteam" for backward compatibility. The docs use ACP naming.

Recommendation:
Either update CLAUDE.md with migration status or create a tracking issue as noted in the PR checklist. Consider adding:

> **Note:** This project was formerly known as "vTeam". Technical artifacts (image names, namespaces, API groups, routes) still use "vteam" for backward compatibility. The docs use ACP naming. See [Issue #XXX] for code migration tracking.

2. Mixed URL References in Documentation

Location: Multiple files (e.g., docs/api/gitlab-endpoints.md, docs/gitlab-integration.md)
Issue: Documentation consistently uses vteam-backend, vteam-frontend URLs in examples, which conflicts with the ACP naming in descriptive text.

Examples:

  • http://vteam-backend:8080/api (appears 40+ times)
  • https://vteam-frontend-vteam-dev.apps-crc.testing

Recommendation: This is correct per the backward compatibility note, but consider adding a prominent callout in affected docs:

> **Note**: API endpoints and routes still use `vteam-*` naming for backward compatibility. Future releases will migrate to `acp-*` naming.

🔵 Minor Issues

1. .gitignore Security Scan Artifacts

Location: .gitignore:140-142
Issue: New security scan exclusions added (.security-scan/, .security-scan.zip) are good, but this appears unrelated to the PR's stated purpose (docs migration).

Recommendation: Consider:

  • Moving to a separate housekeeping PR, OR
  • Adding to PR description that this is an opportunistic cleanup

2. Diagram File Naming Convention

Location: docs/*.mmd files
Issue: New Mermaid files use kebab-case (agentic-session-flow.mmd), which is good, but inconsistent with some existing docs that might use different conventions.

Recommendation: Verify this aligns with the documentation standards in CLAUDE.md:1069-1075. The pattern looks correct - files are colocated in docs/ for cross-cutting concerns.

3. Diagram Complexity

Location: docs/deployment-stack.mmd
Issue: The deployment stack diagram is quite large (125 lines) and complex. May be difficult to render or navigate.

Recommendation:

  • Test rendering in MkDocs (as per checklist)
  • Consider splitting into multiple focused diagrams if rendering is slow
  • Ensure it's readable at different zoom levels

Positive Highlights

✅ Excellent Documentation Quality

  1. Comprehensive Coverage: All 27 files updated consistently with no missed references

  2. Architectural Diagrams: Four new Mermaid diagrams provide excellent visual documentation:

    • platform-architecture.mmd - Clean, clear architecture overview
    • agentic-session-flow.mmd - Detailed sequence diagram with good use of notes
    • component-structure.mmd - Helpful development workflow visualization
    • deployment-stack.mmd - Comprehensive tech stack overview
  3. Backward Compatibility Awareness: Correctly preserves technical artifact names while updating product branding

  4. Consistent Terminology: "Ambient Code Platform" and "ACP" used appropriately throughout

  5. Documentation Standards: Follows CLAUDE.md guidance on documentation organization

✅ Well-Structured Changes

  1. Logical Grouping: Changes organized by documentation category (guides, GitLab integration, testing, etc.)
  2. No Code Changes: Maintains separation between docs and code, reducing risk
  3. GitLab Integration Docs: Comprehensive updates to 5 GitLab files show attention to detail
  4. Testing Documentation: Updates to e2e testing guide maintain accuracy

✅ Diagram Excellence

agentic-session-flow.mmd:

  • Clear participant definitions with emojis for visual identification
  • Proper use of sequence diagram notes for context
  • Good flow representation of the async execution model

platform-architecture.mmd:

  • Numbered flow steps make the architecture easy to follow
  • Color-coded component layers
  • Clear separation of concerns

Recommendations

Pre-Merge Checklist Compliance

Ensure the PR checklist items are completed:

  • Run mkdocs serve and verify all 4 new diagrams render correctly
  • Spot-check 2-3 updated doc pages in browser
  • Verify components/README.md diagram links work
  • Update CLAUDE.md migration status note (or create follow-up issue)
  • Create tracking issue for code migration (or confirm one exists)

Documentation Testing

Since this is a docs-only PR, recommend:

  1. Build MkDocs locally: mkdocs build
  2. Check for any broken internal links
  3. Verify Mermaid diagrams render in GitHub's preview
  4. Test diagrams at different screen sizes

Follow-Up Work

Consider creating issues for:

  1. Code Migration: Track the actual migration of technical artifacts (image names, namespaces, API groups, routes) from vteam to acp naming
  2. URL Migration: Plan for migrating vteam-* URLs to acp-* with proper deprecation notices
  3. Diagram Maintenance: Establish process for keeping architecture diagrams up-to-date as code evolves

Alignment with Project Standards

✅ Follows CLAUDE.md Guidelines

Per CLAUDE.md:1069-1075:

Default to improving existing documentation rather than creating new files.

Assessment: The PR correctly:

  • Improves existing documentation files (27 modified)
  • Adds new diagrams in docs/ for cross-cutting architectural concerns
  • Updates components/README.md with diagram references (colocated documentation)
  • Follows the pattern of docs/adr/ for architectural documentation

✅ No Security Concerns

  • Documentation-only changes pose no security risk
  • No exposure of credentials, tokens, or sensitive data
  • Maintains security documentation accuracy (docs/SECURITY_DEV_MODE.md)

✅ No Performance Impact

  • Static documentation changes
  • Mermaid diagrams are rendered client-side, no backend impact

Final Recommendation

Approve and merge after completing the pre-merge checklist.

Suggested merge order:

  1. Complete checklist items (especially diagram rendering verification)
  2. Create tracking issue for code migration (if not already exists)
  3. Squash and merge with commit message:
    docs: Replace vteam references with ACP naming
    
    - Update all documentation from vTeam to Ambient Code Platform (ACP)
    - Add 4 new Mermaid architecture diagrams
    - Maintain backward compatibility for technical artifacts
    - Update GitLab integration documentation (5 files)
    - Update testing and deployment documentation
    
    Total: 27 modified files, 4 new diagram files
    

Post-merge:

  • Monitor MkDocs deployment for diagram rendering
  • Update any external documentation that links to these files
  • Begin work on code migration tracking issue

Reviewed by: Claude Code (Sonnet 4.5)
Review Date: 2026-01-05
Standards Checked: CLAUDE.md, backend-development.md, frontend-development.md, security-standards.md


🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

  1. CLAUDE.md - Master project instructions, development standards
  2. backend-development.md - Go backend, K8s integration patterns
  3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
  4. security-standards.md - Auth, RBAC, token handling
  5. k8s-client-usage.md - User token vs service account patterns
  6. error-handling.md - Consistent error patterns
  7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

Copy link
Collaborator

@MichaelClifford MichaelClifford left a comment

Choose a reason for hiding this comment

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

LGTM

@Gkrumbach07 Gkrumbach07 merged commit d4f996e into ambient-code:main Jan 5, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants