Add git info to combine files script#36
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds GitPython-based per-file Git metadata into generated config headers, normalizes relative paths to POSIX, updates header wording/format, enhances the test harness for regex-aware expected outputs, updates fixtures, and adds a Windows integration-test job. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tools/combine-files/test_files/run_tests.py (1)
32-32: Consider addingstrict=Truetozip()for defensive programming.While the length check at lines 26-29 prevents mismatched lengths, adding
strict=Trueto thezip()call would make the code more explicit and provide an additional safety check. This parameter is available in Python 3.10+.Apply this diff:
- for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines)): + for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines, strict=True)):Note: Ensure the project's minimum Python version supports this feature (Python 3.10+).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(5 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(2 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
🧰 Additional context used
📓 Path-based instructions (3)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_starlark_inline.conf.expectedtools/combine-files/test_files/test_multiple_commands.conf.expectedtools/combine-files/test_files/test_whitespace.conf.expectedtools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/requirements.txttools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_multiple_scripts.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_both_inline.conf.expectedtools/combine-files/README.mdtools/combine-files/test_files/test_shell_inline.conf.expected
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_starlark_inline.conf.expectedtools/combine-files/test_files/test_multiple_commands.conf.expectedtools/combine-files/test_files/test_whitespace.conf.expectedtools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/requirements.txttools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_multiple_scripts.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_both_inline.conf.expectedtools/combine-files/README.mdtools/combine-files/test_files/test_shell_inline.conf.expected
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Documentation files should clearly communicate the dual audience: (1) server-side dashboard users who want to keep agent with bundled configs, and (2) edge device developers who want to customize agent behavior. Ensure examples and instructions are appropriate for the intended skill level and use case. Since this is a public repository, we should not include any sensitive information, the instructions should be easily understandable for a wide audience, and we should avoid using any jargon or technical terms that are not commonly used.
Headings should not include special characters like emojis or backticks. The table of contents should be generated with themarkdown-toctool, meaning that we should use the<!-- toc -->and<!-- tocstop -->tags.
Files:
tools/combine-files/README.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:350-351
Timestamp: 2025-08-10T14:54:48.316Z
Learning: In the fixedit-data-agent-examples repository, shell portability requirements (such as Axis devices using POSIX /bin/sh instead of bash) should be documented in a general scripting guide rather than repeated in individual project README files. This approach was confirmed by daniel-falk for better documentation organization.
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/README.md
📚 Learning: 2025-08-08T10:22:37.500Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T10:22:37.500Z
Learning: In project-strobe-color-from-github-workflow, the GITHUB_WORKFLOW environment variable is currently not used by any config or script; it appears only in README examples. If later introduced for GitHub API filtering, use a workflow file name (e.g., ci.yml) or numeric id and avoid quotes because the Data Agent passes quotes literally into the value, which can break URL paths.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expected
📚 Learning: 2025-11-07T15:57:47.716Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 26
File: project-qr-code-decoder-http-push/combined.conf:190-190
Timestamp: 2025-11-07T15:57:47.716Z
Learning: In project-qr-code-decoder-http-push/combined.conf (and similar Telegraf config files with embedded Starlark code), comments within Starlark source strings intentionally avoid apostrophes (e.g., "Dont" instead of "Don't") because single quotes break syntax highlighting in VS Code.
Applied to files:
tools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/README.md
🪛 GitHub Actions: Python Code Quality Check for combine-files tool
tools/combine-files/combine_files.py
[error] 1523-1523: F841: local variable 'e' is assigned to but never used.
[error] 1559-1559: E501: line too long (113 > 100 characters).
🪛 GitHub Check: SonarCloud Code Analysis
tools/combine-files/combine_files.py
[warning] 1523-1523: Remove the unused local variable "e".
🪛 Ruff (0.14.8)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
tools/combine-files/combine_files.py
1523-1523: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (15)
tools/combine-files/test_files/test_multiple_mixed.conf.expected (2)
4-7: LGTM! Git metadata headers are well-structured.The regex patterns for git metadata validation are well-formed and provide good test coverage. The use of
{{REGEX:...}}syntax allows the tests to validate structure while accommodating dynamic git states.
6-6: No action needed—implementation already ensures 12-character commit hashes.The implementation explicitly slices the commit hash to 12 characters at line 1491 using
repo.head.commit.hexsha[:12], and the doctest at line 1379 validates this withlen(info['commit']) == 12. The test expectation regex[0-9a-f]{12}correctly reflects this behavior. Git's default short hash length variability is not a concern here because the code does not rely on defaults—it always takes the first 12 characters of the full SHA.tools/combine-files/test_files/test_multiline_variable.conf.expected (1)
1-7: Git metadata header template looks correct and non-intrusiveThe updated header with
From fileplus the four{{REGEX:...}}lines is well-structured, uses appropriate patterns for repo/path/commit/status, and stays safely in comment lines so it won’t affect Telegraf parsing. Looks good.tools/combine-files/README.md (1)
188-208: Git header behavior is clearly documented and aligned with testsThe new section under “Configuration File Combining” accurately describes the per-file header, matches the comment format and fields used in the fixtures (repo, file path, 12-char commit, status), and documents the non-git fallback string. No issues from a docs/code-alignment perspective.
tools/combine-files/requirements.txt (1)
2-2: GitPython dependency range looks reasonable; verify environment compatibilityAdding
GitPython>=3.0.0,<4.0.0is consistent with the new git metadata functionality and keeps the major version stable. Please just confirm this range is compatible with the Python versions and environments wherecombine_files.pyis expected to run (e.g., CI, dev machines).tools/combine-files/test_files/test_both_inline.conf.expected (1)
1-7: Consistent git header template across fixturesThe updated header (
From fileplus the four git-related{{REGEX:...}}lines) matches the documented format and other test fixtures, and keeps dynamic values abstracted behind regexes. No problems here.tools/combine-files/test_files/test_shell_inline.conf.expected (1)
1-7: Git metadata header expectations look good for shell inlining caseThe new header block with
From fileand the four regex placeholders is consistent with the global convention and remains comment-only, so it won’t interfere with the inlined shell script content. Looks solid.tools/combine-files/test_files/test_multiple_commands.conf.expected (1)
1-7: Header regex placeholders correctly applied to multi-command fixtureThe git metadata header template (repo, path, 12-char commit, status with optional details) is correctly added and mirrors other fixtures. This keeps expectations DRY conceptually while allowing dynamic values. No changes needed.
tools/combine-files/test_files/test_default_values.conf.expected (1)
1-7: Git header expectations are consistent for default-values scenarioThe added
From fileline and git metadata regex placeholders follow the same structure as in other .expected files and won’t affect the TOML semantics. Header coverage here looks complete and correct.tools/combine-files/test_files/test_script_with_args.conf.expected (1)
1-7: Git header template integrates cleanly with script-with-args fixtureThe standardized git metadata header (including the status regex with optional parenthesized detail) is correctly added ahead of the existing content and doesn’t interfere with the complex shell-argument example. Looks good.
tools/combine-files/test_files/run_tests.py (1)
117-151: LGTM - Well-structured regex-based comparison with comprehensive error reporting.The integration of regex-based comparison is well done. The code handles both regex patterns and exact matches, provides detailed mismatch reporting (showing up to 10 mismatches), and includes a unified diff fallback for defensive programming. The use of
splitlines(keepends=True)preserves line endings for accurate comparison.tools/combine-files/combine_files.py (4)
21-21: LGTM!The GitPython import is appropriate for the new git metadata extraction functionality.
1315-1475: Well-documented function with comprehensive doctests.The
_get_git_info()function provides thorough coverage of git status detection scenarios including clean, modified, staged, and untracked files. The doctests serve as excellent executable documentation.
1591-1611: LGTM!The git info integration into headers is well-implemented with:
- Clear status descriptions that help users understand the file state
- Graceful fallback when not in a git repository
- Proper formatting that maintains the existing header style
1646-1654: LGTM!The updated doctest correctly uses flexible assertions to validate the combined output structure without depending on specific git metadata values. The separator count check (
>= 4) properly verifies that headers are generated for each config file.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
daniel-falk
force-pushed
the
add-git-info-to-script-to-combine
branch
from
64cf00d to
b8db398
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/tools-combine-files-quality.yml (1)
31-55: Linux/Windows split for integration tests looks soundRenaming to
integration-tests-linuxand adding a Windows counterpart with explicit Git installation makes sense given the new GitPython usage and git-heavy tests. Steps are consistent across platforms; if you want to reduce duplication later, you could migrate these to a single matrix job overos: [ubuntu-latest, windows-latest], but that’s optional.Also applies to: 56-84
tools/combine-files/test_files/run_tests.py (1)
12-52: Regex comparison helper matches current fixture design
compare_with_regexcorrectly enforces equal line counts, supports a single{{REGEX:...}}placeholder per line, and usesre.fullmatchon the substituted pattern so the whole line must match. That aligns well with the updated*.expectedfixtures and gives clearer mismatch messages than a plain diff.tools/combine-files/combine_files.py (1)
1567-1641: Header formatting and git metadata integration are coherent with testsThe changes to
_add_config_headerandcombine_configs’ doctest are internally consistent:
- A leading blank line before each header improves readability, and the conditional extra newline between files avoids headers running together.
- Switching to
# From file: <relative path>via_get_relative_path_or_originalmatches both integration fixtures and the CLI behavior with--file-path-root.- When
_get_git_inforeturns data, the four lines (# Git repo: …,# File path in repo: …,# Commit: …,# Status: …) line up exactly with the{{REGEX:...}}expectations in the updated*.expectedfiles; the fallback# No version tracking info foundstill keeps non‑git usage graceful.- The
combine_configsdoctest’s>= 4separator count correctly reflects two separators per file (top and bottom) once per-file headers are in place.No functional issues here; the headers and tests look well-aligned.
Also applies to: 1675-1683
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(2 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(5 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(2 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
✅ Files skipped from review due to trivial changes (2)
- tools/combine-files/test_files/test_nonstring_variables.conf.expected
- tools/combine-files/test_files/test_starlark_inline.conf.expected
🚧 Files skipped from review as they are similar to previous changes (11)
- tools/combine-files/test_files/test_multiple_mixed.conf.expected
- tools/combine-files/test_files/test_comments.conf.expected
- tools/combine-files/test_files/test_multiline_variable.conf.expected
- tools/combine-files/README.md
- tools/combine-files/test_files/test_path_variables.conf.expected
- tools/combine-files/test_files/test_multiple_commands.conf.expected
- tools/combine-files/test_files/test_shell_inline.conf.expected
- tools/combine-files/test_files/test_multiple_scripts.conf.expected
- tools/combine-files/test_files/test_both_inline.conf.expected
- tools/combine-files/test_files/test_script_with_args.conf.expected
- tools/combine-files/test_files/test_whitespace.conf.expected
🧰 Additional context used
📓 Path-based instructions (2)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/combine_files.pytools/combine-files/requirements.txttools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/run_tests.py
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/combine_files.pytools/combine-files/requirements.txttools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/run_tests.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
tools/combine-files/combine_files.py
🪛 GitHub Actions: Python Code Quality Check for combine-files tool
tools/combine-files/combine_files.py
[error] 1501-1501: pylint: R1702 Too many nested blocks (6/5) in combine_files.py:1501:4
🪛 Ruff (0.14.8)
tools/combine-files/combine_files.py
1555-1560: Consider moving this statement to an else block
(TRY300)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (4)
tools/combine-files/requirements.txt (1)
1-5: GitPython pin matches new git integration usageThe GitPython constraint looks appropriate for the new
_get_git_infohelper and keeps the major version bounded; no issues from a tooling perspective. Just ensure callers of this tool actually install from thisrequirements.txtsocombine_files.pydoesn’t hitImportErrorin ad‑hoc usage.tools/combine-files/test_files/test_multiline.conf.expected (1)
1-7: Regex-based header expectations align with generatorThe updated header (
# From file: …) and the four{{REGEX:...}}lines match what_add_config_headernow emits (repo, path, 12-char commit, and human-readable status). The status regex correctly allows both bare states and explanatory text, so this fixture should stay robust across different git states.tools/combine-files/test_files/test_default_values.conf.expected (1)
1-7: Header and Git regex expectations are consistentThis fixture’s new header format and
{{REGEX:...}}lines are consistent with both the generator (_add_config_header+_get_git_info) and the regex comparison helper inrun_tests.py, so it should behave correctly across different git states.tools/combine-files/test_files/run_tests.py (1)
55-93: Command wrapper and failure reporting improve test diagnosticsWrapping the CLI invocation in
_run_combine_command(with explicit return-code and “Successfully …” checks) and funnelling output comparison through_report_test_failuregives much clearer error reporting while keepingrun_testreadable. The truncation of stderr and limiting mismatches/diff lines are sensible safeguards for noisy failures.Also applies to: 95-128, 157-185
daniel-falk
force-pushed
the
add-git-info-to-script-to-combine
branch
2 times, most recently
from
67b26e6 to
7140c55
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/tools-combine-files-quality.yml (1)
58-93: Windows integration test job looks good.The addition of Windows-specific testing is appropriate for git-dependent functionality. The explicit Git installation and duplicate doctest run help catch platform-specific issues.
Consider pinning the
git-for-windows/setup-gitaction to a specific version (e.g.,@v2.0.0) instead of@v2to avoid unexpected breaking changes:- name: Install Git - uses: git-for-windows/setup-git@v2 + uses: git-for-windows/setup-git@v2.0.0 with: version: "latest"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(3 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(5 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(2 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
🚧 Files skipped from review as they are similar to previous changes (7)
- tools/combine-files/test_files/test_both_inline.conf.expected
- tools/combine-files/test_files/test_multiple_commands.conf.expected
- tools/combine-files/test_files/test_starlark_inline.conf.expected
- tools/combine-files/test_files/test_shell_inline.conf.expected
- tools/combine-files/test_files/test_multiline_variable.conf.expected
- tools/combine-files/test_files/test_multiple_mixed.conf.expected
- tools/combine-files/test_files/test_path_variables.conf.expected
🧰 Additional context used
📓 Path-based instructions (3)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_whitespace.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/run_tests.pytools/combine-files/README.mdtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/requirements.txttools/combine-files/test_files/test_multiple_scripts.conf.expected
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_whitespace.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/run_tests.pytools/combine-files/README.mdtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/requirements.txttools/combine-files/test_files/test_multiple_scripts.conf.expected
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Documentation files should clearly communicate the dual audience: (1) server-side dashboard users who want to keep agent with bundled configs, and (2) edge device developers who want to customize agent behavior. Ensure examples and instructions are appropriate for the intended skill level and use case. Since this is a public repository, we should not include any sensitive information, the instructions should be easily understandable for a wide audience, and we should avoid using any jargon or technical terms that are not commonly used.
Headings should not include special characters like emojis or backticks. The table of contents should be generated with themarkdown-toctool, meaning that we should use the<!-- toc -->and<!-- tocstop -->tags.
Files:
tools/combine-files/README.md
🧠 Learnings (6)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/combine_files.py
📚 Learning: 2025-08-10T14:54:48.316Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:350-351
Timestamp: 2025-08-10T14:54:48.316Z
Learning: In the fixedit-data-agent-examples repository, shell portability requirements (such as Axis devices using POSIX /bin/sh instead of bash) should be documented in a general scripting guide rather than repeated in individual project README files. This approach was confirmed by daniel-falk for better documentation organization.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-09-03T14:18:52.406Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-11-07T15:57:47.716Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 26
File: project-qr-code-decoder-http-push/combined.conf:190-190
Timestamp: 2025-11-07T15:57:47.716Z
Learning: In project-qr-code-decoder-http-push/combined.conf (and similar Telegraf config files with embedded Starlark code), comments within Starlark source strings intentionally avoid apostrophes (e.g., "Dont" instead of "Don't") because single quotes break syntax highlighting in VS Code.
Applied to files:
tools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/README.md
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/README.md
🪛 GitHub Actions: Python Code Quality Check for combine-files tool
tools/combine-files/combine_files.py
[error] 1345-1345: flake8: E501 line too long (105 > 100 characters)
[error] 1364-1364: flake8: E501 line too long (105 > 100 characters)
[error] 1384-1384: flake8: E501 line too long (105 > 100 characters)
[error] 1405-1405: flake8: E501 line too long (105 > 100 characters)
🪛 Ruff (0.14.8)
tools/combine-files/combine_files.py
1560-1565: Consider moving this statement to an else block
(TRY300)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (17)
tools/combine-files/README.md (2)
192-203: LGTM!The example header format is clear and well-documented. The conditional behavior when files are in a git repository is appropriately explained.
204-212: No issues found. The documented git status values and their descriptions accurately match the implementation. The function returns bare status strings (clean,modified,staged,untracked,unknown), which are then mapped to display versions with contextual descriptions in parentheses for some statuses (modified (uncommitted changes),staged (changes ready to commit),unknown (could not determine status)). The documentation correctly reflects what users will see in the output..github/workflows/tools-combine-files-quality.yml (2)
7-7: LGTM!Adding the workflow file itself to trigger paths is good practice, ensuring workflow changes are tested before merge.
Also applies to: 11-11
33-57: LGTM!Renaming the job to
integration-tests-linuximproves clarity when paired with the new Windows job.tools/combine-files/test_files/test_default_values.conf.expected (1)
1-8: LGTM!The updated header format with git metadata placeholders is well-structured. The regex patterns correctly validate:
- 12-character commit hashes
- Status values with optional descriptive suffixes
tools/combine-files/test_files/test_script_with_args.conf.expected (1)
1-8: LGTM!Header format is consistent with other test fixtures and correctly uses regex placeholders for dynamic git metadata.
tools/combine-files/test_files/test_comments.conf.expected (1)
1-8: LGTM!Header updates are consistent with the new git metadata format across all test fixtures.
tools/combine-files/test_files/test_whitespace.conf.expected (1)
1-8: LGTM!Header format matches the pattern established across all test fixtures.
tools/combine-files/test_files/test_multiline.conf.expected (1)
1-8: LGTM!Header format is consistent with all other test fixtures, completing the uniform update across the test suite.
tools/combine-files/test_files/test_nonstring_variables.conf.expected (1)
1-7: LGTM! Test fixture properly updated with Git metadata placeholders.The header format changes align with the PR objectives:
- Leading blank line added
- Label changed from "From:" to "From file:"
- Git metadata placeholders added with appropriate regex patterns for dynamic content validation
tools/combine-files/test_files/test_multiple_scripts.conf.expected (1)
1-7: LGTM! Fixture updates are consistent with other test files.The header format changes match the pattern applied across all test fixtures in this PR.
tools/combine-files/test_files/run_tests.py (4)
12-52: LGTM! Regex-aware comparison is well-implemented.The function properly handles both exact matches and regex patterns, with clear mismatch reporting. The length check before
zip()ensures alignment, making thestrict=parameter suggestion unnecessary.
55-92: LGTM! Command execution helper is well-structured.The function properly encapsulates command execution with appropriate error handling and validation. The subprocess call is safe as it's running the test suite's own scripts.
95-127: LGTM! Failure reporting provides clear diagnostics.The function delivers helpful mismatch details with sensible output limits, and falls back to unified diff when needed. This will significantly improve debugging experience.
158-185: LGTM! Refactoring improves test runner maintainability.The use of helper functions makes the test flow clearer and enables regex-based validation for dynamic Git metadata in test fixtures.
tools/combine-files/combine_files.py (2)
1444-1569: LGTM! Git info extraction is comprehensive and well-tested.The function properly integrates repository metadata with file status using
_get_git_status, includes robust error handling, and provides thorough doctest coverage. The implementation correctly handles edge cases like files outside repos and repos without commits.
1572-1645: LGTM! Header generation cleanly integrates Git metadata.The changes properly implement the new header format with:
- Leading blank line for visual separation
- Updated "From file:" label
- Comprehensive Git metadata when available
- Graceful fallback message for non-git files
- User-friendly status descriptions
The integration with
_get_git_infois clean and the doctests appropriately reflect the new behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/combine-files/test_files/run_tests.py (1)
26-32: Consider addingstrict=Truetozip()for defensive coding.While the length check on line 26 makes this safe, explicitly adding
strict=Trueto thezip()call on line 32 would make the code more defensive and self-documenting.Apply this diff:
- for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines)): + for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines, strict=True)):Note: This requires Python 3.10+. If supporting older Python versions, the current approach with the length check is appropriate.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(3 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(7 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(2 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
✅ Files skipped from review due to trivial changes (2)
- tools/combine-files/test_files/test_both_inline.conf.expected
- tools/combine-files/test_files/test_whitespace.conf.expected
🚧 Files skipped from review as they are similar to previous changes (5)
- tools/combine-files/test_files/test_multiple_commands.conf.expected
- tools/combine-files/requirements.txt
- tools/combine-files/test_files/test_multiple_scripts.conf.expected
- tools/combine-files/test_files/test_path_variables.conf.expected
- tools/combine-files/test_files/test_multiline.conf.expected
🧰 Additional context used
📓 Path-based instructions (3)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/README.mdtools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_starlark_inline.conf.expected
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Documentation files should clearly communicate the dual audience: (1) server-side dashboard users who want to keep agent with bundled configs, and (2) edge device developers who want to customize agent behavior. Ensure examples and instructions are appropriate for the intended skill level and use case. Since this is a public repository, we should not include any sensitive information, the instructions should be easily understandable for a wide audience, and we should avoid using any jargon or technical terms that are not commonly used.
Headings should not include special characters like emojis or backticks. The table of contents should be generated with themarkdown-toctool, meaning that we should use the<!-- toc -->and<!-- tocstop -->tags.
Files:
tools/combine-files/README.md
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/README.mdtools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_starlark_inline.conf.expected
🧠 Learnings (6)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-11-07T15:57:47.716Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 26
File: project-qr-code-decoder-http-push/combined.conf:190-190
Timestamp: 2025-11-07T15:57:47.716Z
Learning: In project-qr-code-decoder-http-push/combined.conf (and similar Telegraf config files with embedded Starlark code), comments within Starlark source strings intentionally avoid apostrophes (e.g., "Dont" instead of "Don't") because single quotes break syntax highlighting in VS Code.
Applied to files:
tools/combine-files/README.mdtools/combine-files/test_files/test_comments.conf.expected
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/README.mdtools/combine-files/test_files/test_multiline_variable.conf.expected
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/combine_files.py
📚 Learning: 2025-08-10T14:54:48.316Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:350-351
Timestamp: 2025-08-10T14:54:48.316Z
Learning: In the fixedit-data-agent-examples repository, shell portability requirements (such as Axis devices using POSIX /bin/sh instead of bash) should be documented in a general scripting guide rather than repeated in individual project README files. This approach was confirmed by daniel-falk for better documentation organization.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-09-03T14:18:52.406Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
🪛 GitHub Actions: Python Code Quality Check for combine-files tool
tools/combine-files/combine_files.py
[error] 1315-1315: pylint: Too many return statements (R0911) in combine_files.py at line 1315. (7/6) - Exit code 8 during 'pylint *.py'.
🪛 Ruff (0.14.8)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
tools/combine-files/combine_files.py
1569-1574: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (12)
.github/workflows/tools-combine-files-quality.yml (3)
7-7: LGTM! Self-triggering workflow ensures changes are tested.Adding the workflow file itself to the trigger paths ensures that changes to the CI configuration are automatically validated.
Also applies to: 11-11
84-89: LGTM! Windows doctest execution catches platform-specific issues.Running doctests on Windows is essential for validating cross-platform compatibility, especially given the Git path handling changes that address Windows path separators. The comment clearly explains why this duplication is necessary.
74-77: Thegit-for-windows/setup-git@v2action is the standard and recommended approach for installing Git on Windows runners. The action has official README examples and is documented for use withwindows-latestrunners. No changes are needed.tools/combine-files/combine_files.py (2)
1453-1579: LGTM! Git metadata extraction is well-implemented.The function correctly:
- Uses GitPython to detect repository and extract metadata
- Handles missing commits and non-git directories gracefully
- Delegates status determination to the dedicated
_get_git_statushelper- Returns structured metadata with all required fields
The Ruff TRY300 hint is a minor style suggestion that can be safely ignored—the current structure is clear and readable.
1628-1653: LGTM! Header generation with Git metadata is well-structured.The updated header format includes:
- Leading blank line for visual separation
- Clear "From file:" label
- Complete Git metadata when available (repo, path, commit, status)
- User-friendly status descriptions
- Graceful fallback message for non-git files
The implementation is clean and the doctests validate the new behavior.
tools/combine-files/test_files/test_starlark_inline.conf.expected (1)
1-7: LGTM! Test fixtures correctly reflect new header format.All test fixtures consistently implement the new header structure:
- Leading blank line for visual separation
- Updated label: "From file:" instead of "From:"
- Regex placeholders for dynamic Git metadata validation
The regex patterns accurately match the implementation:
- Commit hash: exactly 12 hex characters
- Status values: all possible states with optional display notes
- Flexible matching for repo name and file path
This approach allows tests to pass regardless of the actual Git state of test files.
tools/combine-files/test_files/test_multiline_variable.conf.expected (1)
1-8: LGTM! Test fixture correctly reflects new Git header format.The header changes align with the PR's objective to add Git metadata to combined file headers. The regex placeholders appropriately match dynamic Git information during test validation.
tools/combine-files/test_files/test_nonstring_variables.conf.expected (1)
1-8: LGTM! Header format consistent with other test fixtures.The Git metadata regex placeholders match the pattern used across the test suite. The commit hash regex pattern was previously discussed and confirmed to be appropriately strict for the test environment.
tools/combine-files/README.md (1)
192-212: Excellent documentation of the new Git metadata feature.The documentation clearly explains the per-file Git header format, provides a concrete example, and covers the fallback behavior when files aren't in a Git repository. This will help users understand what information is included in the combined output.
tools/combine-files/test_files/run_tests.py (3)
55-92: LGTM! Helper function is well-structured.The function properly encapsulates command execution with appropriate error handling. The subprocess security warning from static analysis is a false positive, as
script_pathis derived from a trusted source (parent directory).
95-127: LGTM! Clear and helpful error reporting.The function provides well-formatted failure messages with appropriate output limiting to avoid overwhelming users. The fallback to unified diff for edge cases is a good design choice.
158-185: Excellent refactoring with helper functions.The updated
run_testfunction is cleaner and more maintainable with the extracted helpers. The static analysis suggestion about moving line 185 to an else block is a style preference; the current structure is clear and follows standard patterns for this type of validation flow.
daniel-falk
force-pushed
the
add-git-info-to-script-to-combine
branch
2 times, most recently
from
be163b3 to
50aed1f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/combine-files/test_files/run_tests.py (1)
12-52: Consider addingstrict=Truetozip()for safer comparison.The regex comparison logic is sound. However, using
zip()withoutstrict=Truecan silently truncate if line counts differ. While the line count check at line 26 catches this, addingstrict=Trueprovides defense-in-depth.Apply this diff if Python 3.10+ is guaranteed:
- for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines)): + for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines, strict=True)):Note:
strict=requires Python 3.10+. If supporting older versions, the current code is acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(3 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(5 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(2 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
🚧 Files skipped from review as they are similar to previous changes (11)
- tools/combine-files/test_files/test_script_with_args.conf.expected
- tools/combine-files/test_files/test_multiple_commands.conf.expected
- tools/combine-files/requirements.txt
- tools/combine-files/README.md
- tools/combine-files/test_files/test_comments.conf.expected
- tools/combine-files/test_files/test_whitespace.conf.expected
- tools/combine-files/test_files/test_multiple_scripts.conf.expected
- tools/combine-files/test_files/test_nonstring_variables.conf.expected
- tools/combine-files/test_files/test_starlark_inline.conf.expected
- tools/combine-files/test_files/test_multiple_mixed.conf.expected
- tools/combine-files/test_files/test_default_values.conf.expected
🧰 Additional context used
📓 Path-based instructions (2)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_both_inline.conf.expectedtools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/combine_files.py
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_both_inline.conf.expectedtools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/combine_files.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expected
📚 Learning: 2025-08-08T10:22:37.500Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T10:22:37.500Z
Learning: In project-strobe-color-from-github-workflow, the GITHUB_WORKFLOW environment variable is currently not used by any config or script; it appears only in README examples. If later introduced for GitHub API filtering, use a workflow file name (e.g., ci.yml) or numeric id and avoid quotes because the Data Agent passes quotes literally into the value, which can break URL paths.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expected
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
tools/combine-files/combine_files.py
🪛 Ruff (0.14.8)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
tools/combine-files/combine_files.py
1567-1572: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (15)
tools/combine-files/test_files/test_multiline_variable.conf.expected (1)
1-8: LGTM! Well-structured test expectation for Git metadata headers.The updated header format correctly validates the new Git metadata fields using regex patterns:
- Changed label from "From:" to "From file:" as specified
- Four regex patterns properly capture dynamic Git info (repo, path, 12-char commit, status)
- Status pattern comprehensively covers all five possible states with optional parenthetical details
- Standardized separators and blank line create clean visual structure
The test expectation aligns perfectly with the PR objectives and will properly validate the tool's output.
tools/combine-files/test_files/test_path_variables.conf.expected (1)
1-7: LGTM! Header format update is consistent.The updated header correctly reflects the new format with "From file:" and Git metadata placeholders. The regex patterns properly capture the expected Git information fields.
tools/combine-files/combine_files.py (4)
21-21: LGTM! GitPython import added.The import is necessary for the new Git metadata functionality.
1315-1448: LGTM! Git status detection logic is sound.The function correctly:
- Uses
as_posix()for cross-platform path compatibility- Implements proper status precedence (untracked → modified → staged → clean)
- Handles errors gracefully with "unknown" status
- Uses
repo.index.diff(None, ...)to detect unstaged changes- Uses
repo.index.diff(repo.head.commit, ...)to detect staged changesThe refactored single-return structure addresses the previous pylint R0911 violation.
1451-1577: LGTM! Git info extraction is well-designed.The function properly:
- Finds the Git repository with
search_parent_directories=True- Computes relative paths correctly
- Handles empty repositories (no commits) via
ValueErrorandAttributeErrorcatching- Delegates status determination to
_get_git_status- Returns
Nonefor non-Git paths with appropriate exception handling
1579-1652: LGTM! Header generation correctly integrates Git metadata.The updated function:
- Adds a leading blank line for consistent spacing
- Changes label from "From:" to "From file:"
- Integrates Git metadata (repo, path, commit, status) when available
- Provides clear fallback message "No version tracking info found" for non-Git files
- Uses descriptive status text for better user experience
The doctests properly validate the new behavior.
tools/combine-files/test_files/test_both_inline.conf.expected (1)
1-8: LGTM! Test fixture updated consistently.The header format changes match the implementation updates and are consistent with other test fixtures.
tools/combine-files/test_files/test_shell_inline.conf.expected (1)
1-7: LGTM! Test fixture header updated correctly.The changes are consistent with the new header format and Git metadata integration.
tools/combine-files/test_files/test_multiline.conf.expected (1)
1-7: LGTM! Test fixture aligned with new header format.The updates match the implementation changes consistently.
tools/combine-files/test_files/run_tests.py (4)
4-4: LGTM! Import added for regex support.The
remodule is needed for the new regex-based comparison functionality.
55-92: LGTM! Command execution helper is well-designed.The function:
- Properly constructs the command with
sys.executable- Captures output for validation
- Checks both exit code and success message
- Returns clear error messages
The S603 static analysis warning is a false positive—this test runner executes trusted scripts from the same repository.
95-127: LGTM! Failure reporting is clear and helpful.The function provides excellent user experience by:
- Showing the first 10 mismatches with details
- Falling back to unified diff when needed
- Limiting output to prevent overwhelming displays
130-190: LGTM! Test function properly refactored.The updated function:
- Delegates command execution to
_run_combine_command- Uses
compare_with_regexfor flexible output matching- Provides clear pass/fail reporting via
_report_test_failure- Preserves line endings with
keepends=Truefor accurate comparisonThe TRY300 static analysis suggestion is a minor style preference. The current control flow is clear and readable.
.github/workflows/tools-combine-files-quality.yml (2)
7-7: Self-trigger workflow on changes to the CI file itself.Adding the workflow file to the trigger paths is a good practice and ensures that CI updates are tested immediately.
Also applies to: 11-11
33-33: Job rename clarifies platform-specific variant.Renaming
integration-teststointegration-tests-linuxproperly signals the introduction of the Windows variant below.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tools/combine-files/combine_files.py (1)
1315-1448: Fix line length violations in doctests to pass pipeline checks.The implementation logic is sound—path normalization via
.as_posix()correctly handles cross-platform paths, and the git status precedence (untracked → modified → staged → clean) is correct.However, several doctest lines exceed the 100-character limit, causing pipeline failures:
Apply this pattern to all four locations (lines 1338, 1359, 1381, 1404):
- ... _ = subprocess.run(["git", "config", "user.email", "test@example.com"], - ... cwd=repo_dir, capture_output=True) + ... _ = subprocess.run( + ... ["git", "config", "user.email", "test@example.com"], + ... cwd=repo_dir, capture_output=True + ... )
🧹 Nitpick comments (2)
tools/combine-files/combine_files.py (1)
1567-1572: Optional: Consider else block for clarity (Ruff TRY300).Static analysis suggests moving the return statement into an else block for better exception handling structure. This is a minor style suggestion:
except (git.exc.InvalidGitRepositoryError, git.exc.GitError): # Not in a git repository or git error return None + else: + return { + "repo_name": repo_name, + "file_path_in_repo": file_path_normalized, + "commit": commit, + "status": status, + } - - return { - "repo_name": repo_name, - "file_path_in_repo": file_path_normalized, - "commit": commit, - "status": status, - }tools/combine-files/test_files/run_tests.py (1)
12-52: Well-structured regex comparison utility.The function correctly handles the
{{REGEX:pattern}}syntax and provides useful mismatch details. Good use offullmatchto ensure the entire line matches.One minor improvement: while the length check at lines 26-29 makes
strict=Truetechnically redundant, adding it tozip()is good defensive practice in case the length check is ever removed or bypassed.- for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines)): + for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines, strict=True)):
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(3 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(5 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(2 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/combine-files/test_files/test_default_values.conf.expected
- tools/combine-files/requirements.txt
- tools/combine-files/test_files/test_multiline.conf.expected
- tools/combine-files/test_files/test_multiple_mixed.conf.expected
- tools/combine-files/test_files/test_both_inline.conf.expected
- tools/combine-files/test_files/test_whitespace.conf.expected
- tools/combine-files/test_files/test_starlark_inline.conf.expected
- tools/combine-files/test_files/test_shell_inline.conf.expected
🧰 Additional context used
📓 Path-based instructions (3)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_multiple_scripts.conf.expectedtools/combine-files/test_files/test_multiple_commands.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/README.mdtools/combine-files/test_files/test_comments.conf.expected
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_multiple_scripts.conf.expectedtools/combine-files/test_files/test_multiple_commands.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/README.mdtools/combine-files/test_files/test_comments.conf.expected
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Documentation files should clearly communicate the dual audience: (1) server-side dashboard users who want to keep agent with bundled configs, and (2) edge device developers who want to customize agent behavior. Ensure examples and instructions are appropriate for the intended skill level and use case. Since this is a public repository, we should not include any sensitive information, the instructions should be easily understandable for a wide audience, and we should avoid using any jargon or technical terms that are not commonly used.
Headings should not include special characters like emojis or backticks. The table of contents should be generated with themarkdown-toctool, meaning that we should use the<!-- toc -->and<!-- tocstop -->tags.
Files:
tools/combine-files/README.md
🧠 Learnings (9)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/README.md
📚 Learning: 2025-08-08T10:22:37.500Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T10:22:37.500Z
Learning: In project-strobe-color-from-github-workflow, the GITHUB_WORKFLOW environment variable is currently not used by any config or script; it appears only in README examples. If later introduced for GitHub API filtering, use a workflow file name (e.g., ci.yml) or numeric id and avoid quotes because the Data Agent passes quotes literally into the value, which can break URL paths.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expected.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/combine_files.py.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-08-10T14:54:48.316Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:350-351
Timestamp: 2025-08-10T14:54:48.316Z
Learning: In the fixedit-data-agent-examples repository, shell portability requirements (such as Axis devices using POSIX /bin/sh instead of bash) should be documented in a general scripting guide rather than repeated in individual project README files. This approach was confirmed by daniel-falk for better documentation organization.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-09-03T14:18:52.406Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-08-08T09:31:58.742Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T09:31:58.742Z
Learning: In project-strobe-color-from-github-workflow/README.md, the FixedIT Data Agent “Extra env” semicolon-separated KEY=VALUE string must not include quotes around values. Use unquoted placeholders (e.g., GITHUB_WORKFLOW=your_workflow_name).
Applied to files:
.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-08-10T15:15:33.003Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:126-127
Timestamp: 2025-08-10T15:15:33.003Z
Learning: In the FixedIT Data Agent Extra env configuration, spaces in values are acceptable without quotes. For the GITHUB_WORKFLOW variable in project-strobe-color-from-github-workflow, using a placeholder with spaces like "Your Workflow Name" is preferred as it demonstrates proper configuration for the common case where GitHub workflow names contain spaces.
Applied to files:
.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-11-07T15:57:47.716Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 26
File: project-qr-code-decoder-http-push/combined.conf:190-190
Timestamp: 2025-11-07T15:57:47.716Z
Learning: In project-qr-code-decoder-http-push/combined.conf (and similar Telegraf config files with embedded Starlark code), comments within Starlark source strings intentionally avoid apostrophes (e.g., "Dont" instead of "Don't") because single quotes break syntax highlighting in VS Code.
Applied to files:
tools/combine-files/README.mdtools/combine-files/test_files/test_comments.conf.expected
🪛 GitHub Actions: Python Code Quality Check for combine-files tool
tools/combine-files/combine_files.py
[error] 1615-1615: DOCTEST: _add_config_header example failed. Expected header content to be added to content but was not.
[error] 1475-1475: DOCTEST: _get_git_info example failed due to a PermissionError during temporary directory cleanup on Windows (WinError 32/5).
[error] 1498-1498: DOCTEST: _get_git_info example failed due to a PermissionError during temporary directory cleanup on Windows (WinError 32/5).
[error] 1334-1334: DOCTEST: _get_git_status example failed due to a PermissionError during temporary directory cleanup on Windows (WinError 32/5).
[error] 1355-1355: DOCTEST: _get_git_status example failed due to a PermissionError during temporary directory cleanup on Windows (WinError 32/5).
🪛 Ruff (0.14.8)
tools/combine-files/combine_files.py
1567-1572: Consider moving this statement to an else block
(TRY300)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (13)
.github/workflows/tools-combine-files-quality.yml (1)
58-88: LGTM! Previous issue resolved.The Windows CI job is properly configured. The previous concern about the non-existent
git-for-windows/setup-git@v2action has been addressed—the current implementation uses only standard actions (actions/checkout@v4,actions/setup-python@v5) and relies on the pre-installed Git onwindows-latestrunners.Running doctests explicitly on Windows (line 84) is valuable for catching platform-specific issues like path handling differences.
tools/combine-files/combine_files.py (1)
1579-1653: Git metadata integration looks good.The header generation correctly integrates Git metadata when available and falls back gracefully when not in a repository. The status display mapping (lines 1641-1647) provides user-friendly descriptions.
The change from "From:" to "From file:" with the leading blank line improves visual separation between file sections.
tools/combine-files/test_files/test_script_with_args.conf.expected (1)
1-8: Test fixture correctly updated for new header format.The changes align with the implementation in
combine_files.py:
- Leading blank line for visual separation
- "From file:" label matches the updated header generation
- REGEX placeholders correctly validate dynamic Git metadata (repo, path, commit hash, status)
tools/combine-files/test_files/test_path_variables.conf.expected (1)
1-8: Consistent header update.The fixture follows the same pattern as other test files, correctly incorporating the new header format and Git metadata placeholders.
tools/combine-files/test_files/test_comments.conf.expected (1)
1-8: Header updates applied consistently.The fixture correctly reflects the new header format with Git metadata placeholders.
tools/combine-files/test_files/test_nonstring_variables.conf.expected (1)
1-8: Strict commit hash regex is appropriate for test environment.The regex pattern
[0-9a-f]{12}enforces a valid 12-character hex commit hash. While the implementation can return "unknown" for repos without commits, this strictness is intentional for the test environment where a proper git history always exists, helping catch unexpected scenarios.tools/combine-files/test_files/test_multiple_scripts.conf.expected (1)
1-8: Fixture properly updated.Header changes are consistent with the implementation and other test fixtures.
tools/combine-files/test_files/test_multiple_commands.conf.expected (1)
1-8: Final fixture correctly updated.Header format matches the implementation and maintains consistency across all test fixtures.
tools/combine-files/test_files/test_multiline_variable.conf.expected (1)
1-7: LGTM!The test fixture correctly uses regex placeholders to match dynamic Git metadata. The patterns appropriately cover:
- Repository name (any string)
- File path in repo (any string)
- 12-character hex commit hash
- Status values with optional parenthetical details
tools/combine-files/test_files/run_tests.py (3)
55-92: Clean encapsulation of command execution.Good separation of concerns. The success validation via stderr message is appropriate since click outputs to stderr by default.
95-127: LGTM!Good UX with output limiting (10 mismatches, 20 diff lines) to keep failure output manageable while still being informative.
171-185: Clean refactor using the new comparison and reporting helpers.The flow is clear and the regex-aware comparison integrates well. The static analysis hint about moving
return Falseto anelseblock (TRY300) is a style preference - the current structure is readable.tools/combine-files/README.md (1)
192-212: Clear and comprehensive documentation of the new Git metadata feature.The example header block and field descriptions are well-written. The status values and their meanings are clearly documented, and the fallback behavior for non-git files is properly noted.
daniel-falk
force-pushed
the
add-git-info-to-script-to-combine
branch
3 times, most recently
from
264449e to
205dfa4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tools/combine-files/README.md (1)
1-50: Consider documenting platform compatibility.As per coding guidelines for
tools/**, READMEs should explain how tools work across Windows, Linux, and macOS or clarify which environments are supported. The README could benefit from an explicit statement about platform compatibility for the git metadata feature (e.g., "GitPython is cross-platform" or similar). This is optional but would align with documentation best practices.tools/combine-files/test_files/run_tests.py (2)
177-185: Restructure the conditional for clarity.The code would be clearer with an explicit
elseblock. This makes the control flow more obvious and aligns with the static analysis suggestion.Apply this diff:
if matches: print(" [OK]") print() return True - - _report_test_failure( - mismatches, expected_lines, actual_lines, expected_file - ) - return False + else: + _report_test_failure( + mismatches, expected_lines, actual_lines, expected_file + ) + return False
187-190: Consider catching more specific exceptions.The broad
Exceptioncatch could mask unexpected errors in the test harness. For a test runner, it's important to distinguish between expected test failures and unexpected errors in the test infrastructure itself.Consider catching specific exceptions or at least including exception details in the output:
except Exception as e: - print(f" X FAILED: Exception: {e}") + import traceback + print(f" X FAILED: Exception: {e}") + print(f" Traceback:\n{traceback.format_exc()}") print() return FalseThis helps distinguish between test failures and bugs in the test harness itself.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(3 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(6 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(3 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
✅ Files skipped from review due to trivial changes (1)
- tools/combine-files/test_files/test_whitespace.conf.expected
🚧 Files skipped from review as they are similar to previous changes (6)
- tools/combine-files/requirements.txt
- tools/combine-files/test_files/test_both_inline.conf.expected
- tools/combine-files/test_files/test_default_values.conf.expected
- tools/combine-files/test_files/test_comments.conf.expected
- tools/combine-files/test_files/test_starlark_inline.conf.expected
- tools/combine-files/test_files/test_shell_inline.conf.expected
🧰 Additional context used
📓 Path-based instructions (3)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_multiple_scripts.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/test_files/test_multiple_commands.conf.expectedtools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/README.mdtools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/run_tests.py
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_multiple_scripts.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/test_files/test_multiple_commands.conf.expectedtools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/combine_files.pytools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/README.mdtools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/run_tests.py
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Documentation files should clearly communicate the dual audience: (1) server-side dashboard users who want to keep agent with bundled configs, and (2) edge device developers who want to customize agent behavior. Ensure examples and instructions are appropriate for the intended skill level and use case. Since this is a public repository, we should not include any sensitive information, the instructions should be easily understandable for a wide audience, and we should avoid using any jargon or technical terms that are not commonly used.
Headings should not include special characters like emojis or backticks. The table of contents should be generated with themarkdown-toctool, meaning that we should use the<!-- toc -->and<!-- tocstop -->tags.
Files:
tools/combine-files/README.md
🧠 Learnings (9)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-08-08T09:31:58.742Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T09:31:58.742Z
Learning: In project-strobe-color-from-github-workflow/README.md, the FixedIT Data Agent “Extra env” semicolon-separated KEY=VALUE string must not include quotes around values. Use unquoted placeholders (e.g., GITHUB_WORKFLOW=your_workflow_name).
Applied to files:
.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-08-10T15:15:33.003Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:126-127
Timestamp: 2025-08-10T15:15:33.003Z
Learning: In the FixedIT Data Agent Extra env configuration, spaces in values are acceptable without quotes. For the GITHUB_WORKFLOW variable in project-strobe-color-from-github-workflow, using a placeholder with spaces like "Your Workflow Name" is preferred as it demonstrates proper configuration for the common case where GitHub workflow names contain spaces.
Applied to files:
.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-08-08T10:22:37.500Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T10:22:37.500Z
Learning: In project-strobe-color-from-github-workflow, the GITHUB_WORKFLOW environment variable is currently not used by any config or script; it appears only in README examples. If later introduced for GitHub API filtering, use a workflow file name (e.g., ci.yml) or numeric id and avoid quotes because the Data Agent passes quotes literally into the value, which can break URL paths.
Applied to files:
.github/workflows/tools-combine-files-quality.ymltools/combine-files/test_files/test_path_variables.conf.expected
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
.github/workflows/tools-combine-files-quality.ymltools/combine-files/combine_files.pytools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/README.md
📚 Learning: 2025-08-10T14:54:48.316Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:350-351
Timestamp: 2025-08-10T14:54:48.316Z
Learning: In the fixedit-data-agent-examples repository, shell portability requirements (such as Axis devices using POSIX /bin/sh instead of bash) should be documented in a general scripting guide rather than repeated in individual project README files. This approach was confirmed by daniel-falk for better documentation organization.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-09-03T14:18:52.406Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-11-07T15:57:47.716Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 26
File: project-qr-code-decoder-http-push/combined.conf:190-190
Timestamp: 2025-11-07T15:57:47.716Z
Learning: In project-qr-code-decoder-http-push/combined.conf (and similar Telegraf config files with embedded Starlark code), comments within Starlark source strings intentionally avoid apostrophes (e.g., "Dont" instead of "Don't") because single quotes break syntax highlighting in VS Code.
Applied to files:
tools/combine-files/README.md
🪛 Ruff (0.14.8)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
187-187: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (21)
tools/combine-files/README.md (1)
192-212: Clear and comprehensive documentation of git metadata headers.The git metadata documentation is well-structured and includes concrete examples. The header format is clearly shown, all four fields are well-described, status values are comprehensive, and the fallback behavior for non-git files is documented.
tools/combine-files/test_files/test_multiple_mixed.conf.expected (1)
1-7: LGTM! Test fixture header updated correctly.The header format matches the new Git metadata integration pattern consistently applied across all test fixtures. The REGEX placeholders will properly validate the dynamic Git information in the test runner.
.github/workflows/tools-combine-files-quality.yml (3)
7-7: LGTM! Workflow self-triggers on changes.Including the workflow file itself in the trigger paths ensures that workflow modifications are validated through CI, which is a best practice.
Also applies to: 11-11
33-33: LGTM! Clear job naming.Renaming to
integration-tests-linuxprovides clarity now that platform-specific jobs exist, making the workflow structure more maintainable.
58-88: LGTM! Windows testing adds valuable platform coverage.The Windows job is well-structured and includes platform-specific doctest execution to catch Windows-only issues (like path separator handling and file handle cleanup). The job appropriately mirrors the Linux configuration.
tools/combine-files/combine_files.py (6)
21-21: LGTM! GitPython import added correctly.The import is properly placed and necessary for the new Git metadata functionality.
923-932: LGTM! Cleaner doctest output.Using
redirect_stderrto suppress expected error output improves test readability while maintaining test coverage for the error case.
1312-1313: LGTM! Cross-platform path normalization.Using
as_posix()ensures consistent forward-slash paths across platforms, which is essential for Git path matching and cross-platform compatibility.
1319-1456: LGTM! Well-implemented Git status detection.The function correctly implements Git status precedence and handles edge cases:
- POSIX path normalization ensures cross-platform compatibility
- Status precedence (untracked → modified → staged → clean) matches Git semantics
- Proper error handling with "unknown" fallback
- Comprehensive doctests with
repo.close()for Windows compatibility
1459-1588: LGTM! Robust Git info retrieval.The function is well-implemented with:
- Proper error handling for non-Git directories and Git errors
repo.close()in a finally block ensures resource cleanup on Windows- Fallback to "unknown" for repos without commits
- Clean integration with
_get_git_statusfor status determination- Comprehensive doctests covering various scenarios
1590-1663: LGTM! Enhanced headers with Git metadata.The header improvements provide valuable context:
- Extra leading blank line improves visual separation between files
- "From file:" is more explicit than "From:"
- Git metadata (repo, path, commit, status) helps track file origins
- Human-readable status descriptions (e.g., "modified (uncommitted changes)") improve clarity
- Graceful fallback message when not in a Git repository
tools/combine-files/test_files/test_script_with_args.conf.expected (1)
1-7: LGTM! Consistent test fixture header update.The header changes match the pattern applied across all test fixtures, ensuring consistent validation of the Git metadata integration.
tools/combine-files/test_files/test_path_variables.conf.expected (1)
1-7: LGTM! Consistent test fixture header update.The header format aligns with the Git metadata integration pattern used across all test fixtures.
tools/combine-files/test_files/test_nonstring_variables.conf.expected (1)
1-7: LGTM! Test fixture header with strict commit pattern.The header update is consistent with other fixtures. The strict
[0-9a-f]{12}pattern for commits is intentional—it serves as a safety check since test environments always have proper Git history, as confirmed in past review discussions.tools/combine-files/test_files/test_multiline.conf.expected (1)
1-7: LGTM! Consistent test fixture header update.The header changes follow the established Git metadata integration pattern across all test fixtures.
tools/combine-files/test_files/test_multiple_scripts.conf.expected (1)
1-7: LGTM! Consistent test fixture header update.The header format matches the Git metadata integration pattern consistently applied across all test fixtures in this PR.
tools/combine-files/test_files/test_multiple_commands.conf.expected (1)
1-8: LGTM! Header format and regex patterns are well-structured.The new header format with "From file:" and the Git metadata placeholders align with the PR objectives. The regex patterns correctly capture:
- Repository and file path with flexible
.*matching- 12-character short commit hash with
[0-9a-f]{12}- Status with enumerated values and optional context in parentheses
tools/combine-files/test_files/test_multiline_variable.conf.expected (1)
1-8: LGTM! Consistent header format across test fixtures.The header changes match the pattern used in other test fixtures, maintaining consistency across the test suite.
tools/combine-files/test_files/run_tests.py (3)
55-92: LGTM! Helper function properly encapsulates command execution.The function is well-structured with clear error handling. The subprocess security warning (S603) is not applicable here since the script path is controlled (from
Path(__file__).parent / "combine_files.py") and all arguments are test-controlled paths and flags.
95-127: LGTM! Clear and user-friendly failure reporting.The function provides excellent UX by showing the first 10 mismatches with a fallback to unified diff, and limits output to prevent overwhelming the user.
12-52: Document minimum Python version before consideringstrict=Trueforzip().The suggestion to add
strict=Trueto thezip()call at line 32 cannot be safely implemented without first establishing a documented minimum Python version requirement. Thestrict=parameter was introduced in Python 3.10, but the tool currently has no minimum Python version specified in its requirements or documentation. Addingstrict=Truewould break compatibility with Python 3.9 and earlier.If the project targets Python 3.10+, either:
- Add a Python version constraint to
requirements.txt(e.g.,python_requires=">=3.10"in setup.py orrequires-python = ">=3.10"in pyproject.toml), or- Document the minimum Python version in the README
Once a minimum Python 3.10+ requirement is established and documented, the
strict=Trueparameter can be safely added.
daniel-falk
force-pushed
the
add-git-info-to-script-to-combine
branch
2 times, most recently
from
7d53105 to
b168aa2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tools/combine-files/combine_files.py (1)
1324-1508: Fix pylint R0911 (“too many return statements”) in_get_git_status(currently breaks CI).This is still failing the “Python Code Quality Check for combine-files tool” per the pipeline output. You can keep semantics while switching to a single return and avoiding deep nesting.
def _get_git_status(repo, file_path_in_repo): @@ - file_path_normalized = file_path_in_repo.as_posix() - - # Check if file is untracked - try: - untracked_normalized = {Path(path).as_posix() for path in repo.untracked_files} - if file_path_normalized in untracked_normalized: - return file_path_normalized, "untracked" - except git.exc.GitCommandError: - return file_path_normalized, "unknown" - - # Check if file has unstaged changes (working tree differs from index) - try: - if repo.index.diff(None, paths=file_path_normalized): - return file_path_normalized, "modified" - except (git.exc.GitCommandError, ValueError): - return file_path_normalized, "unknown" - - # Check if file has staged changes (index differs from HEAD) - try: - if repo.index.diff(repo.head.commit, paths=file_path_normalized): - return file_path_normalized, "staged" - except (ValueError, AttributeError): - # No commits yet (ValueError: Reference does not exist) or detached HEAD - # If we got here, the file is not untracked and has no unstaged changes, - # so it must be in the index. With no commits, this means it's staged - # (ready for the first commit). - return file_path_normalized, "staged" - except git.exc.GitCommandError: - # Other git errors - we can't determine the status - return file_path_normalized, "unknown" - - return file_path_normalized, "clean" + file_path_normalized = file_path_in_repo.as_posix() + status = "clean" + + # Untracked + try: + untracked_normalized = {Path(path).as_posix() for path in repo.untracked_files} + except git.exc.GitCommandError: + status = "unknown" + else: + if file_path_normalized in untracked_normalized: + status = "untracked" + + # Unstaged (index vs working tree) + if status == "clean": + try: + if repo.index.diff(None, paths=file_path_normalized): + status = "modified" + except (git.exc.GitCommandError, ValueError): + status = "unknown" + + # Staged (index vs HEAD) / no-commit handling + if status == "clean": + try: + head_commit = repo.head.commit + except (ValueError, AttributeError): + status = "staged" + else: + try: + if repo.index.diff(head_commit, paths=file_path_normalized): + status = "staged" + except git.exc.GitCommandError: + status = "unknown" + + return file_path_normalized, status
🧹 Nitpick comments (1)
tools/combine-files/combine_files.py (1)
20-22: Consider making GitPython an optional dependency (better UX when running the script “standalone”).Right now
import githard-fails even though your runtime behavior has a “No version tracking info found” fallback. If someone runs the tool without installing requirements, they’ll crash at import-time instead of getting the fallback.import click -import git +try: + import git # GitPython +except ImportError: # pragma: no cover + git = None import tomlkitYou’d then guard
_get_git_info()withif git is None: return None. (Please double-check GitPython import/name and your pinned version behavior in this repo.)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(3 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(7 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(3 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
✅ Files skipped from review due to trivial changes (1)
- tools/combine-files/test_files/test_multiple_scripts.conf.expected
🚧 Files skipped from review as they are similar to previous changes (9)
- tools/combine-files/test_files/test_starlark_inline.conf.expected
- tools/combine-files/test_files/test_multiline_variable.conf.expected
- tools/combine-files/test_files/test_both_inline.conf.expected
- tools/combine-files/test_files/test_default_values.conf.expected
- tools/combine-files/test_files/test_nonstring_variables.conf.expected
- tools/combine-files/README.md
- tools/combine-files/test_files/test_whitespace.conf.expected
- tools/combine-files/test_files/test_multiple_commands.conf.expected
- tools/combine-files/test_files/test_multiline.conf.expected
🧰 Additional context used
📓 Path-based instructions (2)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/requirements.txttools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/combine_files.py
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_path_variables.conf.expectedtools/combine-files/requirements.txttools/combine-files/test_files/run_tests.pytools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_script_with_args.conf.expectedtools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/combine_files.py
🧠 Learnings (7)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-08-08T09:31:58.742Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T09:31:58.742Z
Learning: In project-strobe-color-from-github-workflow/README.md, the FixedIT Data Agent “Extra env” semicolon-separated KEY=VALUE string must not include quotes around values. Use unquoted placeholders (e.g., GITHUB_WORKFLOW=your_workflow_name).
Applied to files:
.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-08-10T15:15:33.003Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:126-127
Timestamp: 2025-08-10T15:15:33.003Z
Learning: In the FixedIT Data Agent Extra env configuration, spaces in values are acceptable without quotes. For the GITHUB_WORKFLOW variable in project-strobe-color-from-github-workflow, using a placeholder with spaces like "Your Workflow Name" is preferred as it demonstrates proper configuration for the common case where GitHub workflow names contain spaces.
Applied to files:
.github/workflows/tools-combine-files-quality.yml
📚 Learning: 2025-08-08T10:22:37.500Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:114-114
Timestamp: 2025-08-08T10:22:37.500Z
Learning: In project-strobe-color-from-github-workflow, the GITHUB_WORKFLOW environment variable is currently not used by any config or script; it appears only in README examples. If later introduced for GitHub API filtering, use a workflow file name (e.g., ci.yml) or numeric id and avoid quotes because the Data Agent passes quotes literally into the value, which can break URL paths.
Applied to files:
.github/workflows/tools-combine-files-quality.ymltools/combine-files/test_files/test_path_variables.conf.expected
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
.github/workflows/tools-combine-files-quality.ymltools/combine-files/combine_files.py
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/test_files/test_path_variables.conf.expected
📚 Learning: 2025-11-07T15:57:47.716Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 26
File: project-qr-code-decoder-http-push/combined.conf:190-190
Timestamp: 2025-11-07T15:57:47.716Z
Learning: In project-qr-code-decoder-http-push/combined.conf (and similar Telegraf config files with embedded Starlark code), comments within Starlark source strings intentionally avoid apostrophes (e.g., "Dont" instead of "Don't") because single quotes break syntax highlighting in VS Code.
Applied to files:
tools/combine-files/test_files/test_comments.conf.expected
🪛 GitHub Actions: Python Code Quality Check for combine-files tool
tools/combine-files/combine_files.py
[error] 1324-1324: pylint: Too many return statements (R0911) (combine_files.py:1324:0).
🪛 Ruff (0.14.8)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
187-187: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (9)
tools/combine-files/test_files/test_multiple_mixed.conf.expected (1)
1-7: LGTM: Fixture updates align with Git-aware header generation.The header format updates across all test fixtures (leading blank line, "From file:" wording, and REGEX placeholders for Git metadata) are consistent and properly support the new Git-aware header generation feature.
.github/workflows/tools-combine-files-quality.yml (2)
7-7: Good practice: Self-triggering workflow.Including the workflow file itself in trigger paths ensures CI runs when the workflow definition changes, catching configuration issues early.
Also applies to: 11-11
58-88: Valuable addition: Windows integration testing.The Windows job properly validates cross-platform behavior, particularly important for the CRLF/LF normalization and Git integration features. Running doctests on Windows catches platform-specific issues that Linux-only CI would miss.
tools/combine-files/test_files/run_tests.py (3)
12-53: Well-designed regex comparison with proper validation.The
compare_with_regexfunction correctly handles both exact matches and regex patterns, with explicit length checking before comparison and full-line regex matching to prevent partial matches.
55-93: Good refactoring: Command execution encapsulated.The
_run_combine_commandhelper centralizes subprocess invocation and success validation, improving maintainability and reducing duplication.
95-128: Enhanced error reporting with mismatch details.The
_report_test_failurefunction provides clear, actionable feedback by showing specific mismatches for regex comparisons and falling back to unified diff when needed.tools/combine-files/combine_files.py (3)
1313-1321: POSIX-normalizing relative paths is the right call for cross-platform headers.
1510-1639:_get_git_info()resource handling (repo.close) and fallbacks look solid.Optional: if this tool is commonly combining many files from the same repo, consider caching repo root discovery to avoid calling
git.Repo(..., search_parent_directories=True)per file.
1686-1714: Header format update + git metadata inclusion is clean and readable.
daniel-falk
force-pushed
the
add-git-info-to-script-to-combine
branch
from
b168aa2 to
6c78ee3
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/combine-files/combine_files.py (1)
1314-1323: Black is failing on this file — please runblackand commit the reformat.
🧹 Nitpick comments (5)
tools/combine-files/test_files/run_tests.py (3)
12-52: Escape non-regex text when building full-line regex; optionally support multiple placeholders; considerzip(..., strict=True).Right now, any regex metacharacters outside
{{REGEX:...}}are treated as regex, which can create accidental matches. Also Ruff flagszip()withoutstrict=(Py3.10+).def compare_with_regex(expected_lines, actual_lines): @@ - mismatches = [] - for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines)): + token_re = re.compile(r"\{\{REGEX:(.+?)\}\}") + mismatches = [] + for i, (expected, actual) in enumerate(zip(expected_lines, actual_lines, strict=True)): # Check if this line uses regex pattern - regex_match = re.search(r"\{\{REGEX:(.+?)\}\}", expected) - if regex_match: - pattern = regex_match.group(1) - # Replace the {{REGEX:...}} with the pattern for matching - expected_pattern = expected.replace(f"{{{{REGEX:{pattern}}}}}", pattern) - # Ensure pattern matches the full line (strip newlines for comparison) - expected_stripped = expected_pattern.rstrip("\n\r") + matches = list(token_re.finditer(expected)) + if matches: + # Build a regex for the full line by escaping literal text outside placeholders. + expected_stripped = expected.rstrip("\n\r") actual_stripped = actual.rstrip("\n\r") - # Use fullmatch to ensure the entire line matches the pattern - if not re.fullmatch(expected_stripped, actual_stripped): + regex_parts = [] + last = 0 + for m in matches: + regex_parts.append(re.escape(expected_stripped[last : m.start()])) + regex_parts.append(m.group(1)) + last = m.end() + regex_parts.append(re.escape(expected_stripped[last:])) + expected_regex = "".join(regex_parts) + if not re.fullmatch(expected_regex, actual_stripped): mismatches.append( - f"Line {i+1}: regex pattern '{pattern}' did not match '{actual_stripped}'" + f"Line {i+1}: regex did not match actual line: {actual_stripped!r}" ) else: # Exact match required if expected != actual: mismatches.append(f"Line {i+1}: expected '{expected}', got '{actual}'")Verification notes:
zip(..., strict=True)requires Python 3.10+. If your minimum is <3.10, keepzip()and add a comment/ignore for Ruff B905 instead.
55-93: Subprocess hardening + reduce lint noise for S603.
subprocess.run(...)is executing a known script with controlled args, but Ruff’s S603 will still complain; also consider a timeout to avoid hung CI.- result = subprocess.run(cmd, capture_output=True, text=True, check=False) + result = subprocess.run( # noqa: S603 + cmd, + capture_output=True, + text=True, + check=False, + timeout=60, + ) @@ - if "Successfully" not in result.stderr: + if "Successfully combined" not in result.stderr: return False, "No success message in output"
157-190: Make file reads deterministic across OS; avoid blindexcept Exception.Windows vs Linux default encodings can differ; and catching
Exceptionhides useful failure context.- expected_content = expected_file.read_text() - actual_content = output_file.read_text() + expected_content = expected_file.read_text(encoding="utf-8") + actual_content = output_file.read_text(encoding="utf-8") @@ - except Exception as e: - print(f" X FAILED: Exception: {e}") + except Exception as e: + # Keep runner resilient, but include enough context for debugging. + print(f" X FAILED: Exception: {e!r}") print() return Falsetools/combine-files/combine_files.py (2)
992-1006: Line-ending normalization is fine; consider decoding robustness.If a
.shcontains non-UTF8 bytes,encoding="utf-8"will crash the tool. Considererrors="surrogateescape"(or reading bytes and base64’ing bytes directly) if you want to be resilient.
1511-1640:_get_git_info: consider handling non-working-tree repos (working_dir=None).
Path(repo.working_dir)will throw ifworking_dirisNone(bare repo / unexpected state). A small guard makes the tool more robust.- repo_root = Path(repo.working_dir) + if not repo.working_dir: + return None + repo_root = Path(repo.working_dir)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/tools-combine-files-quality.yml(3 hunks)tools/combine-files/README.md(1 hunks)tools/combine-files/combine_files.py(7 hunks)tools/combine-files/requirements.txt(1 hunks)tools/combine-files/test_files/run_tests.py(3 hunks)tools/combine-files/test_files/test_both_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_comments.conf.expected(1 hunks)tools/combine-files/test_files/test_default_values.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline.conf.expected(1 hunks)tools/combine-files/test_files/test_multiline_variable.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_commands.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_mixed.conf(0 hunks)tools/combine-files/test_files/test_multiple_mixed.conf.expected(1 hunks)tools/combine-files/test_files/test_multiple_scripts.conf.expected(1 hunks)tools/combine-files/test_files/test_nonstring_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_path_variables.conf.expected(1 hunks)tools/combine-files/test_files/test_script_with_args.conf.expected(1 hunks)tools/combine-files/test_files/test_shell_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_starlark_inline.conf.expected(1 hunks)tools/combine-files/test_files/test_whitespace.conf.expected(1 hunks)
💤 Files with no reviewable changes (1)
- tools/combine-files/test_files/test_multiple_mixed.conf
🚧 Files skipped from review as they are similar to previous changes (9)
- tools/combine-files/test_files/test_both_inline.conf.expected
- tools/combine-files/test_files/test_path_variables.conf.expected
- tools/combine-files/test_files/test_whitespace.conf.expected
- .github/workflows/tools-combine-files-quality.yml
- tools/combine-files/README.md
- tools/combine-files/test_files/test_multiple_scripts.conf.expected
- tools/combine-files/test_files/test_multiple_commands.conf.expected
- tools/combine-files/test_files/test_starlark_inline.conf.expected
- tools/combine-files/test_files/test_script_with_args.conf.expected
🧰 Additional context used
📓 Path-based instructions (2)
tools/**
⚙️ CodeRabbit configuration file
This directory contains developer tools that run on the developer's local machine to streamline the development and deployment workflow for FixedIT Data Agent projects. These tools are not deployed to Axis devices. Tools should be well-documented with comprehensive README files that include installation instructions, usage examples, and explanations of how they work and why they are needed. Python tools should use
clickfor argument parsing and pathlib for path handling, expose helpful command-line help texts, include arequirements.txtfile for easy installation and include comprehensive docstrings with doctests where appropriate. Tools should be generic and reusable across multiple projects rather than project-specific (otherwise the tool should be placed in the project-specific directory in a subfolder calledtools/). Each tool should have its own subdirectory undertools/(e.g.,tools/combine-files/). Documentation should be placed in the root of the tool directory and should be namedREADME.md. Tool READMEs should follow standard documentation practices including table of contents with<!-- toc -->markers. The README should consider that users might use Windows, Linux or MacOS and should explain how the tools work in the different environments or which environments it does work under.
Files:
tools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/run_tests.pytools/combine-files/requirements.txttools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/combine_files.py
**/*
⚙️ CodeRabbit configuration file
This repository serves FixedIT Data Agent users across a spectrum from plug-and-play dashboard deployment to advanced edge device customization. Consider whether changes maintain accessibility for both DevOps professionals (server-side focus) and developers (edge customization focus). If new features are added or existing ones changed significantly, ensure documentation clearly explains the intended audience and usage level. We use prettier for formatting of common file formats like markdown, yaml, json, etc. Example projects should be placed in the repo-root in a directory named project-*/. Whenever referencing Axis device model names like "D4100-VE mk II" or "M3045", prefix it with "AXIS" in capital letters (e.g. "AXIS D4100-VE mk II"). When using Axis as a company name, use "Axis Communications", note that all-caps is only used when "AXIS" is used in their product names, not their company name. When using the name of an Axis product, google on the name to verify that it is correctly identified. Avoid using 'cameras' or 'Axis cameras' unless the solution in related to visual analytics, otherwise prefer using 'Axis devices' to show that the FixedIT Data Agent also works with strobes, speakers, door stations, etc. Images used for the README should be placed in a directory called .images/ in the affected project folder. These images might also be referred to from other sources like the top-level README.md file.
Files:
tools/combine-files/test_files/test_multiple_mixed.conf.expectedtools/combine-files/test_files/test_default_values.conf.expectedtools/combine-files/test_files/run_tests.pytools/combine-files/requirements.txttools/combine-files/test_files/test_multiline.conf.expectedtools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/test_files/test_comments.conf.expectedtools/combine-files/test_files/test_shell_inline.conf.expectedtools/combine-files/test_files/test_multiline_variable.conf.expectedtools/combine-files/combine_files.py
🧠 Learnings (6)
📓 Common learnings
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
📚 Learning: 2025-08-07T15:23:05.927Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:57-62
Timestamp: 2025-08-07T15:23:05.927Z
Learning: In the fixedit-ai/fixedit-data-agent-examples repository, when GitHub generates anchors for headings that contain backticks with spaces around them (like `### `filename` - Description`), it removes the backticks but preserves those spaces as dashes in the anchor, creating triple dashes (---) between the filename and description parts. TOC generators may not handle this correctly and need manual adjustment.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expectedtools/combine-files/combine_files.py
📚 Learning: 2025-08-10T14:54:48.316Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 5
File: project-strobe-color-from-github-workflow/README.md:350-351
Timestamp: 2025-08-10T14:54:48.316Z
Learning: In the fixedit-data-agent-examples repository, shell portability requirements (such as Axis devices using POSIX /bin/sh instead of bash) should be documented in a general scripting guide rather than repeated in individual project README files. This approach was confirmed by daniel-falk for better documentation organization.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-09-03T14:18:52.406Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 18
File: dashboard-deployments/system-monitoring-influxdb2-flux-grafana/provisioning/dashboards/overview_of_devices.json:1121-1130
Timestamp: 2025-09-03T14:18:52.406Z
Learning: When fixing unit mismatches in Grafana dashboards, daniel-falk prefers changing the panel unit configuration to match the data rather than transforming the query values, choosing simplicity over data conversion when both approaches are valid.
Applied to files:
tools/combine-files/test_files/test_nonstring_variables.conf.expected
📚 Learning: 2025-11-07T15:57:47.716Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 26
File: project-qr-code-decoder-http-push/combined.conf:190-190
Timestamp: 2025-11-07T15:57:47.716Z
Learning: In project-qr-code-decoder-http-push/combined.conf (and similar Telegraf config files with embedded Starlark code), comments within Starlark source strings intentionally avoid apostrophes (e.g., "Dont" instead of "Don't") because single quotes break syntax highlighting in VS Code.
Applied to files:
tools/combine-files/test_files/test_comments.conf.expected
📚 Learning: 2025-12-08T18:36:51.918Z
Learnt from: daniel-falk
Repo: fixedit-ai/fixedit-data-agent-examples PR: 32
File: tools/combine-files/test_files/test_nonstring_variables.conf.expected:8-11
Timestamp: 2025-12-08T18:36:51.918Z
Learning: In tools/combine-files/combine_files.py, the --temporary-expand-var flag performs temporary variable expansion only during processing to correctly parse TOML and resolve file paths. The original variable references (e.g., ${VAR}) are intentionally preserved in the output for runtime expansion by the target application (Telegraf). This allows the tool to process configs with variables while keeping them dynamic for deployment.
Applied to files:
tools/combine-files/test_files/test_multiline_variable.conf.expected
🪛 GitHub Actions: Python Code Quality Check for combine-files tool
tools/combine-files/combine_files.py
[error] 1320-1323: Black formatting check failed. 1 file would be reformatted by Black. Run 'black' to format the code. Command: black --check --diff .
🪛 Ruff (0.14.8)
tools/combine-files/test_files/run_tests.py
32-32: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
81-81: subprocess call: check for execution of untrusted input
(S603)
185-185: Consider moving this statement to an else block
(TRY300)
187-187: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (10)
tools/combine-files/test_files/test_default_values.conf.expected (1)
1-8: LGTM! Consistent Git metadata header pattern.The updated header format with per-file Git metadata placeholders is well-structured and consistent with the PR objectives.
tools/combine-files/test_files/test_comments.conf.expected (1)
1-8: LGTM! Header format matches expected pattern.The Git metadata placeholders are correctly formatted and consistent across test fixtures.
tools/combine-files/test_files/test_shell_inline.conf.expected (1)
1-8: LGTM! Consistent header implementation.The per-file Git metadata header follows the established pattern correctly.
tools/combine-files/test_files/test_nonstring_variables.conf.expected (1)
1-8: LGTM! Header pattern is correct.The strict commit hash regex pattern (
[0-9a-f]{12}) is intentionally maintained to catch unexpected scenarios in the test environment, as discussed in previous reviews.tools/combine-files/test_files/test_multiple_mixed.conf.expected (1)
1-8: LGTM! Header correctly formatted.The Git metadata placeholders match the established pattern across all test fixtures.
tools/combine-files/test_files/test_multiline.conf.expected (1)
1-8: LGTM! Consistent Git metadata integration.The header format aligns with the Git metadata enhancement implemented across the test suite.
tools/combine-files/requirements.txt (1)
3-7: LGTM! Security vulnerabilities addressed.The GitPython version constraint
>=3.1.41,<4.0.0correctly addresses known CVEs (CVE-2022-24439, CVE-2023-40590, CVE-2024-22190) with helpful comments and a reference link to security advisories.tools/combine-files/test_files/test_multiline_variable.conf.expected (1)
1-8: LGTM! Header format is consistent.The Git metadata header implementation matches the pattern used across all test fixtures, ensuring uniform test expectations.
tools/combine-files/combine_files.py (2)
920-935: Doctest stderr suppression is the right move.
1687-1714: Header formatting + POSIX path normalization looks consistent.
| def _get_git_status(repo, file_path_in_repo): # pylint: disable=too-many-return-statements | ||
| """ | ||
| Determine the git status of a file in a repository. | ||
|
|
||
| Args: | ||
| repo: GitPython Repo object | ||
| file_path_in_repo: Path object relative to repository root | ||
|
|
||
| Returns: | ||
| Tuple (file_path_normalized, status) where: | ||
| - file_path_normalized: POSIX-normalized path string | ||
| - status: One of 'untracked', 'modified', 'staged', 'clean', 'unknown' | ||
|
|
||
| Examples: | ||
| >>> import tempfile | ||
| >>> import subprocess | ||
| >>> from pathlib import Path | ||
| >>> import git | ||
| >>> # Test clean file | ||
| >>> with tempfile.TemporaryDirectory() as tmpdir: | ||
| ... repo_dir = Path(tmpdir) / "test_repo" | ||
| ... repo_dir.mkdir() | ||
| ... _ = subprocess.run(["git", "init"], cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.email", "test@example.com"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.name", "Test"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... test_file = repo_dir / "test.conf" | ||
| ... _ = test_file.write_text("content") | ||
| ... _ = subprocess.run(["git", "add", "test.conf"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "commit", "-m", "Initial"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... repo = git.Repo(repo_dir) | ||
| ... file_path = test_file.relative_to(repo_dir) | ||
| ... path_norm, status = _get_git_status(repo, file_path) | ||
| ... repo.close() | ||
| ... status == 'clean' | ||
| True | ||
|
|
||
| >>> # Test modified file (unstaged changes) | ||
| >>> with tempfile.TemporaryDirectory() as tmpdir: | ||
| ... repo_dir = Path(tmpdir) / "test_repo" | ||
| ... repo_dir.mkdir() | ||
| ... _ = subprocess.run(["git", "init"], cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.email", "test@example.com"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.name", "Test"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... test_file = repo_dir / "test.conf" | ||
| ... _ = test_file.write_text("original") | ||
| ... _ = subprocess.run(["git", "add", "test.conf"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "commit", "-m", "Initial"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = test_file.write_text("modified") | ||
| ... repo = git.Repo(repo_dir) | ||
| ... file_path = test_file.relative_to(repo_dir) | ||
| ... path_norm, status = _get_git_status(repo, file_path) | ||
| ... repo.close() | ||
| ... status == 'modified' | ||
| True | ||
|
|
||
| >>> # Test untracked file | ||
| >>> with tempfile.TemporaryDirectory() as tmpdir: | ||
| ... repo_dir = Path(tmpdir) / "test_repo" | ||
| ... repo_dir.mkdir() | ||
| ... _ = subprocess.run(["git", "init"], cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.email", "test@example.com"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.name", "Test"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... dummy = repo_dir / "dummy.txt" | ||
| ... _ = dummy.write_text("dummy") | ||
| ... _ = subprocess.run(["git", "add", "dummy.txt"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "commit", "-m", "Initial"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... test_file = repo_dir / "untracked.conf" | ||
| ... _ = test_file.write_text("untracked") | ||
| ... repo = git.Repo(repo_dir) | ||
| ... file_path = test_file.relative_to(repo_dir) | ||
| ... path_norm, status = _get_git_status(repo, file_path) | ||
| ... repo.close() | ||
| ... status == 'untracked' | ||
| True | ||
|
|
||
| >>> # Test staged file (no unstaged changes) | ||
| >>> with tempfile.TemporaryDirectory() as tmpdir: | ||
| ... repo_dir = Path(tmpdir) / "test_repo" | ||
| ... repo_dir.mkdir() | ||
| ... _ = subprocess.run(["git", "init"], cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.email", "test@example.com"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.name", "Test"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... test_file = repo_dir / "test.conf" | ||
| ... _ = test_file.write_text("original") | ||
| ... _ = subprocess.run(["git", "add", "test.conf"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "commit", "-m", "Initial"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = test_file.write_text("modified") | ||
| ... _ = subprocess.run(["git", "add", "test.conf"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... repo = git.Repo(repo_dir) | ||
| ... file_path = test_file.relative_to(repo_dir) | ||
| ... path_norm, status = _get_git_status(repo, file_path) | ||
| ... repo.close() | ||
| ... status == 'staged' | ||
| True | ||
|
|
||
| >>> # Test with empty git repo (no commits yet) | ||
| >>> with tempfile.TemporaryDirectory() as tmpdir: | ||
| ... repo_dir = Path(tmpdir) / "test_repo" | ||
| ... repo_dir.mkdir() | ||
| ... _ = subprocess.run(["git", "init"], cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.email", "test@example.com"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.name", "Test"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... test_file = repo_dir / "test.conf" | ||
| ... _ = test_file.write_text("content") | ||
| ... repo = git.Repo(repo_dir) | ||
| ... file_path = test_file.relative_to(repo_dir) | ||
| ... path_norm, status = _get_git_status(repo, file_path) | ||
| ... repo.close() | ||
| ... # File is untracked in empty repo | ||
| ... status == 'untracked' | ||
| True | ||
|
|
||
| >>> # Test with empty git repo - staged file (no commits yet) | ||
| >>> with tempfile.TemporaryDirectory() as tmpdir: | ||
| ... repo_dir = Path(tmpdir) / "test_repo" | ||
| ... repo_dir.mkdir() | ||
| ... _ = subprocess.run(["git", "init"], cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.email", "test@example.com"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... _ = subprocess.run(["git", "config", "user.name", "Test"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... test_file = repo_dir / "test.conf" | ||
| ... _ = test_file.write_text("content") | ||
| ... _ = subprocess.run(["git", "add", "test.conf"], | ||
| ... cwd=repo_dir, capture_output=True) | ||
| ... repo = git.Repo(repo_dir) | ||
| ... file_path = test_file.relative_to(repo_dir) | ||
| ... path_norm, status = _get_git_status(repo, file_path) | ||
| ... repo.close() | ||
| ... # File is staged (in index, ready for first commit) | ||
| ... status == 'staged' | ||
| True | ||
| """ | ||
| file_path_normalized = file_path_in_repo.as_posix() | ||
|
|
||
| # Check if file is untracked | ||
| try: | ||
| untracked_normalized = {Path(path).as_posix() for path in repo.untracked_files} | ||
| if file_path_normalized in untracked_normalized: | ||
| return file_path_normalized, "untracked" | ||
| except git.exc.GitCommandError: | ||
| return file_path_normalized, "unknown" | ||
|
|
||
| # Check if file has unstaged changes (working tree differs from index) | ||
| try: | ||
| if repo.index.diff(None, paths=file_path_normalized): | ||
| return file_path_normalized, "modified" | ||
| except (git.exc.GitCommandError, ValueError): | ||
| return file_path_normalized, "unknown" | ||
|
|
||
| # Check if file has staged changes (index differs from HEAD) | ||
| try: | ||
| if repo.index.diff(repo.head.commit, paths=file_path_normalized): | ||
| return file_path_normalized, "staged" | ||
| except (ValueError, AttributeError): | ||
| # No commits yet (ValueError: Reference does not exist) or detached HEAD | ||
| # If we got here, the file is not untracked and has no unstaged changes, | ||
| # so it must be in the index. With no commits, this means it's staged | ||
| # (ready for the first commit). | ||
| return file_path_normalized, "staged" | ||
| except git.exc.GitCommandError: | ||
| # Other git errors - we can't determine the status | ||
| return file_path_normalized, "unknown" | ||
|
|
||
| return file_path_normalized, "clean" |
There was a problem hiding this comment.
Empty repository without commits can misclassify ignored files as staged.
When a repository has no commits yet, repo.index.diff(repo.head.commit, ...) raises ValueError. The current exception handler returns "staged" unconditionally, but ignored files (which are excluded from repo.untracked_files) are not in the index and should return "unknown" instead. Check index membership before returning the status.
- except (ValueError, AttributeError):
+ except (ValueError, AttributeError):
# No commits yet (ValueError: Reference does not exist) or detached HEAD
# If we got here, the file is not untracked and has no unstaged changes,
- # so it must be in the index. With no commits, this means it's staged
- # (ready for the first commit).
- return file_path_normalized, "staged"
+ # so it might be staged (in index) OR ignored (not reported as untracked).
+ in_index = any(
+ path == file_path_normalized for (path, _stage) in repo.index.entries
+ )
+ return file_path_normalized, "staged" if in_index else "unknown"🤖 Prompt for AI Agents
In tools/combine-files/combine_files.py around lines 1325 to 1508, the except
block for ValueError/AttributeError (empty repo / no commits) currently returns
"staged" unconditionally which misclassifies ignored files; modify the handler
to first check whether the given path is present in the repository index (i.e.,
the file has an index entry) and only return "staged" if it is present,
otherwise return "unknown"; ensure the check is robust to the same exceptions
and preserve the existing git.exc.GitCommandError fallback to "unknown".
1 participant
This PR extends the
tools/combine_files.pyscript to also add git info about the files. This makes it easier to see from which exact versions of the files a combined file was created.This PR also adds a new CICD job using windows to ensure everything is platform agnostic and makes some changes to fix issues found while adding that.
Note
Adds Git metadata to combined file headers, normalizes shell inlining for cross-platform consistency, updates tests/fixtures and docs, and runs CI on Linux and Windows.
tools/combine-files/combine_files.py):_get_git_info/_get_git_status(repo, path, commit, status); header now usesFrom file:and POSIX paths; insert leading blank line before headers..github/workflows/tools-combine-files-quality.yml):integration-tests-linuxand newintegration-tests-windows; run doctests on Windows; trigger on workflow changes.GitPython>=3.1.41,<4.0.0to requirements.tools/combine-files/README.md):Written by Cursor Bugbot for commit 6c78ee3. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.