14 fix the deployment error#20
Merged
Merged
Conversation
… from file renames
…rvivor badge shows branch name
…N-migrated repos like NumPy showing wrong year)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a CI step to run an incremental survivor-only fossil update, moves fossil computation into Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant ANA as analyse_repository.py
participant ADD as add_fossils.py
participant REPO as Git Repository
participant DATA as data/*.json
Note over CI,ANA: Snapshot analysis (preserve fossils)
CI->>ANA: run analysis
ANA->>REPO: collect snapshots (git ops)
ANA->>DATA: write snapshots (preserve fossils)
Note over CI,ADD: Survivor update step
CI->>ADD: run add_fossils.py --update-survivor
ADD->>REPO: checkout default branch / re-blame HEAD
ADD->>ADD: compute survivor fossil (includes view_commit)
ADD->>DATA: compare identity (file,line,commit,view_commit)
alt Identity changed
ADD->>DATA: atomic write updated survivor to JSON
else Identity unchanged
ADD-->>DATA: no write
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 5
🧹 Nitpick comments (2)
app.js (1)
688-701: Loading-state accessibility can be improved.When showing/hiding the skeleton, also toggle accessibility state (
aria-hiddenfor loader andaria-busyfor chart container) so assistive tech gets correct loading context.♿ Suggested accessibility update
showLoading(show) { + this.loadingState?.setAttribute('aria-hidden', String(!show)); if (show) { this.loadingState.classList.remove('hidden'); // Also hide the chart container while skeleton shows const chartContainer = document.getElementById('chart-container'); - if (chartContainer) chartContainer.style.opacity = '0'; + if (chartContainer) { + chartContainer.style.opacity = '0'; + chartContainer.setAttribute('aria-busy', 'true'); + } } else { this.loadingState.classList.add('hidden'); // Fade the chart back in smoothly const chartContainer = document.getElementById('chart-container'); if (chartContainer) { chartContainer.style.transition = 'opacity 0.35s ease'; chartContainer.style.opacity = '1'; + chartContainer.setAttribute('aria-busy', 'false'); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app.js` around lines 688 - 701, When toggling the skeleton loader (the block that checks the show variable), also update ARIA attributes: set this.loadingState.setAttribute('aria-hidden','false') when showing and 'true' when hiding, and set chartContainer.setAttribute('aria-busy','true') while showing and 'false' when hiding (ensure chartContainer exists before setting). Keep the existing opacity/transition behavior but add these attribute updates in both branches so assistive tech receives the correct loading context.data/numpy_data.json (1)
1-1: Add schema validation forview_commitin integrity tests.The data now consistently includes
view_commit, and runtime link generation depends on it.tests/test_data_integrity.pyshould assert this field so regressions are caught.✅ Suggested test update
for fossil_type, fossil_data in fossils.items(): if not isinstance(fossil_data, dict): continue if not fossil_data: # Empty fossil object is OK continue @@ assert "commit" in fossil_data, ( f"Error in {json_file.name}: Fossil '{fossil_type}' missing 'commit' field" ) + assert "view_commit" in fossil_data, ( + f"Error in {json_file.name}: Fossil '{fossil_type}' missing 'view_commit' field" + ) assert "line" in fossil_data, ( f"Error in {json_file.name}: Fossil '{fossil_type}' missing 'line' field" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/numpy_data.json` at line 1, Update tests/test_data_integrity.py to validate that every fossil entry includes a non-empty 'view_commit' string: locate the fossils checks (inspect 'fossils', specifically 'genesis' and 'survivor') and add assertions that 'view_commit' is present and is a non-empty str (and fail the test if missing/empty) so runtime link generation won't break when view_commit is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app.js`:
- Around line 655-678: buildLink currently returns an HTML string that injects
fossil.file into innerHTML (used by genesis-file and survivor-file), which
allows HTML injection and breaks on special characters; change buildLink to
return a DOM node instead: if no fossil.file return a text node or string,
otherwise create an <a> element via document.createElement('a'), set its
textContent to `${fossil.file}:${fossil.line}`, set href to the constructed URL
but ensure you URL-encode the path portion (encode each path segment or use
encodeURIComponent on fossil.file) and the line fragment, and set attributes
(target, rel) and styles/event listeners via element properties instead of
inline HTML; finally, replace assignments to .innerHTML for genesis-file and
survivor-file with DOM insertion (e.g., element.textContent = display or
element.appendChild(linkNode)) so no unescaped HTML is injected.
In `@scripts/add_fossils.py`:
- Line 395: The logger.info call in scripts/add_fossils.py uses an unnecessary
f-string prefix; change the call to use a plain string (remove the leading f in
the logger.info invocation that currently reads f" Fetching latest...") so it
becomes " Fetching latest..." to satisfy Ruff F541 and eliminate the unused
f-string; locate the logger.info(...) call and update the literal accordingly.
- Around line 405-413: The current equality check uses
_fossil_identity(existing_survivor) vs _fossil_identity(new_survivor) which only
considers (file,line,commit) and therefore skips writing when view_commit
differs (or is empty/old-format); update the logic so view_commit is part of the
identity or add an explicit comparison of existing_survivor.get('view_commit')
vs new_survivor.get('view_commit') and treat them as different if they differ or
if the existing one is empty/old-format, ensuring the write path runs and the
new view_commit metadata is persisted (referencing _fossil_identity,
existing_survivor, new_survivor, and view_commit to locate where to change).
- Around line 420-423: The current write truncates json_file in place and can
leave a corrupted file on crashes; instead write the JSON payload ({"snapshots":
snapshots, "fossils": updated_fossils}) to a temporary file in the same
directory (e.g., using tempfile.NamedTemporaryFile or json_file + ".tmp"), flush
and fsync the temp file, then atomically replace the original with
os.replace(temp_path, json_file); target the block that creates updated_fossils
and the with-open write (references: updated_fossils, snapshots, json_file) so
the swap is atomic and crash-safe.
In `@style.css`:
- Around line 230-240: The overlay currently allows pointer events because it
sets pointer-events: none on .skeleton-loading-overlay, so underlying
SVG/tooltips remain interactive; change the overlay to capture events by
replacing that rule so .skeleton-loading-overlay uses pointer-events: auto (or
otherwise captures pointer events) and, if the overlay's internal elements must
remain non-interactive, add rules to set pointer-events: none on its child
elements (e.g., .skeleton-loading-overlay > *). This ensures the overlay blocks
interactions with underlying charts while still preventing unwanted interaction
on any decorative children.
---
Nitpick comments:
In `@app.js`:
- Around line 688-701: When toggling the skeleton loader (the block that checks
the show variable), also update ARIA attributes: set
this.loadingState.setAttribute('aria-hidden','false') when showing and 'true'
when hiding, and set chartContainer.setAttribute('aria-busy','true') while
showing and 'false' when hiding (ensure chartContainer exists before setting).
Keep the existing opacity/transition behavior but add these attribute updates in
both branches so assistive tech receives the correct loading context.
In `@data/numpy_data.json`:
- Line 1: Update tests/test_data_integrity.py to validate that every fossil
entry includes a non-empty 'view_commit' string: locate the fossils checks
(inspect 'fossils', specifically 'genesis' and 'survivor') and add assertions
that 'view_commit' is present and is a non-empty str (and fail the test if
missing/empty) so runtime link generation won't break when view_commit is
required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b13c68e8-0ccf-45e0-b379-337be5f4425b
📒 Files selected for processing (11)
.github/workflows/theseus-engine.ymlapp.jsdata/claude-code_data.jsondata/langchain_data.jsondata/numpy_data.jsondata/react_data.jsondata/zed_data.jsonindex.htmlscripts/add_fossils.pyscripts/analyse_repository.pystyle.css
…dotexe/Theseus into 14-fix-the-deployment-error
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)
style.css (1)
854-870:⚠️ Potential issue | 🟡 MinorThe new ellipsis styling won't reliably truncate the fossil text.
The actual content lives in the nested
<code>element, but the overflow rules are only on.fossil-code. In this flex row, the<code>item keeps its intrinsic width unless you also give itmin-width: 0and the ellipsis rules, so long fossils can still spill out of the card.Suggested fix
.fossil-code { background: rgba(0, 0, 0, 0.4); border: 1px solid rgba(255, 255, 255, 0.08); border-radius: 0.75rem; padding: 0.6rem 1rem; font-family: var(--font-mono); font-size: 0.85rem; color: var(--text-primary); opacity: 0.8; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; display: flex; align-items: center; gap: 0.5rem; text-align: left; } + +.fossil-code code { + flex: 1; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@style.css` around lines 854 - 870, The nested <code> inside .fossil-code is maintaining intrinsic width so long fossils escape the container; update the styles so the inner code can shrink: add rules targeting .fossil-code code (or the element) to set min-width: 0 and inherit the truncation rules (overflow: hidden; text-overflow: ellipsis; white-space: nowrap) or alternatively give the code flex: 1 with min-width: 0; keep .fossil-code as the flex container but ensure the inner code is allowed to shrink to enable reliable truncation.
🧹 Nitpick comments (1)
tests/test_analyse_repository.py (1)
101-126: Add a test for the object-form state that already contains fossils.
load_existing_state()now has a dedicated{"snapshots": [...], "fossils": {...}}path, but this suite only exercises legacy-list input plus empty/corrupt fallbacks. A regression there would still pass these tests whilescripts/analyse_repository.pyrelies on that structure to preserve fossil data during writes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_analyse_repository.py` around lines 101 - 126, Add a new unit test that covers the object-form state with existing fossils: create a temp JSON file containing {"snapshots": [...], "fossils": {"some_key": {"meta": "..."} }}, call load_existing_state(f.name) and assert the returned dict contains both "snapshots" and "fossils", that snapshots length and snapshot_date match the provided data, and that the fossils object equals the original fossils (e.g. contains "some_key"). Place this alongside test_load_valid_json and name it test_load_object_state_with_fossils to ensure fossil-preservation behavior is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/add_fossils.py`:
- Around line 257-271: The code currently collapses remote refs in
_get_default_branch (it uses result.split("/")[-1]) causing view_commit to be
set to only the last path segment; update the logic so view_commit preserves the
full default-branch ref (e.g., "origin/release/main") instead of collapsing to
"main": either change _get_default_branch to return the full remote ref string
(avoid splitting by "/") or adjust the assignment to view_commit to use the raw
value returned by _get_default_branch; ensure get_survivor_fossil (which now
persists default_branch) receives and stores the full branch name so GitHub
links and survivor labels point to the correct branch.
---
Outside diff comments:
In `@style.css`:
- Around line 854-870: The nested <code> inside .fossil-code is maintaining
intrinsic width so long fossils escape the container; update the styles so the
inner code can shrink: add rules targeting .fossil-code code (or the element) to
set min-width: 0 and inherit the truncation rules (overflow: hidden;
text-overflow: ellipsis; white-space: nowrap) or alternatively give the code
flex: 1 with min-width: 0; keep .fossil-code as the flex container but ensure
the inner code is allowed to shrink to enable reliable truncation.
---
Nitpick comments:
In `@tests/test_analyse_repository.py`:
- Around line 101-126: Add a new unit test that covers the object-form state
with existing fossils: create a temp JSON file containing {"snapshots": [...],
"fossils": {"some_key": {"meta": "..."} }}, call load_existing_state(f.name) and
assert the returned dict contains both "snapshots" and "fossils", that snapshots
length and snapshot_date match the provided data, and that the fossils object
equals the original fossils (e.g. contains "some_key"). Place this alongside
test_load_valid_json and name it test_load_object_state_with_fossils to ensure
fossil-preservation behavior is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82a9658f-7a71-4779-90d8-806b9a6bde9f
📒 Files selected for processing (15)
app.jsdata/claude-code_data.jsondata/langchain_data.jsondata/manifest.jsondata/numpy_data.jsondata/react_data.jsondata/zed_data.jsonindex.htmlscripts/add_fossils.pyscripts/analyse_repository.pyscripts/cleanup_data.pystyle.csstests/test_analyse_repository.pytests/test_data_integrity.pytheseus.config.json
💤 Files with no reviewable changes (1)
- data/manifest.json
✅ Files skipped from review due to trivial changes (4)
- scripts/cleanup_data.py
- theseus.config.json
- tests/test_data_integrity.py
- data/claude-code_data.json
🚧 Files skipped from review as they are similar to previous changes (3)
- data/zed_data.json
- data/numpy_data.json
- data/langchain_data.json
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)
scripts/add_fossils.py (1)
309-316:⚠️ Potential issue | 🟠 MajorA single malformed JSON file can abort the whole job.
Both loaders parse JSON outside a per-repo guard. If one file is corrupt, the script exits before processing the rest.
💡 Suggested fix
- with open(json_file, "r", encoding="utf-8") as f: - raw_data = json.load(f) + try: + with open(json_file, "r", encoding="utf-8") as f: + raw_data = json.load(f) + except (OSError, json.JSONDecodeError) as e: + logger.error(" ✗ Failed to read %s: %s", json_file.name, e) + continueApply the same guard in both
backfill_fossils(...)andupdate_survivor_fossils(...).Also applies to: 420-428
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/add_fossils.py` around lines 309 - 316, Both loaders call json.load(...) outside a per-repo guard which lets one malformed JSON abort the whole job; wrap the file open/json parsing in a try/except inside backfill_fossils(...) and update_survivor_fossils(...) so each repository file is parsed guardedly, catch json.JSONDecodeError (and OSError), log the error with the file path/repo context, and continue to the next file instead of raising—specifically protect the with open(json_file, ...) / raw_data = json.load(f) block and ensure snapshots fallback logic (isinstance check or .get("snapshots")) still runs when parsing succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/add_fossils.py`:
- Around line 260-266: Current checkout can use a stale local branch tip; change
the logic in the block that calls _run_command(["git", "checkout", "--force",
default_branch], cwd=repo_path) so it explicitly checks out the remote tip
(origin/{default_branch}) first (e.g., git checkout --force
origin/{default_branch}) or resets the local branch to the remote (git fetch
origin && git checkout -B {default_branch} origin/{default_branch}), using the
same _run_command and repo_path variables; keep a fallback for detached-head
errors but ensure the primary path uses the remote ref (origin/{default_branch})
to avoid computing Survivor against stale local code.
---
Outside diff comments:
In `@scripts/add_fossils.py`:
- Around line 309-316: Both loaders call json.load(...) outside a per-repo guard
which lets one malformed JSON abort the whole job; wrap the file open/json
parsing in a try/except inside backfill_fossils(...) and
update_survivor_fossils(...) so each repository file is parsed guardedly, catch
json.JSONDecodeError (and OSError), log the error with the file path/repo
context, and continue to the next file instead of raising—specifically protect
the with open(json_file, ...) / raw_data = json.load(f) block and ensure
snapshots fallback logic (isinstance check or .get("snapshots")) still runs when
parsing succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/add_fossils.py (1)
294-296: Anchor runtime paths to the repository root.
theseus.config.json,dataDir, and./temp_fossil_reposare all resolved from the process CWD right now. Deriving them fromPath(__file__)makes CI and local invocation deterministic.Also applies to: 404-405, 516-525
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/add_fossils.py` around lines 294 - 296, The current code resolves runtime paths (data_path, temp_dir and other occurrences) from the process CWD; change them to be anchored to the repository/script root by computing a base root = Path(__file__).resolve().parent (or the repo root directory as needed) and then derive paths like data_path = base / data_dir and temp_dir = base / "temp_fossil_repos"; apply the same pattern for the other occurrences referenced (lines around 404-405 and 516-525) so all references to theseus.config.json, dataDir, and temp dirs use the base variable instead of the CWD.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/add_fossils.py`:
- Around line 383-384: The except blocks that currently only log errors (the
except Exception as e handlers that log "✗ Error computing fossils for %s: %s"
using logger and similar at the other location) must mark the run as failed and
cause main() to exit non‑zero; introduce a boolean (e.g., had_failures = False)
or an integer failure_count in the scope of main(), set had_failures = True or
increment failure_count inside each except block (reference the repo_name
variable used in the log), and after processing all repos check that flag and
call sys.exit(1) (or return a non‑zero code from main) if any failures occurred;
ensure you import sys and update both exception handlers (the one shown and the
similar handler around lines 499-500) to flip the failure flag rather than only
logging.
---
Nitpick comments:
In `@scripts/add_fossils.py`:
- Around line 294-296: The current code resolves runtime paths (data_path,
temp_dir and other occurrences) from the process CWD; change them to be anchored
to the repository/script root by computing a base root =
Path(__file__).resolve().parent (or the repo root directory as needed) and then
derive paths like data_path = base / data_dir and temp_dir = base /
"temp_fossil_repos"; apply the same pattern for the other occurrences referenced
(lines around 404-405 and 516-525) so all references to theseus.config.json,
dataDir, and temp dirs use the base variable instead of the CWD.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 492be261-cd1f-4a5c-8c6b-235cace2fd2b
📒 Files selected for processing (6)
data/claude-code_data.jsondata/langchain_data.jsondata/numpy_data.jsondata/react_data.jsondata/zed_data.jsonscripts/add_fossils.py
✅ Files skipped from review due to trivial changes (1)
- data/react_data.json
🚧 Files skipped from review as they are similar to previous changes (3)
- data/zed_data.json
- data/claude-code_data.json
- data/langchain_data.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/add_fossils.py (1)
384-386: Consider narrowing the exception type.The broad
Exceptioncatch is appropriate for batch processing (ensuring one repo failure doesn't halt others), and the failure tracking pattern is correct. For better specificity, you could narrow to the expected exception types:- except Exception as e: # pylint: disable=broad-exception-caught + except (RuntimeError, OSError, json.JSONDecodeError, KeyError) as e:This would still catch git failures (
RuntimeError), file I/O issues (OSError), and data parsing errors while avoiding silencing truly unexpected exceptions.Also applies to line 504.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/add_fossils.py` around lines 384 - 386, The except block currently catches broad Exception when computing fossils (the handler that logs "✗ Error computing fossils for %s: %s", repo_name, e and sets had_failures = True); narrow it to expected error types (e.g., RuntimeError, OSError, ValueError or json/parse exceptions used in your parsing code) so you still capture git, file I/O and parsing failures but avoid silencing unexpected errors, and apply the same change to the similar except at the other occurrence around line 504; keep the same logging and had_failures behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/add_fossils.py`:
- Around line 384-386: The except block currently catches broad Exception when
computing fossils (the handler that logs "✗ Error computing fossils for %s: %s",
repo_name, e and sets had_failures = True); narrow it to expected error types
(e.g., RuntimeError, OSError, ValueError or json/parse exceptions used in your
parsing code) so you still capture git, file I/O and parsing failures but avoid
silencing unexpected errors, and apply the same change to the similar except at
the other occurrence around line 504; keep the same logging and had_failures
behavior.
Merged
This was referenced May 31, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores
Tests