Skip to content

Conversation

@marcorosa
Copy link
Member

Added

Changed

  • Replace the old generative-ai-hub-sdk library with the new sap-ai-gen-sdk
  • Backend connection is injected at frontend startup (instead of statically linked at build-time)

Minor

  • Several maintenance tasks (new gha for docker images builds, AI Pr bot, fixed eslint)

Caroline BANCHEREAU and others added 30 commits July 29, 2025 11:30
aligned model file to add cascade delete
Bumps [requests](https://github.com/psf/requests) from 2.32.4 to 2.32.5.
- [Release notes](https://github.com/psf/requests/releases)
- [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md)
- [Commits](psf/requests@v2.32.4...v2.32.5)

---
updated-dependencies:
- dependency-name: requests
  dependency-version: 2.32.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [flask](https://github.com/pallets/flask) from 3.1.1 to 3.1.2.
- [Release notes](https://github.com/pallets/flask/releases)
- [Changelog](https://github.com/pallets/flask/blob/main/CHANGES.rst)
- [Commits](pallets/flask@3.1.1...3.1.2)

---
updated-dependencies:
- dependency-name: flask
  dependency-version: 3.1.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
These models will disappear soon (i.e., in less than 1 month) from AI
Core as they are being retired by their providers (according to the
official notes).
gpt-4 is being retired in one week. gpt-4o is its natural replacement.
Replace SAP library for accessing LLMs
…flask-3.1.2

Bump flask from 3.1.1 to 3.1.2 in /backend-agent
…requests-2.32.5

Bump requests from 2.32.4 to 2.32.5 in /backend-agent
Bumps [flask-cors](https://github.com/corydolphin/flask-cors) from 6.0.0 to 6.0.1.
- [Release notes](https://github.com/corydolphin/flask-cors/releases)
- [Changelog](https://github.com/corydolphin/flask-cors/blob/main/CHANGELOG.md)
- [Commits](corydolphin/flask-cors@6.0.0...6.0.1)

---
updated-dependencies:
- dependency-name: flask-cors
  dependency-version: 6.0.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…flask-cors-6.0.1

Bump flask-cors from 6.0.0 to 6.0.1 in /backend-agent
Add GitHub action for SAP PR bot
added agent title to fit with heatmap page
fixed some linter
This reverts commit 16f1ae4d251c6104c13e89e898907be906c147fe.
@marcorosa marcorosa requested a review from a team as a code owner September 12, 2025 09:51
@github-actions
Copy link
Contributor

The recent update introduces significant enhancements to the project. It adds automated workflows for changelog and Docker image management while refining existing workflows to leverage newer action versions. Frontend and backend updates streamline configuration, dependency management, and maintainability. Notably, support for AI-assisted PR review and a mechanism to update attack weights dynamically have been incorporated, improving user experience and project efficiency.

Walkthrough

  • Chore: Added GitHub workflows to build Docker images and AI-assisted GitHub actions for automated PR summary and review.
  • Refactor: Streamlined backend Dockerfile with new dependency management; updated Python environments for installation and linting workflows.
  • New Feature: Backend API endpoints for attacks retrieval and weight updates; frontend updates for dynamic configuration support and weight management UI.
  • Documentation: Updated README with new dependencies and installation instructions.
  • Style: Improved user interface styles in the frontend for better alignment and display of information.
  • Test: Added initial tests for frontend weight-dialog component.

Model: gpt-4o | Prompt Tokens: 22516 | Completion Tokens: 201

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Here's a collaborative code review enhanced by AI assistance. These suggestions offer helpful perspectives and potential improvements, though they're recommendations rather than requirements. You have full creative control over your code—AI simply provides additional insights to consider. Use whatever feels valuable for your project and trust your judgment on implementation decisions.
Model: anthropic--claude-4-sonnet | Prompt Tokens: 37929 | Completion Tokens: 2090

@github-actions
Copy link
Contributor

The recent changes introduce a significant number of updates, particularly aimed at improving automation, code quality, and user experience. This includes updates to workflow files for better CI/CD processes, integration of Docker management, refinement of application code by leveraging new library functionalities, and enhancements to the frontend application for more user-friendly interactions.

Walkthrough

  • New Feature: Added GitHub Actions for building and pushing Docker images, and checking version changes to trigger builds.
  • New Feature: API endpoints added for attack management, offering enhanced backend functionality.
  • New Feature: Introduced AI-assisted summaries and reviews in workflows for better code reviews.
  • Refactor: Updated backend to use uv and upgraded library dependencies, aligning with best practices.
  • Style: Polished UI and layout of frontend application for improved user interaction.
  • Chore: Updated Python and Node.js versions in CI tasks, ensuring compatibility and up-to-date environments.
  • Documentation: Updated README.md to reflect new dependencies and configurations.
  • Test: Introduced a test file for weight dialog component ensuring UI functionality is checked.

Model: gpt-4o | Prompt Tokens: 22702 | Completion Tokens: 224

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Here's a collaborative code review enhanced with AI insights. These suggestions offer helpful perspectives, though some are predictive rather than definitive. Please use what resonates with your approach and project needs. You remain the expert developer making the final choices—AI simply provides additional support along the way.

Always critique what AI says. Do not let AI replace YOUR I.
Model: anthropic--claude-4-sonnet | Prompt Tokens: 38239 | Completion Tokens: 3597

Comment on lines +44 to +45
BACKEND_VERSION=$(grep '^version = ' backend-agent/pyproject.toml | sed "s/version = '\(.*\)'/\1/")
FRONTEND_VERSION=$(grep '"version":' frontend/package.json | sed 's/.*"version": "\(.*\)".*/\1/')
Copy link
Contributor

Choose a reason for hiding this comment

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

The version extraction logic using sed is fragile and could break with different formatting. Consider using a more robust JSON/TOML parser or add validation:

# For backend version
BACKEND_VERSION=$(python -c "import tomllib; print(tomllib.load(open('backend-agent/pyproject.toml', 'rb'))['project']['version'])")
# For frontend version
FRONTEND_VERSION=$(node -p "require('./frontend/package.json').version")

Comment on lines +53 to +59
if [ "$BACKEND_VERSION" != "$PREVIOUS_BACKEND_VERSION" ]; then
# The version has changed, set flag to build backend
BUILD_BACKEND=true
echo "✅ Backend version changed: $PREVIOUS_BACKEND_VERSION → $BACKEND_VERSION"
else
echo "❎ Backend version unchanged: $BACKEND_VERSION. Skip docker backend."
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

The script assumes that version changes always indicate the need to build. Consider adding validation to ensure the version actually increased (not decreased or unchanged in a different way):

if [ "$BACKEND_VERSION" != "$PREVIOUS_BACKEND_VERSION" ]; then
  # Add semantic version comparison here
  if [[ "$BACKEND_VERSION" > "$PREVIOUS_BACKEND_VERSION" ]]; then
    BUILD_BACKEND=true
    echo "✅ Backend version increased: $PREVIOUS_BACKEND_VERSION$BACKEND_VERSION"
  else
    echo "⚠️ Backend version changed but not increased: $PREVIOUS_BACKEND_VERSION$BACKEND_VERSION"
  fi
fi

Comment on lines +106 to +116
- name: 🐳 Build and push Backend Docker image
uses: docker/build-push-action@v6
with:
context: ./backend-agent
file: ./backend-agent/Dockerfile
push: true
tags: |
${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:${{ needs.check_version_update.outputs.backend_version }}
${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:latest
cache-from: type=gha
cache-to: type=gha,mode=max
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding error handling for Docker build failures and implementing retry logic for robustness:

- name: 🐳 Build and push Backend Docker image
  uses: docker/build-push-action@v6
  with:
    context: ./backend-agent
    file: ./backend-agent/Dockerfile
    push: true
    tags: |
      ${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:${{ needs.check_version_update.outputs.backend_version }}
      ${{ secrets.DOCKER_REGISTRY_URL }}/stars-backend:latest
    cache-from: type=gha
    cache-to: type=gha,mode=max
  retry:
    max_attempts: 3
    retry_wait_seconds: 30

Comment on lines +6 to +15
jobs:
summary:
name: PR Summary
runs-on: [ubuntu-latest]
steps:
- uses: SAP/ai-assisted-github-actions/pr-summary@v3
with:
aicore-service-key: ${{ secrets.AICORE_SERVICE_KEY }}
model: gpt-4o
exclude-files: package-lock.json, uv.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

The workflow lacks proper error handling and could benefit from conditional execution. Consider adding failure handling:

jobs:
  summary:
    name: PR Summary
    runs-on: [ubuntu-latest]
    continue-on-error: true  # Don't fail the entire PR if AI summary fails
    steps:
      - uses: SAP/ai-assisted-github-actions/pr-summary@v3
        with:
          aicore-service-key: ${{ secrets.AICORE_SERVICE_KEY }}
          model: gpt-4o
          exclude-files: package-lock.json, uv.lock
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Comment on lines +1 to +59
FROM astral/uv:python3.11-trixie AS builder

# Install build dependencies including Rust for packages that need it
RUN apt-get update && apt-get install -y \
build-essential \
curl \
pkg-config \
libssl-dev \
&& curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y \
&& . ~/.cargo/env \
&& rm -rf /var/lib/apt/lists/*

# Add Rust to PATH
ENV PATH="/root/.cargo/bin:${PATH}"

WORKDIR /app

# Copy dependency files first for better Docker layer caching
COPY pyproject.toml uv.lock ./

# Set environment variables to use CPU-only versions and reduce download size
ENV UV_EXTRA_INDEX_URL="https://download.pytorch.org/whl/cpu"
ENV TORCH_INDEX_URL="https://download.pytorch.org/whl/cpu"

# Install dependencies using uv with proper build environment
RUN . ~/.cargo/env && \
uv sync --frozen --no-dev --no-cache && \
# Clean up any temporary files to reduce layer size
rm -rf /root/.cache/uv /tmp/* /var/tmp/* && \
# Remove Rust toolchain after build to reduce image size
rustup self uninstall -y

# ----------------------------------------

FROM python:3.11-slim-trixie

# Install only runtime dependencies
RUN apt-get update && apt-get install -y \
libssl3 \
libffi8 \
&& rm -rf /var/lib/apt/lists/*

WORKDIR /app

COPY requirements.txt .
RUN pip install -r requirements.txt --no-cache-dir
# Copy the virtual environment from builder stage
COPY --from=builder /app/.venv /app/.venv

# Copy dependency files
COPY pyproject.toml uv.lock ./

# Copy the rest of the application
COPY . .

# Make sure we use the virtual environment
ENV PATH="/app/.venv/bin:$PATH"
ENV PYTHONPATH="/app"

EXPOSE 8080
CMD [ "python", "main.py" ]
CMD ["python", "main.py"]
Copy link
Contributor

Choose a reason for hiding this comment

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

The Dockerfile is well-structured with multi-stage build, but consider adding specific version pinning for better reproducibility:

FROM astral/uv:0.5.0-python3.11-trixie AS builder

Also, consider adding a health check:

HEALTHCHECK --interval=30s --timeout=10s --start-period=40s --retries=3 \
  CMD curl -f http://localhost:8080/health || exit 1

Comment on lines +9 to +17
# Copy the config generation script
COPY ./generate-config.sh /usr/local/bin/generate-config.sh

# Make the script executable and create config directory with proper permissions
USER root
RUN chmod +x /usr/local/bin/generate-config.sh && \
mkdir -p /usr/share/nginx/html/assets/configs && \
chown -R nginx:nginx /usr/share/nginx/html/assets/configs
USER nginx
Copy link
Contributor

Choose a reason for hiding this comment

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

The entrypoint script approach is good, but consider adding error handling and validation:

# Copy the config generation script
COPY ./generate-config.sh /usr/local/bin/generate-config.sh

# Make the script executable and validate it
RUN chmod +x /usr/local/bin/generate-config.sh && \
    # Test the script syntax
    sh -n /usr/local/bin/generate-config.sh && \
    mkdir -p /usr/share/nginx/html/assets/configs && \
    chown -R nginx:nginx /usr/share/nginx/html/assets/configs

Comment on lines +20 to +26
cat > /usr/share/nginx/html/assets/configs/config.json << EOF
{
"backend_url": "${BACKEND_URL}/process",
"backend_url_ws": "${BACKEND_WS_URL}",
"api_url": "${API_URL}"
}
EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

The script should validate that the generated config is valid JSON:

# Generate the config.json file
cat > /usr/share/nginx/html/assets/configs/config.json << EOF
{
  "backend_url": "${BACKEND_URL}/process",
  "backend_url_ws": "${BACKEND_WS_URL}",
  "api_url": "${API_URL}"
}
EOF

# Validate generated JSON
if ! python3 -m json.tool /usr/share/nginx/html/assets/configs/config.json > /dev/null 2>&1; then
    echo "Error: Generated config.json is not valid JSON"
    exit 1
fi

Comment on lines -38 to 56
loadHeatmapData() {
let url = '';
url = `${environment.api_url}/api/heatmap`;
const url = `${this.configService.apiUrl}/api/heatmap`;
this.http.get<ScoreResponse>(url).subscribe({
next: scoresData => {
this.processDataAfterScan(scoresData);
},
error: error => console.error('❌ Erreur API:', error),
error: error => console.error('❌ Error API:', error),
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding loading states and error handling for better UX:

private isLoading = false;
private error: string | null = null;

loadHeatmapData() {
  this.isLoading = true;
  this.error = null;
  const url = `${this.configService.apiUrl}/api/heatmap`;
  this.http.get<ScoreResponse>(url).subscribe({
    next: scoresData => {
      this.processDataAfterScan(scoresData);
      this.isLoading = false;
    },
    error: error => {
      console.error('❌ Error API:', error);
      this.error = 'Failed to load heatmap data';
      this.isLoading = false;
    },
  });
}

Comment on lines +21 to 34
loadConfig(): Observable<AppConfig> {
return this.http.get<AppConfig>('/assets/configs/config.json').pipe(
tap(config => {
this.config = config;
console.log('Runtime configuration loaded:', config);
}),
catchError(async () => {
const errorMsg = 'Could not load runtime configuration. Make sure config.json is accessible and contains valid configuration.';
console.error(errorMsg);
alert(errorMsg);
throw new Error(errorMsg);
})
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The error handling could be improved to provide more actionable feedback:

loadConfig(): Observable<AppConfig> {
  return this.http.get<AppConfig>('/assets/configs/config.json').pipe(
    tap(config => {
      // Validate required configuration properties
      const requiredProps = ['backend_url', 'backend_url_ws', 'api_url'];
      const missingProps = requiredProps.filter(prop => !config[prop as keyof AppConfig]);
      
      if (missingProps.length > 0) {
        throw new Error(`Missing required configuration properties: ${missingProps.join(', ')}`);
      }
      
      this.config = config;
      console.log('Runtime configuration loaded:', config);
    }),
    catchError(error => {
      const errorMsg = `Configuration load failed: ${error.message || 'Unknown error'}`;
      console.error(errorMsg);
      alert(errorMsg);
      throw error;
    })
  );
}

Comment on lines +40 to +50
onSave() {
// Validate weights before sending
const invalidWeights = Object.entries(this.currentWeights)
.filter(([_, weight]) => weight < 0 || !Number.isInteger(weight));

if (invalidWeights.length > 0) {
this.snackBar.open('Please enter valid positive integers for all weights', '❌', {
duration: 3000
});
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider improving the validation logic and error handling:

onSave() {
  // Validate weights before sending
  const invalidEntries = Object.entries(this.currentWeights)
    .filter(([_, weight]) => {
      const numWeight = Number(weight);
      return !Number.isInteger(numWeight) || numWeight < 0;
    });

  if (invalidEntries.length > 0) {
    const attackNames = invalidEntries.map(([name]) => name).join(', ');
    this.snackBar.open(
      `Invalid weights for: ${attackNames}. Please enter positive integers.`, 
      '❌', 
      { duration: 5000 }
    );
    return;
  }

  // Convert values to numbers to ensure type safety
  const numericWeights = Object.fromEntries(
    Object.entries(this.currentWeights).map(([k, v]) => [k, Number(v)])
  );

  // Rest of the method...
}

@marcorosa marcorosa merged commit 97b88f0 into main Sep 12, 2025
3 checks passed
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