Skip to content

Add Layout Components for TnT#12

Merged
t0mdavid-m merged 17 commits intodevelopfrom
tnt_fdr
May 15, 2025
Merged

Add Layout Components for TnT#12
t0mdavid-m merged 17 commits intodevelopfrom
tnt_fdr

Conversation

@t0mdavid-m
Copy link
Copy Markdown
Member

@t0mdavid-m t0mdavid-m commented May 14, 2025

Enables the following components for FLASHTnT:

  • Raw Heatmap
  • Deconvolved Heatmap
  • Score Distribution Plot

Summary by CodeRabbit

  • New Features

    • Added new layout options: Score Distribution Plot, MS1 raw heatmap, and MS1 deconvolved heatmap.
    • Introduced a Score Distribution Plot visualizing kernel density estimates of target and decoy proteoform-level Q-values.
    • Enhanced data initialization to support the new Score Distribution Plot component.
  • Bug Fixes

    • Limited Cython package version to below 3.1 for improved compatibility.
  • Chores

    • Updated Docker build workflow to discard build output, optimizing the image build process.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented May 14, 2025

Walkthrough

Three new component options and corresponding internal names were added to the layout manager. Kernel density estimation (KDE) calculations for target and decoy proteoform Q-value distributions were implemented in the TNT parser, with results saved for later use. The data initialization logic was updated to support the new FDR plot component, fetching target and decoy density data. The Dockerfile was modified to restrict the Cython version, and the GitHub Actions workflow was updated to discard Docker build output.

Changes

File(s) Change Summary
content/FLASHTnT/FLASHTnTLayoutManager.py Added three new component options: "Score Distribution Plot," "MS1 raw heatmap," and "MS1 deconvolved heatmap," with corresponding internal component names.
src/parse/tnt.py Added KDE computation for target and decoy Q-value distributions via a new fdr_density_distribution function; results stored using file manager keys.
src/render/initialize.py Added logic in initialize_data for the new 'id_fdr_plot' component, fetching and assigning target/decoy density data from file manager.
Dockerfile Modified pip install command to restrict Cython version to <3.1.
.github/workflows/build-docker-images.yml Updated workflow to add --output type=tar,dest=/dev/null option to Docker build, discarding build output.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI
    participant Backend
    participant FileManager
    participant Parser

    User->>UI: Selects "Score Distribution Plot"
    UI->>Backend: Requests data for 'id_fdr_plot'
    Backend->>FileManager: Fetch 'density_id_target' and 'density_id_decoy'
    FileManager-->>Backend: Returns density data
    Backend->>UI: Sends density data for rendering

    Note over Parser,FileManager: During parsing
    Parser->>FileManager: Store 'density_id_target' and 'density_id_decoy' after KDE computation
Loading

Poem

Three new plots hop into view,
With FDR curves, their densities true.
The parser now smooths Q-values with care,
While Docker builds lighter, swift as air.
A rabbit’s delight in code so neat—
New features and fixes, a treat to greet!
🐇✨

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.


Note

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.
Enjoy the performance boost—your workflow just got faster.


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/parse/tnt.py (1)

168-188: Consider adding sample count for context in the returned dataframes.

Including the count of samples used in the KDE calculation would provide valuable context for the visualization and could help users interpret the plots.

def fdr_density_distribution(df, logger=None):
    df = df[df['ProteoformLevelQvalue'] > 0]
    # Find density targets
    target_qscores = df[~df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
    if len(target_qscores) > 0:
        x_target = np.linspace(target_qscores.min(), target_qscores.max(), 200)
        kde_target = gaussian_kde(target_qscores)
        density_target = pd.DataFrame({'x': x_target, 'y': kde_target(x_target)})
+       density_target['sample_count'] = len(target_qscores)
    else:
        density_target = pd.DataFrame(columns=['x', 'y'])
+       density_target['sample_count'] = 0

    # Find density decoys (if present)
    decoy_qscores = df[df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
    if len(decoy_qscores) > 0:
        x_decoy = np.linspace(decoy_qscores.min(), decoy_qscores.max(), 200)
        kde_decoy = gaussian_kde(decoy_qscores)
        density_decoy = pd.DataFrame({'x': x_decoy, 'y': kde_decoy(x_decoy)})
+       density_decoy['sample_count'] = len(decoy_qscores)
    else:
        density_decoy = pd.DataFrame(columns=['x', 'y'])
+       density_decoy['sample_count'] = 0

    return density_target, density_decoy
src/render/initialize.py (1)

112-112: Consider adding a more descriptive title for the FDR Plot.

The current title "FDR Plot" is generic. Consider using a more descriptive title like "Proteoform Q-value Distribution" to better describe the specific data being visualized.

-   component_arguments = FDRPlotly(title="FDR Plot")
+   component_arguments = FDRPlotly(title="Proteoform Q-value Distribution")
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62252e3 and 9b2abe7.

📒 Files selected for processing (3)
  • content/FLASHTnT/FLASHTnTLayoutManager.py (1 hunks)
  • src/parse/tnt.py (2 hunks)
  • src/render/initialize.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/render/initialize.py (2)
src/workflow/FileManager.py (1)
  • get_results (403-445)
src/render/components.py (1)
  • FDRPlotly (69-72)
src/parse/tnt.py (2)
src/parse/deconv.py (1)
  • fdr_density_distribution (113-130)
src/workflow/FileManager.py (1)
  • store_data (308-323)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build-full-app
  • GitHub Check: build-openms
🔇 Additional comments (5)
src/parse/tnt.py (2)

8-8: Added dependency for KDE calculation.

The import of gaussian_kde from scipy.stats is correctly added to support the new kernel density estimation functionality.


163-165: Good integration of density estimation in the parseTnT workflow.

The code computes density distributions for target and decoy proteoforms and stores them in the file manager with appropriate keys that will be used by the Score Distribution Plot component.

content/FLASHTnT/FLASHTnTLayoutManager.py (2)

15-17: Added new visualization component options.

The three new visualization components have been appropriately added to the COMPONENT_OPTIONS list: Score Distribution Plot, MS1 raw heatmap, and MS1 deconvolved heatmap.


26-28: Added corresponding internal component identifiers.

The three new internal component identifiers have been properly added to the COMPONENT_NAMES list, matching the order of the previously added component options.

src/render/initialize.py (1)

107-112: Properly implemented Score Distribution Plot initialization.

The implementation correctly retrieves the target and decoy density data computed by the fdr_density_distribution function in the tnt.py file and configures the FDRPlotly component to display the data.

Comment on lines +168 to +188
def fdr_density_distribution(df, logger=None):
df = df[df['ProteoformLevelQvalue'] > 0]
# Find density targets
target_qscores = df[~df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
if len(target_qscores) > 0:
x_target = np.linspace(target_qscores.min(), target_qscores.max(), 200)
kde_target = gaussian_kde(target_qscores)
density_target = pd.DataFrame({'x': x_target, 'y': kde_target(x_target)})
else:
density_target = pd.DataFrame(columns=['x', 'y'])

# Find density decoys (if present)
decoy_qscores = df[df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
if len(decoy_qscores) > 0:
x_decoy = np.linspace(decoy_qscores.min(), decoy_qscores.max(), 200)
kde_decoy = gaussian_kde(decoy_qscores)
density_decoy = pd.DataFrame({'x': x_decoy, 'y': kde_decoy(x_decoy)})
else:
density_decoy = pd.DataFrame(columns=['x', 'y'])

return density_target, density_decoy No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Implement error handling for the KDE computation.

While the function correctly computes the kernel density estimates for target and decoy distributions, it could benefit from additional error handling. The gaussian_kde function may raise exceptions if the input data has specific characteristics (e.g., constant values).

def fdr_density_distribution(df, logger=None):
    df = df[df['ProteoformLevelQvalue'] > 0]
    # Find density targets
    target_qscores = df[~df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
    if len(target_qscores) > 0:
+       try:
            x_target = np.linspace(target_qscores.min(), target_qscores.max(), 200)
            kde_target = gaussian_kde(target_qscores)
            density_target = pd.DataFrame({'x': x_target, 'y': kde_target(x_target)})
+       except Exception as e:
+           if logger:
+               logger.warning(f"Failed to compute KDE for target distribution: {str(e)}")
+           density_target = pd.DataFrame(columns=['x', 'y'])
    else:
        density_target = pd.DataFrame(columns=['x', 'y'])

    # Find density decoys (if present)
    decoy_qscores = df[df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
    if len(decoy_qscores) > 0:
+       try:
            x_decoy = np.linspace(decoy_qscores.min(), decoy_qscores.max(), 200)
            kde_decoy = gaussian_kde(decoy_qscores)
            density_decoy = pd.DataFrame({'x': x_decoy, 'y': kde_decoy(x_decoy)})
+       except Exception as e:
+           if logger:
+               logger.warning(f"Failed to compute KDE for decoy distribution: {str(e)}")
+           density_decoy = pd.DataFrame(columns=['x', 'y'])
    else:
        density_decoy = pd.DataFrame(columns=['x', 'y'])

    return density_target, density_decoy
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def fdr_density_distribution(df, logger=None):
df = df[df['ProteoformLevelQvalue'] > 0]
# Find density targets
target_qscores = df[~df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
if len(target_qscores) > 0:
x_target = np.linspace(target_qscores.min(), target_qscores.max(), 200)
kde_target = gaussian_kde(target_qscores)
density_target = pd.DataFrame({'x': x_target, 'y': kde_target(x_target)})
else:
density_target = pd.DataFrame(columns=['x', 'y'])
# Find density decoys (if present)
decoy_qscores = df[df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
if len(decoy_qscores) > 0:
x_decoy = np.linspace(decoy_qscores.min(), decoy_qscores.max(), 200)
kde_decoy = gaussian_kde(decoy_qscores)
density_decoy = pd.DataFrame({'x': x_decoy, 'y': kde_decoy(x_decoy)})
else:
density_decoy = pd.DataFrame(columns=['x', 'y'])
return density_target, density_decoy
def fdr_density_distribution(df, logger=None):
df = df[df['ProteoformLevelQvalue'] > 0]
# Find density targets
target_qscores = df[~df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
if len(target_qscores) > 0:
try:
x_target = np.linspace(target_qscores.min(), target_qscores.max(), 200)
kde_target = gaussian_kde(target_qscores)
density_target = pd.DataFrame({'x': x_target, 'y': kde_target(x_target)})
except Exception as e:
if logger:
logger.warning(f"Failed to compute KDE for target distribution: {e}")
density_target = pd.DataFrame(columns=['x', 'y'])
else:
density_target = pd.DataFrame(columns=['x', 'y'])
# Find density decoys (if present)
decoy_qscores = df[df['accession'].str.startswith('DECOY_')]['ProteoformLevelQvalue'].dropna()
if len(decoy_qscores) > 0:
try:
x_decoy = np.linspace(decoy_qscores.min(), decoy_qscores.max(), 200)
kde_decoy = gaussian_kde(decoy_qscores)
density_decoy = pd.DataFrame({'x': x_decoy, 'y': kde_decoy(x_decoy)})
except Exception as e:
if logger:
logger.warning(f"Failed to compute KDE for decoy distribution: {e}")
density_decoy = pd.DataFrame(columns=['x', 'y'])
else:
density_decoy = pd.DataFrame(columns=['x', 'y'])
return density_target, density_decoy

Comment on lines +25 to +26
'combined_spectrum',
'id_fdr_plot',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify component mapping between options and names.

The component name 'combined_spectrum' at line 25 appears to map to 'Sequence tag view (Tag table needed)' in the options list based on position. Please verify this mapping is correct, as there seems to be a potential mismatch.


🏁 Script executed:

#!/bin/bash
# Check for potential mismatch between COMPONENT_OPTIONS and COMPONENT_NAMES

echo "Checking component mappings in FLASHTnTLayoutManager.py..."
echo "Component 'combined_spectrum' should map to 'Sequence tag view (Tag table needed)'"

# Verify the option at index 4 in both lists
grep -A 7 "COMPONENT_OPTIONS=\[" content/FLASHTnT/FLASHTnTLayoutManager.py | tail -n 7 | head -n 1
grep -A 6 "COMPONENT_NAMES=\[" content/FLASHTnT/FLASHTnTLayoutManager.py | tail -n 6 | head -n 1

Length of output: 556


🏁 Script executed:

#!/bin/bash
# Extract full COMPONENT_OPTIONS and COMPONENT_NAMES lists for mapping review
echo "=== COMPONENT_OPTIONS ==="
grep -n "COMPONENT_OPTIONS" content/FLASHTnT/FLASHTnTLayoutManager.py -A 20

echo
echo "=== COMPONENT_NAMES ==="
grep -n "COMPONENT_NAMES" content/FLASHTnT/FLASHTnTLayoutManager.py -A 20

Length of output: 12478


Fix misaligned COMPONENT_OPTIONS/COMPONENT_NAMES entries

The 5th and 6th elements in your arrays are currently swapped/misnamed:

  • COMPONENT_OPTIONS[4] = 'Sequence tag view (Tag table needed)' ↔ COMPONENT_NAMES[4] = 'combined_spectrum'
  • COMPONENT_OPTIONS[5] = 'Score Distribution Plot' ↔ COMPONENT_NAMES[5] = 'id_fdr_plot'

These mappings don’t align semantically. Please update one or both lists so that:

  • 'Sequence tag view (Tag table needed)' maps to a name like 'sequence_tag_view'
  • 'Score Distribution Plot' maps to something like 'score_distribution_plot'
  • 'combined_spectrum' pairs with its intended option label (e.g. 'Combined spectrum')

Locations:

  • content/FLASHTnT/FLASHTnTLayoutManager.py lines 10–18 (COMPONENT_OPTIONS)
  • content/FLASHTnT/FLASHTnTLayoutManager.py lines 20–28 (COMPONENT_NAMES)

Suggested diff sketch:

 COMPONENT_OPTIONS = [
     …,
-    'Sequence tag view (Tag table needed)',
+    'Combined spectrum',                  # new or corrected label
+    'Sequence tag view (Tag table needed)',
     'Score Distribution Plot',
     …
 ]

 COMPONENT_NAMES = [
     …,
-    'combined_spectrum',
-    'id_fdr_plot',
+    'combined_spectrum',                  # match new/renamed option above
+    'sequence_tag_view',                  # maps to Sequence tag view
+    'score_distribution_plot',            # maps to Score Distribution Plot
     …
 ]

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +107 to +112
elif comp_name == 'id_fdr_plot':
data = file_manager.get_results(selected_data, ['density_id_target'])
data_to_send['density_target'] = data['density_id_target']
data = file_manager.get_results(selected_data, ['density_id_decoy'])
data_to_send['density_decoy'] = data['density_id_decoy']
component_arguments = FDRPlotly(title="FDR Plot")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding error handling for missing density datasets.

The code doesn't have explicit error handling for cases where density_id_target or density_id_decoy datasets might be missing. Consider adding a try-except block or using the partial=True parameter in the get_results call to handle such cases gracefully.

elif comp_name == 'id_fdr_plot':
+   try:
        data = file_manager.get_results(selected_data,  ['density_id_target'])
        data_to_send['density_target'] = data['density_id_target']
        data = file_manager.get_results(selected_data,  ['density_id_decoy'])
        data_to_send['density_decoy'] = data['density_id_decoy']
        component_arguments = FDRPlotly(title="FDR Plot")
+   except KeyError:
+       # Handle missing datasets
+       data_to_send['density_target'] = pd.DataFrame(columns=['x', 'y'])
+       data_to_send['density_decoy'] = pd.DataFrame(columns=['x', 'y'])
+       component_arguments = FDRPlotly(title="FDR Plot (No Data Available)")

Alternatively, using the partial=True parameter:

elif comp_name == 'id_fdr_plot':
-   data = file_manager.get_results(selected_data,  ['density_id_target'])
-   data_to_send['density_target'] = data['density_id_target']
-   data = file_manager.get_results(selected_data,  ['density_id_decoy'])
-   data_to_send['density_decoy'] = data['density_id_decoy']
+   data = file_manager.get_results(selected_data,  ['density_id_target', 'density_id_decoy'], partial=True)
+   data_to_send['density_target'] = data.get('density_id_target', pd.DataFrame(columns=['x', 'y']))
+   data_to_send['density_decoy'] = data.get('density_id_decoy', pd.DataFrame(columns=['x', 'y']))
    component_arguments = FDRPlotly(title="FDR Plot")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
elif comp_name == 'id_fdr_plot':
data = file_manager.get_results(selected_data, ['density_id_target'])
data_to_send['density_target'] = data['density_id_target']
data = file_manager.get_results(selected_data, ['density_id_decoy'])
data_to_send['density_decoy'] = data['density_id_decoy']
component_arguments = FDRPlotly(title="FDR Plot")
elif comp_name == 'id_fdr_plot':
# load both densities at once, allowing missing keys
data = file_manager.get_results(
selected_data,
['density_id_target', 'density_id_decoy'],
partial=True
)
data_to_send['density_target'] = data.get(
'density_id_target',
pd.DataFrame(columns=['x', 'y'])
)
data_to_send['density_decoy'] = data.get(
'density_id_decoy',
pd.DataFrame(columns=['x', 'y'])
)
component_arguments = FDRPlotly(title="FDR Plot")

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
Dockerfile (1)

84-84: Use a consistent pip invocation for clarity.

To ensure the right interpreter is used in both upgrade and install steps, consider invoking pip via python -m pip:

-RUN pip install --upgrade pip && python -m pip install -U setuptools nose 'Cython<3.10' autowrap pandas 'numpy==1.26.4' pytest
+RUN python -m pip install --upgrade pip && python -m pip install -U setuptools nose 'Cython<3.10' autowrap pandas 'numpy==1.26.4' pytest
🧰 Tools
🪛 Hadolint (2.12.0)

[error] 84-84: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b2abe7 and ed24cea.

📒 Files selected for processing (1)
  • Dockerfile (1 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
Dockerfile

[error] 84-84: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build-full-app
  • GitHub Check: build-openms
🔇 Additional comments (1)
Dockerfile (1)

84-84: Pin Cython version below 3.10 to avoid incompatibilities.

Locking Cython to <3.10 here is a sensible precaution against breaking API changes introduced in Cython 3.10+. This aligns with the new visualization components’ dependencies.

🧰 Tools
🪛 Hadolint (2.12.0)

[error] 84-84: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
Dockerfile (1)

84-84: Restrict Cython to <3.1 for compatibility
The updated command properly pins Cython<3.1, preventing potential breakages in the Cython extensions used by the new visualization components. Consider adding --no-cache-dir to the pip install to reduce image layer size and caching overhead, and optionally pin other critical Python dependencies (e.g., pandas, pytest) to specific versions for end-to-end reproducibility.

🧰 Tools
🪛 Hadolint (2.12.0)

[error] 84-84: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed24cea and 6dd4b12.

📒 Files selected for processing (1)
  • Dockerfile (1 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
Dockerfile

[error] 84-84: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build-openms
  • GitHub Check: build-full-app

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
.github/workflows/build-docker-images.yml (2)

18-27: Enable Docker BuildKit and inline caching in CI
Setting DOCKER_BUILDKIT: 1 and COMPOSE_DOCKER_CLI_BUILD: 1 along with --build-arg flags ensures BuildKit features and caching are active during the docker build. Consider using the docker/setup-buildx-action to standardize multi-platform builds across runners.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 27-27: no new line character at the end of file

(new-line-at-end-of-file)


27-27: Add a newline at end of file
YAML files should end with a newline to satisfy linter requirements.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 27-27: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6dd4b12 and e303aa1.

📒 Files selected for processing (3)
  • .github/workflows/build-docker-images.yml (1 hunks)
  • Dockerfile (4 hunks)
  • openms-streamlit-vue-component (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • openms-streamlit-vue-component
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-docker-images.yml

[error] 27-27: no new line character at the end of file

(new-line-at-end-of-file)

🪛 Hadolint (2.12.0)
Dockerfile

[error] 88-88: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build-openms
  • GitHub Check: build-full-app
🔇 Additional comments (4)
Dockerfile (4)

1-1: Declare Dockerfile syntax version for BuildKit features
Specifying # syntax=docker/dockerfile:1.4 at the top ensures BuildKit directives are recognized.


13-13: Enable multi-platform builds in the JS stage
Using --platform=$BUILDPLATFORM on the node:21 image allows cross-architecture builds when invoked with BuildKit.


44-47: Add BuildKit-specific build arguments
Introducing ARG BUILDKIT_INLINE_CACHE=1 and ARG DOCKER_BUILDKIT=1 enables inline caching and faster incremental builds under BuildKit.


88-88: Pin setuptools and Cython to compatible versions
Constraining setuptools<80.4 and Cython<3.1 in the pip install line prevents breaking changes from upstream releases.

🧰 Tools
🪛 Hadolint (2.12.0)

[error] 88-88: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
Dockerfile (1)

84-84: Pin Cython<3.1 to maintain compatibility; consider broader version pinning and cache control.

The updated pip install step now pins Cython<3.1, which is excellent to prevent future breakage when Cython 3.1 is released. For improved build reproducibility and smaller image layers, consider:

  • Explicitly pinning versions for other packages (setuptools, nose, autowrap, pandas, pytest).
  • Adding --no-cache-dir to the pip install command.

Example refactor:

 RUN pip install --upgrade pip && \
-  python -m pip install -U setuptools nose 'Cython<3.1' autowrap pandas 'numpy==1.26.4' pytest
+  python -m pip install --no-cache-dir \
+    setuptools==<version> \
+    nose==<version> \
+    autowrap==<version> \
+    'Cython<3.1' \
+    pandas==<version> \
+    'numpy==1.26.4' \
+    pytest==<version>
🧰 Tools
🪛 Hadolint (2.12.0)

[error] 84-84: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

.github/workflows/build-docker-images.yml (1)

19-19: Add a newline at end of file

POSIX and many linters expect a trailing newline in text files. Please append a single newline character at the end of this YAML file.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 19-19: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between e303aa1 and 8a21c76.

📒 Files selected for processing (2)
  • .github/workflows/build-docker-images.yml (1 hunks)
  • Dockerfile (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/build-docker-images.yml

16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🪛 YAMLlint (1.35.1)
.github/workflows/build-docker-images.yml

[error] 19-19: no new line character at the end of file

(new-line-at-end-of-file)

🪛 Hadolint (2.12.0)
Dockerfile

[error] 84-84: Ensure the shebang uses an absolute path to the interpreter.

(SC2239)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: build-openms
  • GitHub Check: build-full-app
🔇 Additional comments (1)
.github/workflows/build-docker-images.yml (1)

16-19: Streamline the build to target only run-app

The new workflow step focuses on building the run-app stage, which aligns with your multi-stage Dockerfile and speeds up CI by skipping earlier layers that aren’t needed for validation. This is a solid approach for rapid feedback.

🧰 Tools
🪛 actionlint (1.7.4)

16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🪛 YAMLlint (1.35.1)

[error] 19-19: no new line character at the end of file

(new-line-at-end-of-file)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/build-docker-images.yml (1)

18-18: Add newline at end of file
YAML parsers typically expect a trailing newline. Please add a blank line at the end of this file to satisfy linting.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 18-18: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 8a21c76 and 1b842d5.

📒 Files selected for processing (1)
  • .github/workflows/build-docker-images.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/build-docker-images.yml

16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🪛 YAMLlint (1.35.1)
.github/workflows/build-docker-images.yml

[error] 18-18: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build-openms
🔇 Additional comments (1)
.github/workflows/build-docker-images.yml (1)

16-16: Skip: No issues with the checkout action version
The actions/checkout@v3 action is the current major release and appropriate for this workflow.

🧰 Tools
🪛 actionlint (1.7.4)

16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/build-docker-images.yml (1)

18-18: Remove trailing whitespace and ensure EOF newline.

YAMLlint flagged a trailing space on this line and the absence of a newline at end of file. Removing the extra space after the period and adding a newline will resolve both issues.

Apply this diff:

-        run: docker build --file Dockerfile --tag streamlitapp:latest --output type=tar,dest=/dev/null . 
+        run: docker build --file Dockerfile --tag streamlitapp:latest --output type=tar,dest=/dev/null .

Also add a blank line at the very end of the file.


<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 YAMLlint (1.35.1)</summary>

[error] 18-18: no new line character at the end of file

(new-line-at-end-of-file)

---

[error] 18-18: trailing spaces

(trailing-spaces)

</details>

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
**Plan: Pro**
**Cache: Disabled due to data retention organization setting**
**Knowledge Base: Disabled due to data retention organization setting**


<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 1b842d545740a001cabcee0378ddfe8b98ff8930 and 8dc755abae574f73d76b6f686b4c76c08029e2c1.

</details>

<details>
<summary>📒 Files selected for processing (1)</summary>

* `.github/workflows/build-docker-images.yml` (1 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 actionlint (1.7.4)</summary>

<details>
<summary>.github/workflows/build-docker-images.yml</summary>

16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

</details>

</details>
<details>
<summary>🪛 YAMLlint (1.35.1)</summary>

<details>
<summary>.github/workflows/build-docker-images.yml</summary>

[error] 18-18: no new line character at the end of file

(new-line-at-end-of-file)

---

[error] 18-18: trailing spaces

(trailing-spaces)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms (2)</summary>

* GitHub Check: build-openms
* GitHub Check: build-full-app

</details>

<details>
<summary>🔇 Additional comments (2)</summary><blockquote>

<details>
<summary>.github/workflows/build-docker-images.yml (2)</summary>

`16-16`: **Verify the GitHub Actions checkout action version.**

The static analysis hint suggests that `actions/checkout@v3` might be outdated. While `v3` is still maintained and widely used, `v4` is available. Please confirm whether you intend to upgrade to `actions/checkout@v4` or keep using `v3`.

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 actionlint (1.7.4)</summary>

16-16: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

</details>

</details>

---

`17-17`: **Build step declaration looks good.**

The `- name: Build the full Docker image` entry is correctly formatted and self-explanatory. The placement of the build context at the end enhances readability and aligns with best practices.

</details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

@t0mdavid-m t0mdavid-m merged commit 45df004 into develop May 15, 2025
5 checks passed
@t0mdavid-m t0mdavid-m deleted the tnt_fdr branch May 15, 2025 09:54
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