Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions frontend/src/components/OBSViewDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,6 @@ export const OBSViewDialog = ({
return;
}

if (meta.story_no !== selectedStory.story_no) {
toast.info(
"Story number cannot be changed. You can edit only the title text.",
);
return;
}

try {
await updateMutation.mutateAsync({
resource_id: resourceId,
Expand Down
21 changes: 6 additions & 15 deletions frontend/src/utils/obsParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const ISL_HEADERS = ["storyNo", "title", "url", "description"];
// Supports:
// # 1. Creation
// # 1۔ تخلیق
const OBS_HEADING_REGEX = /^#+\s*(\d+)\s*[^\p{L}\p{N}]\s*(.+)$/u;
const OBS_HEADING_REGEX = /^#+\s*(\p{N}+)\s*[^\p{L}\p{N}]\s*(.+)$/u;

const cleanHeader = (header: string) =>
header
Expand All @@ -17,10 +17,7 @@ const cleanHeader = (header: string) =>
export const isISLOBSCSV = (headers: string[]) =>
ISL_HEADERS.every((h) => headers.map(cleanHeader).includes(h.toLowerCase()));

function extractHeadingData(headingLine: string): {
story_no: number;
title: string;
} {
function extractHeadingData(headingLine: string): { title: string } {
const normalized = headingLine.normalize("NFC");

const match = normalized.match(OBS_HEADING_REGEX);
Expand All @@ -31,10 +28,7 @@ function extractHeadingData(headingLine: string): {
);
}

return {
story_no: Number(match[1]),
title: match[2].trim(),
};
return { title: match[2].trim() };
}

export function parseISLOBSCSV(file: File): Promise<OBSStory[]> {
Expand Down Expand Up @@ -69,7 +63,7 @@ export function parseISLOBSCSV(file: File): Promise<OBSStory[]> {

export async function parseOBSMarkdown(file: File): Promise<OBSStory> {
const content = await file.text();

const story_no = parseInt(file.name.replace(/\.md$/i, ""), 10);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: parseInt is used directly on the filename stem without validating that the entire stem is numeric. Filenames like abc.md produce NaN, and filenames like 1-extra.md are silently coerced to 1, which can lead to invalid or incorrect story_no values being uploaded and break sorting/dedup behavior. Enforce a strict numeric filename format and reject invalid names before returning the story object. [logic error]

Severity Level: Major ⚠️
- ❌ Malformed filenames create stories with NaN story numbers.
- ⚠️ Story deduplication by story_no fails for NaN.
- ⚠️ Sorting stories by story_no becomes inconsistent with NaN.
Steps of Reproduction ✅
1. In `OBSViewDialog` (`frontend/src/components/OBSViewDialog.tsx:229-241`),
`handleAddStories` accepts any `.md` files without validating that filenames are numeric.

2. `handleAddStories` calls `parseOBSMarkdownFiles`
(`frontend/src/utils/obsParser.ts:84-89`), which in turn calls `parseOBSMarkdown` for each
file.

3. In `parseOBSMarkdown` (`frontend/src/utils/obsParser.ts:64-79`), `story_no` is derived
via `parseInt(file.name.replace(/\.md$/i, ""), 10)` at line 66; passing a file named
`abc.md` yields `parseInt("abc", 10) === NaN`, and a file named `1-extra.md` yields
`parseInt("1-extra", 10) === 1`.

4. The resulting `OBSStory` objects with `story_no` set to `NaN` or coerced values are
used in `handleAddStories` to build `existing` and `toUpload` sets
(`OBSViewDialog.tsx:242-247`) and later for display and deletion, meaning malformed
filenames lead to invalid IDs, broken deduplication (`Set.has(NaN)` is always false), and
inconsistent sorting by `story_no`.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** frontend/src/utils/obsParser.ts
**Line:** 66:66
**Comment:**
	*Logic Error: `parseInt` is used directly on the filename stem without validating that the entire stem is numeric. Filenames like `abc.md` produce `NaN`, and filenames like `1-extra.md` are silently coerced to `1`, which can lead to invalid or incorrect `story_no` values being uploaded and break sorting/dedup behavior. Enforce a strict numeric filename format and reject invalid names before returning the story object.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

const lines = normalizeContent(content).split("\n");

const headingLine = lines.find((l) => l.trim().startsWith("#"));
Expand All @@ -78,7 +72,7 @@ export async function parseOBSMarkdown(file: File): Promise<OBSStory> {
throw new Error("Invalid OBS markdown: missing heading");
}

const { story_no, title } = extractHeadingData(headingLine);
const { title } = extractHeadingData(headingLine);

return {
story_no,
Expand All @@ -95,10 +89,7 @@ export async function parseOBSMarkdownFiles(
return stories.sort((a, b) => a.story_no - b.story_no);
}

export function extractOBSMetadataFromMarkdown(markdown: string): {
story_no: number;
title: string;
} {
export function extractOBSMetadataFromMarkdown(markdown: string): { title: string } {
const lines = normalizeContent(markdown).split("\n");

const headingLine = lines.find((l) => l.trim().startsWith("#"));
Expand Down
Loading