Skip to content

Comments

Fix s_create_directory called with /* wildcard creating junk directory#1214

Open
sbryngelson wants to merge 2 commits intoMFlowCode:masterfrom
sbryngelson:fix/create-directory-wildcard
Open

Fix s_create_directory called with /* wildcard creating junk directory#1214
sbryngelson wants to merge 2 commits intoMFlowCode:masterfrom
sbryngelson:fix/create-directory-wildcard

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Feb 21, 2026

User description

Summary

  • Fix s_read_serial_ic_data_files in m_start_up.fpp calling s_create_directory with a /* suffix, which creates a literal directory named * instead of cleaning the output directory.

Bug Details

The code at line 508 calls:

call s_create_directory(trim(proc_rank_dir)//'/*')

The comment above says "cleaned out to make room for new pre-process data", but s_create_directory runs mkdir -p "path/*", which creates a directory literally named * inside proc_rank_dir. The intent was to remove old files to make room for new data.

The old data files happen to get overwritten by subsequent writes, so this doesn't cause crashes, but it leaves a junk * directory behind and doesn't actually clean old files that may no longer be relevant.

Fix

Replace the s_create_directory(path/*) call with s_delete_directory(path) to actually remove the old directory contents. The subsequent s_create_directory(path/0) uses mkdir -p so it recreates both the parent and the /0 subdirectory.

Test plan

  • Serial pre-process restart (old_ic) cases still work
  • No * directory created in process rank directories

🤖 Generated with Claude Code


CodeAnt-AI Description

Fix creation of a literal '*' directory when preparing per-rank pre-process folders

What Changed

  • Removed the call that created a path ending with '/*' and replaced it with a call that deletes the existing rank directory before recreating the time-step subfolder
  • The code now removes previous pre-process files instead of creating a literal '*' directory and then recreates the '/0' subdirectory for new data

Impact

✅ No leftover '*' directories in per-rank folders
✅ Old pre-process files are removed before new data is written
✅ More predictable serial pre-process restart behavior

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

Release Notes

  • Chores
    • Improved directory cleanup during initialization process to ensure consistent system state before starting operations.

Fixes #1220

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 21, 2026 05:02
@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

Warning

Rate limit exceeded

@sbryngelson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 18 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Modified directory cleanup logic in the serial initial conditions data file reading function. Instead of deleting only directory contents, the entire proc_rank_dir directory is now removed before being recreated.

Changes

Cohort / File(s) Summary
Directory Cleanup Logic
src/pre_process/m_start_up.fpp
Changed post-data-read cleanup from deleting directory contents (trim(proc_rank_dir)//'/*') to deleting the entire directory (trim(proc_rank_dir)) before recreating the per-rank directory structure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A directory's fate, now complete and whole,
Where once we trimmed the edges with care,
Now we clear the slate, reset the role,
Delete and rebuild, with methodical flair! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main bug fix: replacing a problematic s_create_directory call with /* wildcard that creates a junk '*' directory.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description provides clear context: it identifies the bug with s_create_directory being called with a /* suffix, explains the root cause (creating a literal * directory), describes the intended behavior, and provides the fix with a clear test plan. All key sections from the template are addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@codeant-ai codeant-ai bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Feb 21, 2026
cubic-dev-ai[bot]

This comment was marked as off-topic.

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Safety / accidental deletion risk
    Calling a delete routine on a computed path could accidentally remove the wrong directory if the path is empty, malformed, or outside the expected case directory. There is no guard in the new call to prevent accidental deletion of top-level or unrelated dirs.

  • Assumed s_delete_directory semantics
    The fix assumes s_delete_directory removes the directory contents (or the whole dir) safely and that subsequent s_create_directory(trim(proc_rank_dir)//'/0') will recreate required parents. Confirm s_delete_directory behavior (recursive removal vs. removing only contents) to ensure the sequence produces the intended layout and does not leave a partially deleted state.

  • Inconsistent wildcard usage
    The PR replaces a call that created a literal '' directory by calling
    s_delete_directory(trim(proc_rank_dir)) in s_read_serial_ic_data_files. However elsewhere the code still calls
    s_delete_directory(trim(proc_rank_dir)//'/
    ') (in s_read_serial_grid_data_files). This inconsistency can leave the original problem present in other code paths or lead to different deletion semantics depending on how s_delete_directory is implemented.

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Feb 21, 2026

CodeAnt AI finished reviewing your PR.

@sbryngelson sbryngelson added the bug Something isn't working or doesn't seem right label Feb 21, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes serial pre-process restart cleanup by removing an incorrect /* suffix that caused creation of a literal * directory instead of clearing old process-rank output.

Changes:

  • Replace s_create_directory(proc_rank_dir//'/*') with s_delete_directory(proc_rank_dir) to properly remove prior output.
  • Keep subsequent directory creation via s_create_directory(proc_rank_dir//'/0'), which recreates the expected folder structure.

Copy link
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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pre_process/m_start_up.fpp`:
- Around line 508-509: s_read_serial_grid_data_files currently calls
s_delete_directory(trim(proc_rank_dir)//'/*'), which fails to expand the glob
because s_delete_directory wraps the path in quotes; change that call to
s_delete_directory(trim(proc_rank_dir)) to match the fix at the other call site
and ensure the entire per-rank directory is removed before recreating the /0
subdirectory via s_create_directory. Ensure you update the invocation inside
s_read_serial_grid_data_files (the proc_rank_dir argument) and keep the existing
s_delete_directory/s_create_directory implementations unchanged.

…a_files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (55bda76).

Files with missing lines Patch % Lines
src/pre_process/m_start_up.fpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1214   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          70       70           
  Lines       20498    20498           
  Branches     1990     1990           
=======================================
  Hits         9030     9030           
  Misses      10329    10329           
  Partials     1139     1139           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working or doesn't seem right size:XS This PR changes 0-9 lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

s_create_directory called with /* wildcard creates junk directory

1 participant